string.c: reject UTF-8 sequences forbidden by RFC 3629 - #7093
Merged
Conversation
`mrb_utf8len` checked only the length implied by the lead byte and the continuation bytes, so it accepted three classes of sequences that the RFC 3629 grammar excludes from UTF-8: - overlong encodings: `C0`/`C1` leads, `E0 80-9F`, `F0 80-8F` - UTF-16 surrogates U+D800 to U+DFFF: `ED A0-BF` - code points above U+10FFFF: `F4 90-BF`, `F5` to `F7` leads Add the lead-specific second byte range checks after the existing continuation byte checks. A rejected sequence takes the same path as every other invalid sequence: it counts one byte and moves on. `mrb_utf8len` backs `String#size`, character indexing, and `String#valid_encoding?`, which now agree with CRuby on such input: ```ruby s = "\xED\xA0\x80" # encodes the surrogate U+D800 s.valid_encoding? # CRuby: false, mruby before: true, after: false s.size # CRuby: 3, mruby before: 1, after: 3 ``` mruby-string-ext already enforces these rules in `utf8code()` behind `String#ord` and in `str_scrub_char_len()` behind `String#scrub`; this brings `mrb_utf8len` in line with them.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe UTF-8 decoder now enforces RFC 3629 limits. Tests cover overlong encodings, surrogate code points, out-of-range code points, invalid byte counting, and valid boundary code points. ChangesUTF-8 validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Aug 12, 2026
matz
pushed a commit
that referenced
this pull request
Aug 12, 2026
Core stopped accepting the UTF-8 sequences RFC 3629 forbids in #7093, and `utf8code()` in this gem was taught the same rules by hand. The tests for those rules go through `String#scrub`, which reads its lengths from a different helper, so nothing holds `ord` and `codepoints` to them. Pin them: a stray continuation byte, a truncated sequence, an overlong encoding, a surrogate, a code point above U+10FFFF and a five-byte lead all raise. `String#codepoints` also has to keep its place across a 4-byte character, so a string that continues past one is checked too.
matz
pushed a commit
that referenced
this pull request
Aug 12, 2026
`str_scrub_char_len()` asks whether the bytes at p start a valid UTF-8 character and, if they do, how many bytes it takes. `mrb_utf8len()` answers that question in core, and answers it by the same rules: both were brought to RFC 3629 in #7093, one after the other. What has to be bridged is how a refusal comes back. `mrb_utf8len()` answers 1 for every sequence it turns down, and a real one-byte character is a lead byte below 0x80, so a length of 1 on a byte at or above 0x80 is the refusal. The two were checked against each other over both leading bytes, nine representative tail bytes and every available length from 1 to 4: 21,233,664 cases, no difference.
This was referenced Aug 12, 2026
This was referenced Aug 12, 2026
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
2 participants
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.
Problem
mrb_utf8len()checks only the length implied by the lead byte and thatthe following bytes are continuation bytes (
80-BF). That accepts threeclasses of sequences the RFC 3629 grammar excludes from UTF-8:
C0/C1leads,E0 80-9F,F0 80-8FED A0-BFF4 90-BF,F5toF7leadsSince
mrb_utf8len()backsString#size, character indexing, andString#valid_encoding?, these sequences were treated as validcharacters:
Change
Add the lead-specific second byte range checks after the existing
continuation byte checks. A rejected sequence takes the same path as
every other invalid sequence: it counts one byte and moves on.
mruby-string-ext already enforces these rules in
utf8code()behindString#ordand instr_scrub_char_len()behindString#scrub; thisbrings
mrb_utf8len()in line with them. Boundary code points (U+D7FF,U+E000, U+10FFFF) remain valid.
Validation
ABNF: all 1 to 3 byte windows and all 4 byte windows with
F0-FFleads (285,278,464 windows in total, the remaining leads never read a
fourth byte) return the character length for valid sequences and 1
for everything else, with zero mismatches. The throwaway harness is
not part of this PR.
String#sizecounts each byte of a rejected sequence(test/t/string.rb) and
String#valid_encoding?covers all threeclasses plus the valid boundary code points
(mrbgems/mruby-encoding/test/string.rb).
rake testpasses with the default gembox and with full-core(
MRB_UTF8_STRINGenabled).Summary by CodeRabbit
Bug Fixes
Tests