String#chars splits where String#length counts - #7106
Merged
Conversation
Core stopped accepting the UTF-8 sequences RFC 3629 forbids in mruby#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.
`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 mruby#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.
`utf8code()` judged the sequence itself before decoding it: the continuation bytes, then the RFC 3629 rules for overlong encodings, surrogates and code points above U+10FFFF, all written a second time. `mrb_utf8len()` passes that same judgment, so ask it and decode what it accepts. `String#codepoints` stepped over each character by the raw `mrb_utf8len_table`, which is the length a lead byte claims rather than the length that was accepted. The two agree wherever `utf8code()` returns, but the caller no longer has to know that: the byte length comes back through `lenp`. The table is still read by `String#chars`, so the declaration stays. The two were checked against each other over the same 21,233,664 cases as `str_scrub_char_len()`, with no difference in the code point, in what is refused, or in the byte length.
`str_chars_ary()` measured each character itself, by the length its lead byte claims with the continuation bytes checked. That accepts what RFC 3629 forbids and core refuses: an overlong encoding, a UTF-16 surrogate, a code point above U+10FFFF. The same string was then cut into fewer pieces than it was counted as having: ```ruby "\xC0\x80".chars #=> ["\xC0\x80"] (CRuby: ["\xC0", "\x80"]) "\xC0\x80".length #=> 2 "\xED\xA0\x80".chars #=> ["\xED\xA0\x80"] "\xED\xA0\x80".length #=> 3 ``` Walk with `mrb_utf8len()`, the measure the count is taken by, and the two agree again. `mrb_str_char_len()` gives the array its capacity, which is now the number of characters rather than a byte length halved, and on the way there it settles the single-byte flag the walk reads. The gem is left reading no UTF-8 table of its own, so the declaration goes with the loop.
The table says how many bytes a lead byte claims. That is the first question `mrb_utf8len()` asks and no answer on its own: it says nothing about whether the bytes that follow make the character claimed, which is the whole of what the function goes on to check. mruby-string-ext was reading the table directly and cutting strings by it, which is what the previous commit ended. Nothing outside this file reads it now, so let the file keep it.
|
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)
📝 WalkthroughWalkthroughUTF-8 decoding now uses ChangesUTF-8 string behavior
Estimated code review effort: 3 (Moderate) | ~25 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
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
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.
String#charsmeasured characters by a rule of its own: the byte length thelead byte claims, with the continuation bytes checked. Core stopped accepting
what RFC 3629 forbids in #7093, and this rule never did, so one string was cut
into fewer pieces than it was counted as having.
An overlong encoding, a UTF-16 surrogate and a code point above U+10FFFF all
came back as one character each.
The walk
mrb_utf8len()is the measure the count is taken by, so walking with it putscharsandlengthback on the same footing. The rest of the helper followsfrom that:
mrb_str_char_len()gives the array a capacity that is the numberof characters rather than a byte length halved, and on its way there it settles
the single-byte flag the walk reads, which is what tells a byte-indexed string
apart. 31 lines replace 51.
The table
The gem was reaching into
mrb_utf8len_tabledirectly, and this was its lastuse. The table says what a lead byte claims and nothing about whether the bytes
after it deliver, which is the whole of what
mrb_utf8len()goes on to check,so cutting strings by it was the shape of the bug. Nothing outside
src/string.creads it now, so it becomes static there.
Base
Branched off #7105, which is still open, because that PR removes the gem's two
other uses of the table. The first three commits here are that PR and drop out
once it merges.
Tests
The new assertions cover the three sequences RFC 3629 forbids, a truncated
sequence, a stray continuation byte and a lead byte that leads nothing, then
check
str.length == str.chars.sizeover all of them. They fail on the firstthree assertions before this change and pass after it.
Verified
rake teston a full-core build (MRB_UTF8_STRINGthrough mruby-encoding):2244 tests, all green
rake teston the default gembox (noMRB_UTF8_STRING): 2055 tests, allgreen
prek run --all-filespasses, except thatmarkdownlintcould not installlocally (npm engine mismatch); no Markdown is touched here
Summary by CodeRabbit
String#ord,String#codepoints, and character extraction.String#charsnow correctly separates malformed sequences into byte-level elements and reports accurate results.