Fix StringValidator counting bytes instead of code points for minLength/maxLength#1038
Open
danieleades wants to merge 1 commit into
Open
Fix StringValidator counting bytes instead of code points for minLength/maxLength#1038danieleades wants to merge 1 commit into
danieleades wants to merge 1 commit into
Conversation
JSON Schema (and RFC 8259) define minLength/maxLength as counting Unicode code points, not UTF-8 bytes. StringValidator::is_valid used s.len(), which is a byte count in Rust, so multi-byte strings like "héllo" (5 code points, 6 bytes) were wrongly rejected by a maxLength: 5 constraint, and multi-byte strings could wrongly pass a minLength check. This mirrors the fix already applied to the generated FromStr/TryFrom validation code in type_entry.rs (oxidecomputer#776), which this validator was missed by. StringValidator is used at schema-processing time (e.g. matching enum variants against string constraints), so it has the same class of bug independently of the codegen output. Switch both comparisons to chars().count() and add tests covering multi-byte and 4-byte (emoji) code points at exact boundary lengths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
JSON Schema's
minLength/maxLength(validation spec §6.3, and consistent with RFC 8259's treatment of JSON strings) are defined to count Unicode code points, not bytes.StringValidator::is_validintypify-impl/src/util.rscompared againsts.len(), which in Rust is the byte length of a&str. This meant a string like"héllo"(5 code points, 6 UTF-8 bytes) could be wrongly rejected by amaxLength: 5constraint, and multi-byte strings could wrongly pass aminLengthcheck they shouldn't.StringValidatoris used at schema-processing time (e.g. matching string constants/enum variants against string constraints inconvert.rs), separately from the generated Rust validation code.Note: the generated
FromStr/TryFromvalidation code intype_entry.rsalready usesvalue.chars().count()for this, fixed previously in #776 for issue #775. That fix didn't touchStringValidator, which has the same class of bug.Fix
Switch both the
max_lengthandmin_lengthcomparisons inStringValidator::is_validfroms.as_ref().len()tos.as_ref().chars().count(), matching the approach already used in the generated code.Trade-off
chars().count()is O(n) versus O(1) forlen(), but this is negligible on validation paths. Using code points (rather than grapheme clusters) matches what the JSON Schema spec actually specifies.Testing
Added unit tests in
typify-impl/src/util.rscovering multi-byte characters ("héllo") and 4-byte code points (emoji) at exact boundary lengths for bothminLengthandmaxLength.cargo test --workspace --lockedandcargo fmt --all -- --checkpass.