mirror of
https://github.com/ruby/ruby.git
synced 2026-10-29 00:46:26 +08:00
KBS
0f0d3172be
(https://github.com/ruby/stringio/pull/225) `Arrays.fill(byte[], int, int, byte)` takes **fromIndex, toIndex**, but `ungetbyteCommon` passes `memset`'s **destination, length**: ```java if (rest > cl) Arrays.fill(strBytes, len, rest - cl, (byte) 0); ``` straight from `ext/stringio/stringio.c:1181`, `memset(s + len, 0, rest - cl)`. It also drops the ByteList `begin` offset that the C `s +` supplies. Pushing back onto a StringIO positioned more than the pushback length past the end of its string then throws instead of zero-filling: ```ruby s = StringIO.new("abc"); s.pos = 5; s.ungetbyte("d") # JRuby: Java::JavaLang::IllegalArgumentException: fromIndex(3) > toIndex(1) # CRuby: s.string == "abc\0d", s.pos == 4 ``` The two siblings in this file already spell the correct form — `truncate` (`StringIO.java:1462`) uses `buf.getBegin() + plen, buf.getBegin() + l`, and `strioExtend` (`:1044-1048`) uses `begin + olen, begin + pos`. This one line is the odd one out, so the fix reuses their shape rather than adding anything. The added expectations extend the existing `test_ungetc_padding` / `test_ungetbyte_padding`, which stop one short: both start from `StringIO.new()`, so `len == 0` makes the wrong call accidentally correct. <details> <summary>Verification</summary> Both backends built from this clone, `$LOADED_FEATURES` printed and checked on every row. A differential sweep of 2100 cases (7 base strings x 25 positions x 6 pushback values x `ungetbyte`/`ungetc`), JRuby against the C extension: | build | rows differing from CRuby | |---|---| | current `master` | **132** | | with the fix | **0** | Every one of the 132 is the `IllegalArgumentException`. | row | result | |---|---| | the two padding tests on `master` | 2 errors | | with the fix | 2 tests, 12 assertions, 0 failures, 0 errors | | revert | 2 errors | | widen the guard to `rest > 0` | 1 error | | `fromIndex` one low | 1 failure, 1 error | | `toIndex` one high | passes — see below | That last mutant survives, and I checked whether it was a gap in my test or genuinely equivalent: the extra byte lands at `s + pos` after `pos -= cl`, which `System.arraycopy(..., strBytes, s + pos, cl)` two lines later overwrites unconditionally. Sweeping it over the same 2100 cases gives **0 differences from the fixed build**, so it is an equivalent mutant rather than an uncovered line, and I did not add a row for it. Full `test/stringio/test_stringio.rb`: CRuby 3.2 **104 tests, 646 assertions, 0 failures, 0 errors**; JRuby 10.0.6 **104 tests, 643 assertions, 0 failures, 0 errors**. Every added expectation was run against the C extension first, so it encodes CRuby behaviour rather than my reading of it. Two things I want on the record. The `s +` half is not separately covered — I could not construct a StringIO whose backing ByteList has `begin != 0` through the public API. It is included because both siblings and the C both require it; if you would rather keep the change minimal, the two-index correction alone fixes the crash. And `ruby/.github`'s SECURITY.md routes vulnerabilities privately: this is the JVM's own bounds check firing rather than memory unsafety, and I could not produce any stale-byte disclosure, so I filed publicly — say the word if you would rather it had gone the other route. The CI matrix runs `jruby-head`; the closest I could obtain is JRuby 10.0.6. Note `jruby:9.4` cannot compile `ext/java` at master at all (`Helpers.memchr` signature mismatch), so 10.x is already the floor. </details> --- Disclosure: I used Claude (an AI assistant) while preparing this change. Every result above I ran and verified myself. https://github.com/ruby/stringio/commit/9d3e502b3a