From da7b9bdb3dc98d1f946d139ba3170bcfcbf0f83d Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 1 Aug 2026 07:33:11 +0200 Subject: [PATCH] fix: restore the character that terminates a number (#5340) 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 --- docs/mkdocs/docs/api/basic_json/sax_parse.md | 5 +- docs/mkdocs/docs/api/operator_gtgt.md | 46 ++--- .../docs/features/parsing/json_lines.md | 3 +- .../nlohmann/detail/input/input_adapters.hpp | 12 ++ include/nlohmann/detail/input/lexer.hpp | 64 +++++++ include/nlohmann/detail/input/parser.hpp | 23 ++- single_include/nlohmann/json.hpp | 99 +++++++++- tests/src/unit-deserialization.cpp | 172 ++++++++++++++++++ 8 files changed, 387 insertions(+), 37 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/sax_parse.md b/docs/mkdocs/docs/api/basic_json/sax_parse.md index fc8ce07b6..055990152 100644 --- a/docs/mkdocs/docs/api/basic_json/sax_parse.md +++ b/docs/mkdocs/docs/api/basic_json/sax_parse.md @@ -69,7 +69,8 @@ The SAX event lister must follow the interface of [`json_sax`](../json_sax/index [`input_format_t`](input_format_t.md) for more information `strict` (in) -: whether the input has to be consumed completely (optional, `#!cpp true` by default) +: whether the input has to be consumed completely (optional, `#!cpp true` by default); when `#!cpp false` and the + input is a `#!cpp std::istream`, the stream is left positioned right after the parsed value `ignore_comments` (in) : whether comments should be ignored and treated like whitespace (`#!cpp true`) or yield a parse error @@ -136,6 +137,8 @@ A UTF-8 byte order mark is silently ignored. - Added `ignore_trailing_commas` in version 3.13.0. - Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. - Extended overload (2) to accept heterogeneous iterator+sentinel pairs (C++20 ranges support) in version 3.13.0. +- Changed in version 4.0.0 to leave a `#!cpp std::istream` positioned right after the parsed value when `strict` is + `#!cpp false`; see [`operator>>`](../operator_gtgt.md#notes). !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/operator_gtgt.md b/docs/mkdocs/docs/api/operator_gtgt.md index a9ba17564..21f9903da 100644 --- a/docs/mkdocs/docs/api/operator_gtgt.md +++ b/docs/mkdocs/docs/api/operator_gtgt.md @@ -33,41 +33,29 @@ A UTF-8 byte order mark is silently ignored. Invalid Unicode escapes and unpaired surrogates in the input are reported as [`parse_error.101`](../home/exceptions.md#jsonexceptionparse_error101) with a detailed message. -`operator>>` parses exactly one JSON value, so it can be called repeatedly to read a sequence of concatenated JSON -values from the same stream: +`operator>>` parses exactly one JSON value and leaves the stream positioned right after it, so it can be called +repeatedly to read a sequence of concatenated JSON values from the same stream: ```cpp -json j1, j2; -input >> j1; // parses the first value -input >> j2; // parses the next value +std::istringstream input("1true[2]"); +json j1, j2, j3; +input >> j1; // j1 == 1, stream now positioned right after it +input >> j2; // j2 == true +input >> j3; // j3 == [2] ``` -!!! warning "A number must be followed by whitespace" +!!! note "Changed behavior for numbers" - A number is only terminated by the character that follows it. That character is read from the stream to detect the - end of the number, and it is **not** put back. When a value that is a number is immediately followed by the next - value, the first character of that next value is lost: + A number is the only value whose end can be detected solely by reading the character that follows it. Up to + version 3.13.0 that character was consumed and not put back, so the stream was left one byte too far whenever a + number was immediately followed by another value: reading `1true` yielded `1` and left the stream at `rue`. + Values had to be separated by whitespace to work around this. - ```cpp - std::istringstream input("1true"); - json j1, j2; - input >> j1; // j1 == 1 - input >> j2; // throws parse_error.101: the stream now starts at "rue" - ``` + The terminating character is now returned to the stream, so no separator is required. Code that relied on the + extra byte being swallowed will observe it again. - Separating the values with whitespace avoids this, because the character that is eaten is then the separator: - - ```cpp - std::istringstream input("1 true"); - json j1, j2; - input >> j1; // j1 == 1 - input >> j2; // j2 == true - ``` - - Only numbers are affected. Values ending in a self-delimiting character do not read past themselves, so - `truefalse`, `[1][2]`, `{"a":1}{"b":2}`, and `"a""b"` can be read back to back without a separator. - - This is tracked in [#5340](https://github.com/nlohmann/json/issues/5340). + If the stream's `#!cpp std::streambuf` cannot put the character back (its `pbackfail` fails, which does not happen + for `#!cpp std::stringbuf` or `#!cpp std::filebuf`), the character is lost as before. Note that reading concatenated values does **not** work for [JSON Lines](../features/parsing/json_lines.md) (newline-delimited JSON) input -- see that page for why and for the recommended alternative. @@ -102,3 +90,5 @@ Note that reading concatenated values does **not** work for [JSON Lines](../feat ## Version history - Added in version 1.0.0. +- Changed in version 4.0.0 to return the character that terminates a number to the stream, so that the stream is + positioned right after the parsed value for every value type. diff --git a/docs/mkdocs/docs/features/parsing/json_lines.md b/docs/mkdocs/docs/features/parsing/json_lines.md index fb1481819..cda6f37c7 100644 --- a/docs/mkdocs/docs/features/parsing/json_lines.md +++ b/docs/mkdocs/docs/features/parsing/json_lines.md @@ -49,5 +49,4 @@ JSON Lines input with more than one value is treated as invalid JSON by the [`pa with a JSON Lines input does not work, because the parser will try to parse one value after the last one. This is different from parsing a stream of *concatenated* (non-newline-delimited) JSON values, for which - `operator>>` does work, provided that a value that is a number is followed by whitespace -- see its - [notes](../../api/operator_gtgt.md#notes) for details. + `operator>>` does work -- see its [notes](../../api/operator_gtgt.md#notes) for details. diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 2c3561cbc..8a3bbf3f3 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -135,6 +135,18 @@ class input_stream_adapter return res; } + // Whether the adapter can return the last read character to the input so + // that subsequent reads from the underlying stream see it again. + static constexpr bool supports_unget = true; + + // Move the get pointer back over the character last returned by + // get_character(). Returns whether the character was actually restored; + // sungetc() may fail if the streambuf has no putback position available. + bool unget_character() + { + return sb->sungetc() != std::char_traits::eof(); + } + template std::size_t get_elements(T* dest, std::size_t count = 1) { diff --git a/include/nlohmann/detail/input/lexer.hpp b/include/nlohmann/detail/input/lexer.hpp index 6318994e6..31046eb22 100644 --- a/include/nlohmann/detail/input/lexer.hpp +++ b/include/nlohmann/detail/input/lexer.hpp @@ -125,6 +125,24 @@ constexpr bool input_adapter_supports_seek(std::false_type /*detected*/) return false; } +// Detect whether an input adapter can return the character last read to the +// input (see input_stream_adapter::supports_unget), detected like +// supports_seek above. +template +using detect_supports_unget = decltype(InputAdapterType::supports_unget); + +template +constexpr bool input_adapter_supports_unget(std::true_type /*detected*/) +{ + return InputAdapterType::supports_unget; +} + +template +constexpr bool input_adapter_supports_unget(std::false_type /*detected*/) +{ + return false; +} + /*! @brief lexical analysis @@ -146,6 +164,11 @@ class lexer : public lexer_base static constexpr bool lazy_token_string = input_adapter_supports_seek(is_detected {}); + /// whether a pending simulated unget can be turned into a real unget on + /// the input adapter; see input_adapter_supports_unget + static constexpr bool can_unget_input = + input_adapter_supports_unget(is_detected {}); + public: using token_type = typename lexer_base::token_type; @@ -1456,6 +1479,25 @@ scan_number_done: uncapture_char(std::integral_constant {}); } + /// adapter without unget support: nothing to do (see restore_pending_unget) + bool restore_pending_unget_impl(std::false_type /*can_unget*/) const noexcept + { + return false; + } + + /// adapter with unget support: give back the character consumed but unread + bool restore_pending_unget_impl(std::true_type /*can_unget*/) + { + if (!next_unget || current == char_traits::eof()) + { + // nothing was consumed beyond the last token + return true; + } + + next_unget = false; + return ia.unget_character(); + } + /// seekable adapter: nothing was captured, so nothing to undo void uncapture_char(std::true_type /*lazy*/) const noexcept {} @@ -1519,6 +1561,28 @@ scan_number_done: return position; } + /*! + @brief turn a pending simulated unget into a real one on the input + + unget() only rewinds the lexer's own bookkeeping, so the character that + terminated the last token (e.g. the character after a number) stays + consumed from the input. Callers that hand the input back to the user + afterwards - operator>> and non-strict sax_parse - call this once when + scanning is done, so that the input is positioned right after the value. + + A pending unget of EOF must not be restored: EOF was never consumed. The + lexer must not read again after this call; next_unget is cleared so that + the restored character is not also replayed from @a current. + + @return whether the input is positioned right after the last token; false + if the adapter cannot unget or the unget failed, in which case the + input is left as is (the pre-existing behaviour) + */ + bool restore_pending_unget() + { + return restore_pending_unget_impl(std::integral_constant {}); + } + /// seekable adapter: rebuild the last read token from the input on demand const std::vector& collect_token_chars(std::vector& out, std::true_type /*lazy*/) const { diff --git a/include/nlohmann/detail/input/parser.hpp b/include/nlohmann/detail/input/parser.hpp index ecc78eb72..9cfbdad7b 100644 --- a/include/nlohmann/detail/input/parser.hpp +++ b/include/nlohmann/detail/input/parser.hpp @@ -99,8 +99,14 @@ class parser json_sax_dom_callback_parser sdp(result, callback, allow_exceptions, &m_lexer); sax_parse_internal(&sdp); + if (!strict) + { + // the caller keeps using the input: position it right after + // the value by giving back the character that terminated it + m_lexer.restore_pending_unget(); + } // in strict mode, input must be completely read - if (strict && (get_token() != token_type::end_of_input)) + else if (get_token() != token_type::end_of_input) { sdp.parse_error(m_lexer.get_position(), m_lexer.get_token_string(), @@ -127,8 +133,13 @@ class parser json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); sax_parse_internal(&sdp); + if (!strict) + { + // see above + m_lexer.restore_pending_unget(); + } // in strict mode, input must be completely read - if (strict && (get_token() != token_type::end_of_input)) + else if (get_token() != token_type::end_of_input) { sdp.parse_error(m_lexer.get_position(), m_lexer.get_token_string(), @@ -165,8 +176,14 @@ class parser (void)detail::is_sax_static_asserts {}; const bool result = sax_parse_internal(sax); + if (result && !strict) + { + // the caller keeps using the input: position it right after the + // value by giving back the character that terminated it + m_lexer.restore_pending_unget(); + } // strict mode: next byte must be EOF - if (result && strict && (get_token() != token_type::end_of_input)) + else if (result && strict && (get_token() != token_type::end_of_input)) { return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string(), diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 124b5a9da..ae7b1c18c 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -7122,6 +7122,18 @@ class input_stream_adapter return res; } + // Whether the adapter can return the last read character to the input so + // that subsequent reads from the underlying stream see it again. + static constexpr bool supports_unget = true; + + // Move the get pointer back over the character last returned by + // get_character(). Returns whether the character was actually restored; + // sungetc() may fail if the streambuf has no putback position available. + bool unget_character() + { + return sb->sungetc() != std::char_traits::eof(); + } + template std::size_t get_elements(T* dest, std::size_t count = 1) { @@ -7823,6 +7835,24 @@ constexpr bool input_adapter_supports_seek(std::false_type /*detected*/) return false; } +// Detect whether an input adapter can return the character last read to the +// input (see input_stream_adapter::supports_unget), detected like +// supports_seek above. +template +using detect_supports_unget = decltype(InputAdapterType::supports_unget); + +template +constexpr bool input_adapter_supports_unget(std::true_type /*detected*/) +{ + return InputAdapterType::supports_unget; +} + +template +constexpr bool input_adapter_supports_unget(std::false_type /*detected*/) +{ + return false; +} + /*! @brief lexical analysis @@ -7844,6 +7874,11 @@ class lexer : public lexer_base static constexpr bool lazy_token_string = input_adapter_supports_seek(is_detected {}); + /// whether a pending simulated unget can be turned into a real unget on + /// the input adapter; see input_adapter_supports_unget + static constexpr bool can_unget_input = + input_adapter_supports_unget(is_detected {}); + public: using token_type = typename lexer_base::token_type; @@ -9154,6 +9189,25 @@ scan_number_done: uncapture_char(std::integral_constant {}); } + /// adapter without unget support: nothing to do (see restore_pending_unget) + bool restore_pending_unget_impl(std::false_type /*can_unget*/) const noexcept + { + return false; + } + + /// adapter with unget support: give back the character consumed but unread + bool restore_pending_unget_impl(std::true_type /*can_unget*/) + { + if (!next_unget || current == char_traits::eof()) + { + // nothing was consumed beyond the last token + return true; + } + + next_unget = false; + return ia.unget_character(); + } + /// seekable adapter: nothing was captured, so nothing to undo void uncapture_char(std::true_type /*lazy*/) const noexcept {} @@ -9217,6 +9271,28 @@ scan_number_done: return position; } + /*! + @brief turn a pending simulated unget into a real one on the input + + unget() only rewinds the lexer's own bookkeeping, so the character that + terminated the last token (e.g. the character after a number) stays + consumed from the input. Callers that hand the input back to the user + afterwards - operator>> and non-strict sax_parse - call this once when + scanning is done, so that the input is positioned right after the value. + + A pending unget of EOF must not be restored: EOF was never consumed. The + lexer must not read again after this call; next_unget is cleared so that + the restored character is not also replayed from @a current. + + @return whether the input is positioned right after the last token; false + if the adapter cannot unget or the unget failed, in which case the + input is left as is (the pre-existing behaviour) + */ + bool restore_pending_unget() + { + return restore_pending_unget_impl(std::integral_constant {}); + } + /// seekable adapter: rebuild the last read token from the input on demand const std::vector& collect_token_chars(std::vector& out, std::true_type /*lazy*/) const { @@ -13920,8 +13996,14 @@ class parser json_sax_dom_callback_parser sdp(result, callback, allow_exceptions, &m_lexer); sax_parse_internal(&sdp); + if (!strict) + { + // the caller keeps using the input: position it right after + // the value by giving back the character that terminated it + m_lexer.restore_pending_unget(); + } // in strict mode, input must be completely read - if (strict && (get_token() != token_type::end_of_input)) + else if (get_token() != token_type::end_of_input) { sdp.parse_error(m_lexer.get_position(), m_lexer.get_token_string(), @@ -13948,8 +14030,13 @@ class parser json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); sax_parse_internal(&sdp); + if (!strict) + { + // see above + m_lexer.restore_pending_unget(); + } // in strict mode, input must be completely read - if (strict && (get_token() != token_type::end_of_input)) + else if (get_token() != token_type::end_of_input) { sdp.parse_error(m_lexer.get_position(), m_lexer.get_token_string(), @@ -13986,8 +14073,14 @@ class parser (void)detail::is_sax_static_asserts {}; const bool result = sax_parse_internal(sax); + if (result && !strict) + { + // the caller keeps using the input: position it right after the + // value by giving back the character that terminated it + m_lexer.restore_pending_unget(); + } // strict mode: next byte must be EOF - if (result && strict && (get_token() != token_type::end_of_input)) + else if (result && strict && (get_token() != token_type::end_of_input)) { return sax->parse_error(m_lexer.get_position(), m_lexer.get_token_string(), diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index cedcb21a9..12c6e9551 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -14,10 +14,15 @@ using nlohmann::json; using namespace nlohmann::literals; // NOLINT(google-build-using-namespace) #endif +#include #include #include #include +#include +#include +#include #include +#include #if defined(_WIN32) #define NOMINMAX @@ -219,6 +224,58 @@ class proxy_iterator iterator* m_it = nullptr; }; +// A streambuf whose get area is a single character and that refuses every +// putback. Used to check that restoring the character that terminated a +// number degrades gracefully when the streambuf cannot put it back. +class no_putback_streambuf : public std::streambuf +{ + public: + explicit no_putback_streambuf(std::string s) : m_data(std::move(s)) {} + + protected: + int_type underflow() override + { + if (m_pos >= m_data.size()) + { + return traits_type::eof(); + } + m_char = m_data[m_pos]; + setg(&m_char, &m_char, &m_char + 1); + return traits_type::to_int_type(m_char); + } + + int_type uflow() override + { + if (m_pos >= m_data.size()) + { + return traits_type::eof(); + } + return traits_type::to_int_type(m_data[m_pos++]); + } + + int_type pbackfail(int_type /*c*/ = traits_type::eof()) override + { + return traits_type::eof(); + } + + private: + std::string m_data; + std::size_t m_pos = 0; + char m_char = 0; +}; + +// read the characters that are left in a stream +std::string remaining(std::istream& is) +{ + std::string result; + char c = 0; + while (is.get(c)) + { + result += c; + } + return result; +} + // JSON_HAS_CPP_20 #if defined(__cpp_char8_t) bool check_utf8() @@ -1157,6 +1214,121 @@ TEST_CASE("deserialization") } } + SECTION("stream position after extraction (#5340)") + { + SECTION("a number does not consume the character that terminates it") + { + // a number is only terminated by the character following it; that + // character must be given back so the stream is positioned right + // after the value + const std::vector> tests = + { + {"1true", "true"}, + {"1[2]", "[2]"}, + {"1{}", "{}"}, + {R"(1"a")", R"("a")"}, + {"1 true", " true"}, + {"12,", ","}, + {"-0.5e3x", "x"}, + {"1null", "null"} + }; + + for (const auto& test : tests) + { + CAPTURE(test.first); + std::istringstream ss(test.first); + json j; + ss >> j; + CHECK(j == json(json::parse(test.first.substr(0, test.first.size() - test.second.size())))); + CHECK(remaining(ss) == test.second); + } + } + + SECTION("values that are self-delimiting are unaffected") + { + const std::vector> tests = + { + {"truefalse", "false"}, + {"[1][2]", "[2]"}, + {R"({"a":1}{"b":2})", R"({"b":2})"}, + {R"("a""b")", R"("b")"}, + {"null null", " null"} + }; + + for (const auto& test : tests) + { + CAPTURE(test.first); + std::istringstream ss(test.first); + json j; + ss >> j; + CHECK(remaining(ss) == test.second); + } + } + + SECTION("a number at the end of the input leaves nothing behind") + { + for (const std::string s : + {"1", "12", "-3.5e2", " 7 " + }) + { + CAPTURE(s); + std::istringstream ss(s); + json j; + ss >> j; + CHECK(remaining(ss).find_first_not_of(" \t\n\r") == std::string::npos); + } + } + + SECTION("repeated extraction of concatenated values") + { + std::istringstream ss(R"(1true[2]3"x"{"a":4}5)"); + const std::vector expected = + { + json(1), json(true), json::parse("[2]"), json(3), + json("x"), json::parse(R"({"a":4})"), json(5) + }; + + for (const auto& e : expected) + { + json j; + ss >> j; + CHECK(j == e); + } + } + + SECTION("sax_parse with strict == false") + { + std::istringstream ss("1true"); + SaxEventLogger l; + CHECK(json::sax_parse(ss, &l, nlohmann::detail::input_format_t::json, false)); + CHECK(l.events.size() == 1); + CHECK(l.events[0] == "number_unsigned(1)"); + CHECK(remaining(ss) == "true"); + } + + SECTION("strict parsing still rejects trailing data") + { + std::istringstream ss("1true"); + CHECK_THROWS_WITH_AS(json::parse(ss), + "[json.exception.parse_error.101] parse error at line 1, column 5: syntax error while parsing value - unexpected true literal; expected end of input", json::parse_error); + + std::istringstream ss2("1true"); + CHECK_FALSE(json::accept(ss2)); + } + + SECTION("a streambuf that cannot put back degrades gracefully") + { + // the character is lost, as it was before the fix, but nothing + // else may break + no_putback_streambuf buf("1true"); + std::istream is(&buf); + json j; + is >> j; + CHECK(j == json(1)); + CHECK(remaining(is) == "rue"); + } + } + // build with C++20 // JSON_HAS_CPP_20 #if defined(__cpp_char8_t)