Skip to content

Stop a broken string from being read one byte at a time - #7131

Merged
matz merged 3 commits into
mruby:masterfrom
takumin:string-single-byte-ascii-only
Aug 12, 2026
Merged

Stop a broken string from being read one byte at a time#7131
matz merged 3 commits into
mruby:masterfrom
takumin:string-single-byte-ascii-only

Conversation

@takumin

@takumin takumin commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

The flag

MRB_STR_SINGLE_BYTE says a string holds one character per byte, and its
readers take it as leave to step through the bytes without decoding them.
RSTR_SET_ASCII_FLAG is a second name for setting it, and String#ascii_only?
sets it exactly where every byte is ASCII.

Two places set it from a weaker fact. mrb_str_char_len() sets it when the
character count comes out equal to the byte count, and str_escape() sets it on
the receiver of inspect when the walk copied no character of several bytes
across. A byte that spells no character is counted as one, and is escaped one at
a time, so a string carrying nothing else satisfies both.

What that cost

s = "\xED\xA0\x80"
s.codepoints         # ArgumentError
s.length             #=> 3
s.codepoints         #=> [237, 160, 128]
s.ord                #=> 237, where it raised
s.scrub.bytes        #=> [237, 160, 128], where it was [239, 191, 189] * 3

inspect leaves the string the same way, and the flag survives dup,
replace, String#*, freeze and a byte slice, so the answer travels. CRuby
answers all of these the same way whatever ran before.

String#scrub is the one that matters most: a string of broken bytes is exactly
what it exists to repair, and it handed the string back untouched wherever
anything had counted it first.

The change

Every character a non-ASCII byte begins spells two bytes or more, and a
non-ASCII byte that begins none spells no character at all, so a string holds
one character per byte exactly when every byte of it is ASCII. Both places now
ask that.

mrb_str_char_len() already looks for the first non-ASCII byte before it counts
anything, so where there is none the answer is the byte count and the flag
holds, and where there is one the count carries on from it. str_escape()
clears the receiver's flag as soon as it meets a non-ASCII byte. The flag it
puts on what inspect returns keeps its old rule, since \xNN is what a byte
spelling no character is escaped to and the return is still one character per
byte; only a whole character copied across takes that from it. The two answers
needed separating to say so.

What does not change

String#length, #size, #chars, #[], #inspect and #dump answer exactly
as before. A byte that spells no character is still counted as a character and
still takes an index of its own. The walks that #chars and #[] fall back to
without the flag read such a byte as one character anyway, which is what the
first commit pins.

The cost

A string that is not all ASCII is counted every time it is asked. Broken strings
used to be counted once and remembered; they now join the class every valid
multi-byte string was already in. Counting a 100 KB string 2000 times:

receiver before after
all ASCII 0.2 ms 0.1 ms
valid multi-byte 243 ms 187 ms
bytes that spell no character 0.5 ms 196 ms

The ASCII path is one search_nonascii() sweep either way, and the flag still
holds after the first count.

What is left

int_chr_binary() in mruby-string-ext sets the flag on a one-byte string
whose byte may be 0x80 or above, and sets no MRB_STR_BINARY beside it. Under
the narrower meaning that is a false statement, though what it produces is what
CRuby produces:

171.chr.codepoints        #=> [171] in CRuby and in mruby
171.chr.valid_encoding?   #=> true in CRuby, false in mruby

Setting MRB_STR_BINARY there instead would close it, and would settle the
second line as well. That is a separate change and is not in this PR.

With it closed, mrb_str_valid_encoding_p() could answer true from the flag
alone and feed the MRB_STR_VALID_ENC cache. This PR only removes the comment
that said the flag could not be read there.

Tests

The first commit pins what a byte spelling no character counts and indexes as,
before anything moves. Each of the other two carries the cases its own change
decides, and taking either change back out fails that commit's assertions and no
others.

rake test is green on three builds: full-core with MRB_UTF8_STRING, the
default gembox without it, and full-core with MRB_UTF8_STRING and
MRB_INT32. Each commit was checked on its own. A sweep of nine broken strings
against twenty-four ways of touching one first found 91 places where the bytes
came back as characters, and none after.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of malformed UTF-8 strings.
    • String length, character access, iteration, inspection, and escaping now preserve invalid-byte information correctly.
    • Invalid and truncated byte sequences are counted and handled consistently instead of being incorrectly treated as ASCII.
    • Operations such as ord, codepoints, and scrub now produce the expected results for malformed strings while maintaining normal ASCII behavior.
  • Tests

    • Added comprehensive coverage for invalid, truncated, mixed, and valid UTF-8 strings.

