[sampokuokkanen]

The Java parser reads the source string's byte array directly, but it never marks the string as shared. So if a decimal_class callback calls source.tr!, it can rewrite the bytes the parser is still reading.

test_mutating_source_string_during_parsing only passes on JRuby by luck. If the string's coderange isn't known yet, JRuby's tr! builds a new buffer. If it is known, tr! writes in place. Calling source.ascii_only? before parsing is enough to break it:

require 'json'
expected = ([1] * 100) + [2.3] + ([1] * 100)
source = JSON.generate(expected)
source.ascii_only?
expected.delete_at(100)
k = Class.new
k.define_method(:initialize) { |n| source.tr!('1', '0'); n.to_f }
actual = JSON.parse(source, decimal_class: k)
actual.delete_at(100)
p actual == expected # MRI: true, JRuby: false (the last 100 elements are 0)

This came up in jruby/jruby#10000, which adds a faster String#tr. That makes tr! write in place much more often, so the existing test started failing there.

The fix freezes a copy of the source before parsing, the same way parser.c uses rb_str_new_frozen. RubyString#newFrozen shares the bytes and marks the original string as shared, so changing it later copies the bytes first. I added a test with ascii_only? that fails without the fix. The full test suite passes on JRuby master and on the jruby/jruby#10000 branch.

[headius]

This is correct and matches the changes (only done to the C extension) here:

5ca8a67

The Vsource == src check was later removed by @byroot in a round of "hardening against adversarial encode methods" with a test explicitly excluded on JRuby.

429c109

Now we match and should also be able to remove the exclusion of that test.

It might be worth checking for other guards against "adversarial" code that were never ported.

[headius]

These changes will need a backport to the 2.x-stable branch and likely a 2.x release to fix the Java extension along that line like the C extension was fixed in v2.19.7. Note that the "hardening" changes only appear to have been pushed to v3.1.0.

[headius]

@sampokuokkanen I pushed the approve button too quickly... could you also unmask the test I linked, so we are back to parity with the C extension?

[headius]

I filed #1123 to audit other excluded tests and try to bring the Java extension in line.

[headius]

FYI we can't land jruby/jruby#10000 for JRuby until this is patched because it now fails the test added in #985 due to the unported fix (and thus will start failing jruby-head builds here).

We need the backport because JRuby 10.0 and 10.1 must generally match CRuby 3.4 and 4.0 stdlib, neither of which ship json 3.x.

[headius]

I attempted to enable the masked test from #1107 but it fails with an EncodingError, possibly due to the C extensions lack of strictness in handling badly-encoded text (#138).

Error: test_adversary_encode(JSONParserTest): Encoding::InvalidByteSequenceError: "bl" on UTF-16
org/jruby/RubyString.java:6772:in 'encode'
json/ext/Parser.java:169:in 'parse'
/Users/headius/work/json/lib/json/ext.rb:27:in 'parse'
/Users/headius/work/json/lib/json/ext.rb:12:in 'parse'
/Users/headius/work/json/lib/json/common.rb:298:in 'parse'
/Users/headius/work/json/test/json/json_parser_test.rb:1115:in 'test_adversary_encode'
     1112:       end
     1113: 
     1114:       fake = fake_string.new("blah", encoding: Encoding::UTF_16)
  => 1115:       assert_equal 1000.times.to_a, JSON.parse(fake, on_load: ->(v) {
     1116:         if v == 20
     1117:           fake.encoded_str.clear
     1118:         end

We can merge this without enabling that test and examine it in more detail for #1123.