string.c: String#length stops at the end of a shared substring - #7096

mrb_utf8_strlen() counts the characters of a byte range, and the inner
loop that walks a run of non-ASCII bytes tests only the byte it is about
to decode:

while (NOASCII(*p)) {
  p += mrb_utf8len(p, e);
  len++;
}

Nothing there stops at e. The walk has held together because the byte at
e is almost always the terminating NUL, which is ASCII and ends the run.
A string built by mrb_str_byte_subseq() is the exception: a substring too
long to embed shares the parent's buffer and carries a length of its own,
so the byte at its end is the parent's next byte, and a multi-byte one
carries the walk straight past the end of the string being measured.

s = ("あ" * 40)[0, 20]
s.bytesize                          # 60
s.length                            # 80     <- CRuby: 20
s[20]                               # "\xe3" <- CRuby: nil
("あ" * 40).byteslice(0, 59).length  # 82     <- CRuby: 21

Past e, mrb_utf8len() sees a negative e - p and returns 1 for every
byte, so each byte of the parent counts as one character until the walk
reaches the parent's terminator. Those bytes belong to the parent buffer,
which makes this a wrong answer rather than a fault, except over a buffer
that carries no terminator at all: str_init_nofree() keeps the caller's
pointer for a long mrb_str_new_static() string, and sym_intern() keeps
it for mrb_intern_static(), which Symbol#length then measures.

Two things hid it. A substring short enough to embed is copied and
terminated by str_init_embed(), so every short case answers correctly,
and the run stops at once when the parent's next byte is ASCII, so
("aあ" * 30)[0, 20].length is right while ("あ" * 40)[0, 20].length is
not. String#chars walks the bytes itself instead of asking this
function, so it kept answering 20 for the string whose length said 80.

The change

One bound test in the inner loop of mrb_utf8_strlen(), so the walk asks
p < e before reading the byte. The outer loop of the same function, and
the walks in chars2bytes() and bytes2chars(), already ask it at
exactly this point.

Nothing else changes. Counting a truncated sequence is untouched: a lead
byte with no room left still costs one, so a cut character answers as it
did, which is also what CRuby answers.

Related, not required

mrb_utf8len() reads only the top five bits of the lead byte, so it takes
an overlong sequence, a surrogate, or a value above U+10FFFF as a whole
character and counts one where CRuby counts a byte each. #7093 is about
that, and the two changes are independent: this one bounds the walk, that
one decides what a sequence inside the bound is worth. They touch
neighbouring functions in the same file and neither waits on the other.

Testing

rake test is green on full-core with MRB_UTF8_STRING, gcc on
x86_64-linux: 2231 asserts, no failures. mrb_utf8_strlen() sits inside
#ifdef MRB_UTF8_STRING, so a build without it is not compiled against
this code at all.

The new test fails without the change:

Fail: String#length(UTF-8) [15.2.10.5.26] (core)
 - Assertion[3]
    Expected: 20
      Actual: 80
 - Assertion[4]
    Expected "\xe3" to be nil.
 - Assertion[5]
    Expected: 20
      Actual: 50
 - Assertion[6]
    Expected: 10
      Actual: 50
 - Assertion[10]
    Expected: 21
      Actual: 82

It covers a shared substring starting at the parent's first byte and one
starting inside it, a four-byte character, and a substring cut in the
middle of a character. It also pins the three that were right all along,
so the paths this does not touch cannot move silently: a substring short
enough to embed, one reaching the parent's terminator, and one whose run
of non-ASCII bytes ends on an ASCII one. Checked against CRuby 4.0.6.

Summary by CodeRabbit

• Bug Fixes

  • Improved UTF-8 string length handling to avoid reading beyond the available data.
  • Corrected length calculations for embedded, shared, and truncated multibyte strings.

• Tests

  • Added coverage for UTF-8 character counting and incomplete multibyte characters.