Skip to content

fix: restore the character that terminates a number - #5344

Draft
nlohmann wants to merge 6 commits into
developfrom
claude/issue-5340-restore-unget
Draft

fix: restore the character that terminates a number#5344
nlohmann wants to merge 6 commits into
developfrom
claude/issue-5340-restore-unget

Conversation

@nlohmann

@nlohmann nlohmann commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Fixes #5340. Draft: this changes observable behavior of operator>> and sax_parse(strict = false), so it is a candidate for the next major release rather than a patch release. The version numbers in the docs assume 4.0.0 and need adjusting if that changes.

Note

Stacked on #5343 (the short-term documentation qualification), so that the docs diff here reads as "replace the caveat with the fixed behavior". GitHub will retarget this to develop once #5343 merges.

The bug

A number is the only JSON value whose end can only be detected by reading the character that follows it. lexer::scan_number() reads that character and calls lexer::unget(), but unget() is deliberately simulated — it rewinds only the lexer's own bookkeeping (chars_read_total, chars_read_current_line, token_string), not the input. input_stream_adapter::get_character() consumes via sbumpc() with no matching sungetc(), so the terminating character stays consumed:

std::istringstream input("1true");
json j1;
input >> j1;   // j1 == 1, but the stream now starts at "rue"

This is invisible to parse()/accept(), which require the input to end after the value anyway. It is only observable where the caller keeps using the input: operator>> and sax_parse(strict = false).

The fix

Propagating unget() straight through to the adapter does not work — next_unget makes the following get() replay the cached character, so the terminator would be delivered twice. The character has to be restored once, at the point where the input is handed back to the caller.

  • input_stream_adapter gains unget_character() (sungetc()) and advertises it via supports_unget, detected with the same is_detected tag-dispatch idiom already used for supports_seek. Other adapters compile to a no-op.
  • lexer::restore_pending_unget() turns a pending simulated unget of a real (non-EOF) character into a real one, and clears next_unget so the character is not also replayed. A pending unget of EOF is skipped — EOF was never consumed. This is the same pending_real_unget condition the lexer already computes in collect_token_chars.
  • parser calls it on the three non-strict paths (both parse() branches and sax_parse), folded into the existing if (strict && …) checks.

The invariant that makes this safe: get() clears next_unget whenever it consumes it, so a pending unget always refers to the most recent adapter read, and there is never more than one.

When sungetc() fails

sungetc() can fail if the streambuf has no putback position. Then the character stays consumed — which is exactly today's behavior, so a best-effort unget is never worse than the status quo. restore_pending_unget() reports this rather than asserting. Covered by a test using a streambuf whose pbackfail always fails.

Verification

input before after
1true rue true
1 true true true
1[2] 2] [2]
1{} } {}
1"a" a" "a"
-0.5e3x `` ❌ x

