Fix partial string parse advancing past the end of the input

[hey-jj]

Fixes #267.

With with_allow_partial_strings, parsing a string that runs to the end of the input left current_index at data.len() + 1. A following slice_to_current then panics:

let data = br#"["foo"#; // 5 bytes, unterminated string
let mut jiter = jiter::Jiter::new(data).with_allow_partial_strings();
jiter.next_array().unwrap();
let start = jiter.current_index();
let s = jiter.next_str().unwrap();       // Ok("foo")
assert!(jiter.current_index() <= data.len()); // fails: 6 > 5
let _ = jiter.slice_to_current(start);   // panics: range end 6 out of range for len 5

Cause

In partial mode decode_string_chunk reports StringEnd at the end of the data when the closing quote is missing. All three decoder paths in string_decoder.rs then advanced index + 1 as if a quote were there: the direct data path in StringDecoder::decode, the tape path in decode_to_tape, and StringDecoderRange::decode.

Fix

A helper, index_after_string_end, steps over the quote only when index < data.len(). After a partial string parse the index now sits at data.len(), the same index the existing partial escape paths in decode_to_tape already return. When a real closing quote ends the string, index points at the quote, so index < data.len() always holds and the non-partial path is byte-for-byte unchanged.

With the fix, the reproducer prints current_index 5 and slice_to_current returns b"\"foo".

The regression test covers all three paths: next_str on a plain truncated string, next_str on a truncated string containing an escape, and next_bytes for the range decoder.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 71 untouched benchmarks

---

_{Comparing hey-jj:fix-partial-string-over-advance (ade20cd) with main (0fc00a4)}

[samuelcolvin]

looks good overall, but claude thinks we need some more test coverage:

⏺ Verified against the code rather than eyeballed. Verdict: they're genuine regression tests — each changed line is independently covered — but the inputs are narrow.

  What holds up

  I reverted string_decoder.rs to HEAD~1 and split the test's three sections into three separate #[test]s. All three fail without the fix:

  tmp_part_a (next_str, data path)   left: 6  right: 5
  tmp_part_b (next_str, tape path)   left: 10 right: 9
  tmp_part_c (next_bytes, range)     left: 6  right: 5

  So it isn't the common trap where the first assertion masks the rest — each of the three index_after_string_end call sites (string_decoder.rs:129, :189, :310) has a test
  that genuinely exercises it.

  I also probed the neighbouring partial-string exits against the fixed source — trailing backslash, truncated \u12/\u, truncated surrogate pairs, truncated UTF-8
  mid-codepoint, empty string at EOF, and the next_bytes equivalents. Every partial case lands at exactly data.len(), including the e.index early return from parse_escape.
  No remaining out-of-bounds path — the fix is complete, not just the three sites it touches.

  The gaps

  1. Everything is ASCII. All three strings are foo/bar/foo\nbar, so ascii_only is true throughout and the to_str non-ASCII branch never runs alongside the clamp. That
  branch is interesting here precisely because it truncates the returned &str back to a UTF-8 boundary while the index still advances to data.len() — so slice_to_current
  deliberately spans more bytes than the string you got back. Nothing asserts that relationship.

  2. Nothing reaches the SIMD loop. decode_string_chunk at simd/aarch64.rs:202 only enters its vector loop while data.get(index..index + SIMD_STEP) succeeds. The test
  strings are 3–7 bytes, so all three go straight to the fallback scanner. The clamp line is the same either way, so statement coverage is unaffected — but with the x86 port
  in flight, a >16-byte case is a free guard.

  3. Section 3 doesn't assert the reported symptom. Issue #267 was a slice_to_current panic; the next_bytes section only checks current_index(). One extra line.

  What I'd add

  The repo already has the right pattern for this bug class in test_value_partial_array_trailing_strings — truncate at every prefix length. Applied to Jiter (next_value
  won't work; it errors on a truncated array regardless of partial mode, so you have to walk it):

  #[test]
  fn jiter_partial_string_index_never_past_end() {
      let full = "[\"plain\", \"esc \\n \\u00e9 é tail\", \"ααα\", \
                  \"long enough to cross a simd chunk boundary\"]".as_bytes();
      for i in 1..=full.len() {
          let json = &full[..i];
          let mut jiter = Jiter::new(json).with_allow_partial_strings();
          let mut peek = jiter.next_array().ok().flatten();
          while peek.is_some() {
              let start = jiter.current_index();
              if jiter.next_str().is_err() {
                  break;
              }
              let index = jiter.current_index();
              assert!(index <= json.len(), "prefix {i}: index {index} > len {}", json.len());
              let _ = jiter.slice_to_current(start);
              peek = jiter.array_step().ok().flatten();
          }
      }
  }

  Fails at prefix 2: index 3 > len 2 on the unfixed source, passes on the fixed one, and folds gaps 1–3 into a single test — non-ASCII, escapes, and a SIMD-length string all
  get truncated at every byte offset. It's also the shape that would have caught the original bug without anyone thinking about it.

  Full suite is green with the fix (199 + 3 + 2 tests), and I've restored the working tree to HEAD.

  Want me to add the sweep test to the branch, alongside the min change?

LMK if you want me (or my claude) to add the tests, or if you're okay to.

[samuelcolvin]

Suggested change

	if index < data.len() { index + 1 } else { index }
	(index + 1).min(data.len())

is cleaner I think.