`String#length` counts such a byte as a character of its own, and the tests
that ask it do so of strings holding nothing else. What the same byte counts
as beside whole characters, and what index it takes, is asked nowhere, and
both are answers the character count decides.

So ask them: a broken sequence next to ASCII and next to a character of
several bytes, and `String#[]` reading a surrogate's bytes back one at a
time. `String#chars` already pins the split itself, and these are the count
and the index the split is read through.
…aracter

`inspect` walks the string it prints, and what it meets on the way settles
MRB_STR_SINGLE_BYTE on the receiver: the flag stays set unless a character of
several bytes is copied across whole. A byte that spells no character is
copied one at a time, like an ASCII one, so a string carrying one came out of
`inspect` flagged as holding one character per byte.

That flag is read as leave for stepping through the bytes without decoding
them. `String#codepoints` and `String#ord` then hand the bytes back as the
characters the string does not hold, and `String#scrub` finds nothing to
replace, so what a broken string answers turns on whether `inspect` was
called on it first:

```ruby
s = "\xED\xA0\x80"
s.codepoints         # ArgumentError
s.inspect
s.codepoints         #=> [237, 160, 128]
s.scrub.bytes        #=> [237, 160, 128], where it was [239, 191, 189] * 3
```

CRuby answers those the same way whatever ran before. Ask the walk about the
byte instead of about the character it failed to begin: a non-ASCII byte
leaves the string more bytes than characters whether or not it spells one.

What `inspect` returns is unchanged, and the flag it puts on that return is
too. `\xNN` is what a byte spelling no character is escaped to, so the string
that comes back is still one character per byte, which only a whole character
copied across takes from it. The two answers needed separating to say so.
…cter

Counting characters sets MRB_STR_SINGLE_BYTE when the count comes out equal to
the byte count, and the flag is read as leave for stepping through the bytes
without decoding them. A byte that spells no character is counted as one, so a
string carrying nothing else counts one per byte and set the flag too. Its
bytes then came back as the characters it does not hold:

```ruby
s = "\xED\xA0\x80"
s.codepoints         # ArgumentError
s.length             #=> 3
s.codepoints         #=> [237, 160, 128]
s.ord                #=> 237, where it raised
s.scrub.bytes        #=> [237, 160, 128], where it was [239, 191, 189] * 3
```

CRuby answers those the same way whatever ran before. Every character a
non-ASCII byte begins spells two bytes or more, and a non-ASCII byte that
begins none spells no character at all, so a string holds one character per
byte exactly when every byte of it is ASCII. Ask that instead: the count
already looks for the first non-ASCII byte before it counts anything, so where
there is none the answer is the byte count and the flag holds, and where there
is one the count carries on from it.

`String#length` still counts a byte spelling no character as a character, and
`String#chars` and `String#[]` still give it a position of its own. Only what
is remembered about the string changes, and the walk those two take without
the flag reads such a byte as one character anyway. The test before this one
asks for that.

`RSTR_SET_ASCII_FLAG` is already a second name for setting this flag, and
`String#ascii_only?` sets it exactly where every byte is ASCII, so this is the
meaning the flag's readers were written against. The comment in
`mrb_str_valid_encoding_p` argued from the wider reading and goes.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 92fa0e87-2307-44c8-9cec-262eb8b38549

📥 Commits

Reviewing files that changed from the base of the PR and between 57b0e66 and d78e6eb.

📒 Files selected for processing (4)
  • mrbgems/mruby-string-ext/src/string.c
  • mrbgems/mruby-string-ext/test/string.rb
  • src/string.c
  • test/t/string.rb

📝 Walkthrough

Walkthrough

UTF-8 character-length detection now preserves malformed non-ASCII state. String escaping tracks source and output flags separately. Tests cover inspection, indexing, sizing, character iteration, codepoints, ordinals, and scrubbing.

Changes

Malformed UTF-8 handling