Note 1 true was wrong before too — the issue's table lists it as fine because the swallowed byte happened to be whitespace, but the position was still off by one.

  • New tests in tests/src/unit-deserialization.cpp (stream position after extraction (#5340)): number terminators for every following value type, self-delimiting values, a number at end of input, repeated extraction of 1true[2]3"x"{"a":4}5, sax_parse(strict = false), strict parsing still rejecting trailing data, and the failing-putback streambuf. 8 assertions in this section fail without the code change.
  • unit-class_parser, unit-class_lexer, unit-deserialization, unit-class_parser_diagnostic_positions, unit-user_defined_input, unit-regression2, unit-disabled_exceptions pass against both include/ and single_include/ — 20,398 assertions, 0 failures.
  • Parse error messages and reported positions were diffed across 13 malformed inputs (string and stream paths) and are byte-identical to develop.

Public API

No breaking changes to the public API surface: no signature, type, or name changes; nothing added to or removed from the public interface. lexer::restore_pending_unget() and input_stream_adapter::unget_character() are new members in detail::.

There is a behavior change, which is why this is a draft targeting the next major release:

  • operator>> and sax_parse(strict = false) on a std::istream now leave the stream one byte earlier when the parsed value was a number. Code that relied on the terminating byte being swallowed will observe it again.
  • Anyone who worked around the bug by inserting whitespace separators is unaffected — whitespace is still skipped on the next extraction.
  • parse(), accept(), and all non-stream inputs (strings, iterators, containers, FILE*) are unchanged.

Follow-up

docs/mkdocs/docs/api/operator_gtgt.md and sax_parse.md say "version 4.0.0"; these need updating if the change lands elsewhere. #5343's admonition is replaced here by the version note.


  • The changes are described in detail, both the what and why.
  • An existing issue is referenced.
  • All new code is covered by tests (new section in unit-deserialization.cpp; verified to fail without the fix).
  • The documentation is updated.
  • make amalgamate was run; single_include matches include (+96/−3, no unrelated reformatting).

This pull request was written by Claude Code.

operator>>'s notes state that it leaves the stream positioned right
after the parsed value, so that concatenated JSON values can be read
back to back. That does not hold when the value is a number: a number
is only terminated by the character that follows it, and the lexer's
unget() is simulated (it rewinds only the lexer's own bookkeeping),
so that character stays consumed from the stream.

Document the actual behaviour: the guarantee holds for all value types
except numbers, which must be followed by whitespace. Also qualify the
cross-reference on the JSON Lines page, which repeated the unqualified
claim.

Documentation only; the behaviour itself is tracked in #5340.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
operator>> is documented to leave the stream positioned right after the
parsed value, so that concatenated JSON values can be read back to back.
That did not hold for numbers: a number is only terminated by the
character following it, and lexer::scan_number() reads that character
and calls unget() -- which is simulated and rewinds only the lexer's own
bookkeeping. input_stream_adapter consumes via sbumpc() with no matching
sungetc(), so the terminating character stayed consumed and the next
extraction started one byte too late ('1true' left the stream at 'rue').

Propagating unget() to the adapter directly does not work: next_unget
makes the following get() replay the cached character, so the terminator
would be delivered twice. Instead, restore the still-pending character
once at the end of a non-strict parse, where the input is handed back to
the caller:

- input_stream_adapter gains unget_character() (sungetc()) and advertises
  it via supports_unget, detected the same way as supports_seek.
- lexer::restore_pending_unget() turns a pending simulated unget of a
  real (non-EOF) character into a real one and clears next_unget so the
  character is not also replayed. It is a no-op for adapters that cannot
  unget, and reports failure when sungetc() fails, in which case the
  input is left as it was before.
- parser calls it on the three non-strict paths, i.e. for operator>> and
  sax_parse(strict = false).

Strict parse()/accept() are unaffected: they require the input to end
after the value, so the character is consumed by the end-of-input check
anyway. Parse error messages and reported positions are unchanged.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@gregmarr

gregmarr commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

When sungetc() fails
sungetc() can fail if the streambuf has no putback position. Then the character stays consumed — which is exactly today's behavior, so a best-effort unget is never worse than the status quo. restore_pending_unget() reports this rather than asserting. Covered by a test using a streambuf whose pbackfail always fails.

Have you looked at using peek() to get the next character and then advancing the stream if appropriate, rather than getting the next character and having to do unget? I haven't looked to see if it's feasible, just curious if you already had done this.

Base automatically changed from claude/issue-5340-short-term-e64cc3 to develop August 3, 2026 06:18
Four CI failures, all in the new test code:

- GCC (-Werror=useless-cast): drop the `json(...)` wrapper around
  `json::parse(...)`, which already returns a `json`.
- GCC (-Werror=unused-result): assign the discarded `json::parse()`
  result to a dummy, the idiom used elsewhere in the test suite, and
  catch `json::parse_error&` for consistency.
- clang-tidy (google-default-arguments): remove the default argument
  from the `pbackfail()` override; `sungetc()` supplies the base
  declaration's default.
- MSVC (bad allocation): `no_putback_streambuf::underflow()` set a
  one-character get area without advancing `m_pos`, so an implementation
  whose `istream::get` peeks before it bumps re-read the same character
  forever. Keep no get area at all: `underflow()` peeks, `uflow()`
  consumes, and `sungetc()` still always lands in `pbackfail()`, which
  is what the test needs.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Read the character following a number without consuming it, instead of
consuming it and putting it back. input_stream_adapter now peeks with
sgetc() and only steps over the character when the next one is requested
or when the adapter is destroyed, so releasing it cannot fail - no
putback position is required from the streambuf.

Suggested by gregmarr in #5344.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
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.

operator>> does not restore the character that terminates a number

2 participants