Skip to content

RUST-2354 Add option for max length when parsing cstrings#643

Open
isabelatkinson wants to merge 3 commits intomongodb:mainfrom
isabelatkinson:max-cstr-len
Open

RUST-2354 Add option for max length when parsing cstrings#643
isabelatkinson wants to merge 3 commits intomongodb:mainfrom
isabelatkinson:max-cstr-len

Conversation

@isabelatkinson
Copy link
Contributor

@isabelatkinson isabelatkinson commented Feb 19, 2026 •
edited
Loading

The maximum length can be configured on an iterator or through new getter methods:

// document
let key = cstr!("aaaaa");
let doc = rawdoc! { key: "b" };
let iter = doc.iter().max_cstr_parse_len(key.len());
let b = doc.get_with_max_cstr_parse_len(key, key.len());

// array
let array = doc.get_array("array").unwrap();
let iter = array.into_iter().max_cstr_parse_len(5);
let val = array.get_with_max_cstr_parse_len(2, 5);

The API additions are exposed behind an sfp-internal feature flag. The documentation for them is hidden. This will give us more semver flexibility to make breaking changes if needed. Docs for the new features can be generated locally by running:

RUSTDOCFLAGS='-Z unstable-options --document-hidden-items' cargo +nightly doc --all-features --open

jyemin
jyemin requested changes Feb 19, 2026
View reviewed changes
@isabelatkinson isabelatkinson marked this pull request as ready for review February 19, 2026 20:12
@isabelatkinson isabelatkinson requested a review from a team as a code owner February 19, 2026 20:12
@isabelatkinson isabelatkinson requested review from abr-egn, jyemin and kevinAlbs and removed request for abr-egn February 19, 2026 20:12
kevinAlbs
kevinAlbs reviewed Feb 20, 2026
View reviewed changes
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with a suggestion to validate regex.

abr-egn
abr-egn approved these changes Feb 23, 2026
View reviewed changes
src/raw/array.rs
/// encountered that exceeds the provided `len`.
#[cfg(feature = "sfp-internal")]
#[doc(hidden)]
pub fn get_with_max_cstr_parse_len(
Copy link
Contributor

@abr-egn abr-egn Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not needed for this PR since this is an internal API we can change if needed)

If we end up wanting to add any more control over parsing we might want to change this to get_with_options and make a proper options struct.

@patrickfreed
Copy link
Contributor

patrickfreed commented Feb 24, 2026

Going to give this a quick try in monguard tomorrow to confirm we're not missing any requirements, but the interface LGTM after a quick look.

@patrickfreed
Copy link
Contributor

patrickfreed commented Mar 18, 2026

Sorry for the delay on this, I wasn't able to find time to try this out in monguard itself and have rotated off that team. Deferring to @jyemin 's review on this. FWIW, it looks fine to me for now, and whoever picks up https://jira.mongodb.org/browse/SERVER-120749 on the monguard side can surface any further requirements if necessary.

@isabelatkinson
Copy link
Contributor Author

isabelatkinson commented Mar 18, 2026

@jyemin Can you please hit approve on this? Merging is blocked because you requested changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevinAlbs kevinAlbs kevinAlbs approved these changes

@abr-egn abr-egn abr-egn approved these changes

@jyemin jyemin Awaiting requested review from jyemin

@patrickfreed patrickfreed Awaiting requested review from patrickfreed

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@isabelatkinson @patrickfreed @kevinAlbs @jyemin @abr-egn