Layer / File(s) Summary
UTF-8 classification and escaping
src/string.c, mrbgems/mruby-string-ext/src/string.c
Character-length detection marks strings as single-byte only when all bytes are ASCII. Escaping tracks the source string’s single-byte state separately from the generated result.
Malformed-string operation tests
test/t/string.rb, mrbgems/mruby-string-ext/test/string.rb
Tests cover malformed UTF-8 after inspection, character operations, indexing, sizing, codepoint access, ordinal access, and scrubbing. ASCII behavior remains covered.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • mruby/mruby#7105: Extends related malformed UTF-8 handling in src/string.c and string-character tests.
  • mruby/mruby#7106: Refines malformed-sequence handling and MRB_STR_SINGLE_BYTE classification.
  • mruby/mruby#7102: Modifies UTF-8 length handling and MRB_STR_SINGLE_BYTE classification.

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: preventing malformed strings from being treated as single-byte strings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 04b8262 into mruby:master Aug 12, 2026
21 checks passed
takumin pushed a commit to takumin/mruby that referenced this pull request Aug 12, 2026
A search under mruby-regexp refuses a subject whose bytes are not UTF-8
(mruby#7126), and what an unwalked subject pays there is one walk. The walk's
answer is remembered on the string it read, and mruby#7131 lets a string
already known ASCII answer without one, but the operations a subject is
usually built with still dropped what their sources knew: `+` copied no
flags at all, `<<` and interpolation took them down on every append, and
a piece cut out of a walked string kept only the single-byte mark. So
the subject of `(mb + "z") =~ /z/` walked 300001 bytes per search,
300000 of which a walk had already read.

Validity survives every one of those builds, each for a reason the build
can afford to check:

- Concatenation: bytes that read as UTF-8 followed by bytes that do read
  as UTF-8 end to end, each side's sequences being complete on their
  own. `+`, `<<`, `concat` and interpolation hand the result the flag
  when both sides carry it, and two ASCII sides make an ASCII whole,
  which carries MRB_STR_SINGLE_BYTE the same way. An empty side carries
  both by holding nothing.

- Repetition: `str * n` holds nothing but the source's bytes over and
  over, so the source's answer is the result's.

- A byte range: a range of a valid string reads whole exactly when both
  cuts land on character boundaries, and in a valid string every byte
  that is not a continuation byte begins a character. So the byte each
  cut lands on decides it--the one at `beg`, and the one just past the
  range, with the end of the string standing for a boundary--in two byte
  tests, no walk. This is what hands a searched subject's flag down to
  the pieces `split`, `scan` and `byteslice` cut from it.

- Measuring: past the ASCII run the count now returns early on, it
  decodes every sequence anyway, so the same walk also says whether each
  spelled a character, and leaves that answer where the next search
  finds it. A stray byte still counts as one character, which is the
  count this has always returned.

What would defeat the hand-off is the short literal nothing has walked:
the "z" above arrives from the pool flagless on every evaluation, and
one flagless side spoils the pair. So a side of at most 16 bytes is
walked on the spot--all-ASCII answers in a word test--bounded by the
length check and below the cost of the copy its caller is doing anyway.
That is what lets `mb + "z"`, `"#{mb}z"` and the `"あ" * 100000` that
built `mb` come out flagged. The answer is not written back to the
walked side: it is most often a literal this evaluation made and the
next one remakes, so there is no later read to save a walk for, and
re-walking a reused one costs these same few bytes.

The flag never claims more than a walk of the same bytes would find, so
no answer changes. The test that puts the same bytes through the same
operations warm and cold now runs the new builders too, over broken
bases as well as whole ones, and a new block pins the boundary-byte
cases where a cut must not inherit.

At `-O3` on a full-core build, the fastest of seven runs interleaved
between builds, with `mb = "あ" * 100000`:

| | master | with this |
|---|---|---|
| `(mb + "z") =~ /z/`, 200 times, less the builds | 178.2 ms | 96.1 ms |
| `"#{mb}z" =~ /z/`, the same | 195.3 ms | 111.9 ms |
| `"hello world" =~ /world/`, 100000 times | 235.0 ms | 242.0 ms |
| `s << "abcdefgh"`, 100000 times, never searched | 15.1 ms | 16.1 ms |

The first two land where they stood before mruby#7126 made subjects
checkable at all (96.3 ms and 113.7 ms on the same machine): nothing
walks but the "z". Pieces split off a walked subject stop paying their
own first walk the same way. The last row is the cost's worst case, a
loop of nothing but eight-byte appends to a buffer that stays
known-valid, where the flag reads are around a third of the growth and
the bounded walk the rest; it moved six to twelve percent across
measurement rounds here. A loop appending sides longer than the bound
does not walk them, and one whose buffer was never known valid drops to
the flag tests alone. `split`, `gsub`, `rindex`, the third row and
String#length move a few percent either way between rounds, with no
changed code on their paths--the jitter code layout pays for this
file growing.

A literal born flagged would close the walk of the "z" too, but the
pool entry has no bit left to carry the answer in (IREP_TT_STR spends
them all), and stamping it at OP_STRING would only move the same walk
there. That is a dump-format question for another day.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6X4dd382CCZmfrXSpFsb8
@takumin
takumin deleted the string-single-byte-ascii-only branch August 13, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants