From 9e1a09eec0242380339b43691fa6f9174caf8ada Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 28 Sep 2026 17:51:07 +0200 Subject: [PATCH 1/3] Name the key type when rejecting non-string CBOR/MessagePack map keys (#5594) * Name the key type when rejecting non-string CBOR/MessagePack map keys CBOR and MessagePack allow map keys of any type, but JSON object keys are always strings, so such maps are rejected. The error so far was the one for a malformed string (e.g. "expected length specification (0xA0-0xBF, 0xD9-0xDB); last byte: 0xC0" for a nil key), which does not tell the user what went wrong. Report the type of the key instead: syntax error while parsing MessagePack object key: only string keys are supported, but found nil; last byte: 0xC0 The exception id (parse_error.113) and type are unchanged. Malformed string keys and a missing key keep their previous messages. Document the restriction on the CBOR and MessagePack pages. Refs #2766, #3381 Signed-off-by: Niels Lohmann * Point the MessagePack key note to the spec's profile section The note linked to "Serialization: type to format conversion", which says nothing about key types. Restricting map keys to strings is only mentioned in the "Profile" section (under "Future discussion") as an example of a JSON-compatible profile, so link there and describe it as such instead of as a permission. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/from_cbor.md | 4 +- .../docs/api/basic_json/from_msgpack.md | 4 +- .../docs/features/binary_formats/cbor.md | 15 +- .../features/binary_formats/messagepack.md | 15 ++ docs/mkdocs/docs/home/exceptions.md | 11 +- .../nlohmann/detail/input/binary_reader.hpp | 170 +++++++++++++++++- single_include/nlohmann/json.hpp | 170 +++++++++++++++++- tests/src/unit-cbor.cpp | 45 ++++- tests/src/unit-msgpack.cpp | 61 ++++++- tests/src/unit-regression1.cpp | 6 +- 10 files changed, 484 insertions(+), 17 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/from_cbor.md b/docs/mkdocs/docs/api/basic_json/from_cbor.md index b72f55280..8c1062da8 100644 --- a/docs/mkdocs/docs/api/basic_json/from_cbor.md +++ b/docs/mkdocs/docs/api/basic_json/from_cbor.md @@ -80,8 +80,8 @@ Strong guarantee: if an exception is thrown, there are no changes in the JSON va the end of the file was not reached when `strict` was set to true - Throws [parse_error.112](../../home/exceptions.md#jsonexceptionparse_error112) if unsupported features from CBOR were used in the given input or if the input is not valid CBOR -- Throws [parse_error.113](../../home/exceptions.md#jsonexceptionparse_error113) if a string was expected as a map key, - but not found +- Throws [parse_error.113](../../home/exceptions.md#jsonexceptionparse_error113) if a map key is not a string (keys of other + types are not supported, as JSON object keys are always strings) or a string is malformed ## Complexity diff --git a/docs/mkdocs/docs/api/basic_json/from_msgpack.md b/docs/mkdocs/docs/api/basic_json/from_msgpack.md index 2f4b7bb3b..e41edfe7e 100644 --- a/docs/mkdocs/docs/api/basic_json/from_msgpack.md +++ b/docs/mkdocs/docs/api/basic_json/from_msgpack.md @@ -73,8 +73,8 @@ Strong guarantee: if an exception is thrown, there are no changes in the JSON va the end of the file was not reached when `strict` was set to true - Throws [parse_error.112](../../home/exceptions.md#jsonexceptionparse_error112) if unsupported features from MessagePack were used in the given input or if the input is not valid MessagePack -- Throws [parse_error.113](../../home/exceptions.md#jsonexceptionparse_error113) if a string was expected as a map key, - but not found +- Throws [parse_error.113](../../home/exceptions.md#jsonexceptionparse_error113) if a map key is not a string (keys of other + types are not supported, as JSON object keys are always strings) or a string is malformed ## Complexity diff --git a/docs/mkdocs/docs/features/binary_formats/cbor.md b/docs/mkdocs/docs/features/binary_formats/cbor.md index 8e6acf0fb..a488466d4 100644 --- a/docs/mkdocs/docs/features/binary_formats/cbor.md +++ b/docs/mkdocs/docs/features/binary_formats/cbor.md @@ -174,7 +174,20 @@ The library maps CBOR types to JSON value types as follows: !!! warning "Object keys" - CBOR allows map keys of any type, whereas JSON only allows strings as keys in object values. Therefore, CBOR maps with keys other than UTF-8 strings are rejected. + CBOR allows map keys of any type, whereas JSON only allows strings as keys in object values. Therefore, CBOR maps + with keys other than text strings (major type 3) are rejected with a + [`parse_error.113`](../../home/exceptions.md#jsonexceptionparse_error113) exception (or, with `allow_exceptions` set + to `false`, a discarded value) naming the type of the key that was found, for instance: + + ``` + [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found an unsigned integer; last byte: 0x01 + ``` + + This applies to the [SAX interface](../parsing/sax_interface.md) as well, as the key is read before it is passed + on. This is a deliberate restriction of the library's JSON value model, not an oversight: formats built on CBOR + maps with integer keys, such as COSE ([RFC 9052](https://www.rfc-editor.org/rfc/rfc9052.html)) or CWT + ([RFC 8392](https://www.rfc-editor.org/rfc/rfc8392.html)), cannot be read with this library and need a + general-purpose CBOR library instead. !!! warning "UTF-8 validation of text strings" diff --git a/docs/mkdocs/docs/features/binary_formats/messagepack.md b/docs/mkdocs/docs/features/binary_formats/messagepack.md index 0ca82c145..3ce5f7620 100644 --- a/docs/mkdocs/docs/features/binary_formats/messagepack.md +++ b/docs/mkdocs/docs/features/binary_formats/messagepack.md @@ -138,6 +138,21 @@ The library maps MessagePack types to JSON value types as follows: Any MessagePack output created by `to_msgpack` can be successfully parsed by `from_msgpack`. +!!! warning "Object keys" + + MessagePack allows map keys of any type, whereas JSON only allows strings as keys in object values. Like the + JSON-compatible [profile](https://github.com/msgpack/msgpack/blob/master/spec.md#profile) sketched in the + MessagePack specification, this library restricts map keys to `str` values. Maps with keys of any other type are + rejected with a [`parse_error.113`](../../home/exceptions.md#jsonexceptionparse_error113) exception (or, with + `allow_exceptions` set to `false`, a discarded value) naming the type of the key that was found, for instance: + + ``` + [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack object key: only string keys are supported, but found nil; last byte: 0xC0 + ``` + + This applies to the [SAX interface](../parsing/sax_interface.md) as well, as the key is read before it is passed + on. Such input needs a general-purpose MessagePack library instead. + !!! warning "UTF-8 validation of string values" The MessagePack specification requires `str` values (`fixstr`, `str 8`, `str 16`, `str 32`) to be valid UTF-8. diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index bf18baab1..407f3c3f1 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -343,13 +343,20 @@ A string could not be read from a [binary format](../features/binary_formats/ind string was read where one was required (for instance as a map key), the string's length specification is invalid, or the string's bytes are not valid UTF-8. +CBOR and MessagePack allow map keys of any type, but JSON object keys are always strings. Maps with keys of any other +type (for instance integers or `null`) are therefore not supported; see the notes on +[CBOR](../features/binary_formats/cbor.md) and [MessagePack](../features/binary_formats/messagepack.md). + !!! failure "Example messages" ``` - [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xFF + [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found an unsigned integer; last byte: 0x01 ``` ``` - [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack string: expected length specification (0xA0-0xBF, 0xD9-0xDB); last byte: 0xFF + [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack object key: only string keys are supported, but found nil; last byte: 0xC0 + ``` + ``` + [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x7C ``` ``` [json.exception.parse_error.113] parse error at byte 2: syntax error while parsing UBJSON char: byte after 'C' must be in range 0x00..0x7F; last byte: 0x82 diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index b0675c626..b9e6b304b 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -1324,6 +1324,80 @@ class binary_reader } } + /*! + @brief reads a CBOR object key + + RFC 8949 allows any data item as a map key, but only strings have a + counterpart in JSON. A key of any other type is rejected with a message + naming that type, rather than the one @ref get_cbor_string gives for a + malformed string. + + @param[out] result created key + + @return whether key creation completed + */ + bool get_cbor_object_key(string_t& result) + { + // EOF and major type 3 (text string) are left to get_cbor_string + if (current == char_traits::eof() || (static_cast(current) & 0xE0u) == 0x60u) + { + return get_cbor_string(result); + } + + const char* found = nullptr; + switch (static_cast(current) >> 5u) + { + case 0: + found = "an unsigned integer"; + break; + case 1: + found = "a negative integer"; + break; + case 2: + found = "a byte string"; + break; + case 4: + found = "an array"; + break; + case 5: + found = "a map"; + break; + case 6: + found = "a tag"; + break; + default: // major type 7 + switch (current) + { + case 0xF4: + case 0xF5: + found = "a boolean"; + break; + case 0xF6: + found = "null"; + break; + case 0xF7: + found = "undefined"; + break; + case 0xF9: + case 0xFA: + case 0xFB: + found = "a floating-point number"; + break; + case 0xFF: + found = "a break stop code"; + break; + default: + found = "a simple value"; + break; + } + break; + } + + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(input_format_t::cbor, concat("only string keys are supported, but found ", found, "; last byte: 0x", last_token), "object key"), nullptr)); + } + /*! @brief reads a definite-length CBOR byte array @@ -1568,7 +1642,7 @@ class binary_reader if (top.is_object) { key.clear(); - if (JSON_HEDLEY_UNLIKELY(!get_cbor_string(key) || !sax->key(key))) + if (JSON_HEDLEY_UNLIKELY(!get_cbor_object_key(key) || !sax->key(key))) { return false; } @@ -2069,6 +2143,98 @@ class binary_reader } } + /*! + @brief reads a MessagePack object key + + The MessagePack specification allows any type as a map key, but only + strings have a counterpart in JSON. A key of any other type is rejected + with a message naming that type, rather than the one @ref + get_msgpack_string gives for a malformed string. + + @param[out] result created key + + @return whether key creation completed + */ + bool get_msgpack_object_key(string_t& result) + { + const char* found = nullptr; + switch (current) + { + case 0xC0: + found = "nil"; + break; + case 0xC2: + case 0xC3: + found = "a boolean"; + break; + case 0xCA: + case 0xCB: + found = "a float"; + break; + case 0xC4: + case 0xC5: + case 0xC6: + found = "a bin"; + break; + case 0xC7: + case 0xC8: + case 0xC9: + case 0xD4: + case 0xD5: + case 0xD6: + case 0xD7: + case 0xD8: + found = "an ext"; + break; + case 0xCC: + case 0xCD: + case 0xCE: + case 0xCF: + case 0xD0: + case 0xD1: + case 0xD2: + case 0xD3: + found = "an integer"; + break; + case 0xDC: + case 0xDD: + found = "an array"; + break; + case 0xDE: + case 0xDF: + found = "a map"; + break; + default: + // fixint, fixmap, and fixarray; strings, EOF, and the unused + // byte 0xC1 are left to get_msgpack_string + if (current == char_traits::eof()) + { + return get_msgpack_string(result); + } + if (current <= 0x7F || current >= 0xE0) + { + found = "an integer"; + } + else if (current <= 0x8F) + { + found = "a map"; + } + else if (current <= 0x9F) + { + found = "an array"; + } + else + { + return get_msgpack_string(result); + } + break; + } + + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(input_format_t::msgpack, concat("only string keys are supported, but found ", found, "; last byte: 0x", last_token), "object key"), nullptr)); + } + /*! @brief reads a MessagePack byte array @@ -2231,7 +2397,7 @@ class binary_reader { get(); key.clear(); - if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + if (JSON_HEDLEY_UNLIKELY(!get_msgpack_object_key(key) || !sax->key(key))) { return false; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 0e3cae486..591a00b74 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -14059,6 +14059,80 @@ class binary_reader } } + /*! + @brief reads a CBOR object key + + RFC 8949 allows any data item as a map key, but only strings have a + counterpart in JSON. A key of any other type is rejected with a message + naming that type, rather than the one @ref get_cbor_string gives for a + malformed string. + + @param[out] result created key + + @return whether key creation completed + */ + bool get_cbor_object_key(string_t& result) + { + // EOF and major type 3 (text string) are left to get_cbor_string + if (current == char_traits::eof() || (static_cast(current) & 0xE0u) == 0x60u) + { + return get_cbor_string(result); + } + + const char* found = nullptr; + switch (static_cast(current) >> 5u) + { + case 0: + found = "an unsigned integer"; + break; + case 1: + found = "a negative integer"; + break; + case 2: + found = "a byte string"; + break; + case 4: + found = "an array"; + break; + case 5: + found = "a map"; + break; + case 6: + found = "a tag"; + break; + default: // major type 7 + switch (current) + { + case 0xF4: + case 0xF5: + found = "a boolean"; + break; + case 0xF6: + found = "null"; + break; + case 0xF7: + found = "undefined"; + break; + case 0xF9: + case 0xFA: + case 0xFB: + found = "a floating-point number"; + break; + case 0xFF: + found = "a break stop code"; + break; + default: + found = "a simple value"; + break; + } + break; + } + + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(input_format_t::cbor, concat("only string keys are supported, but found ", found, "; last byte: 0x", last_token), "object key"), nullptr)); + } + /*! @brief reads a definite-length CBOR byte array @@ -14303,7 +14377,7 @@ class binary_reader if (top.is_object) { key.clear(); - if (JSON_HEDLEY_UNLIKELY(!get_cbor_string(key) || !sax->key(key))) + if (JSON_HEDLEY_UNLIKELY(!get_cbor_object_key(key) || !sax->key(key))) { return false; } @@ -14804,6 +14878,98 @@ class binary_reader } } + /*! + @brief reads a MessagePack object key + + The MessagePack specification allows any type as a map key, but only + strings have a counterpart in JSON. A key of any other type is rejected + with a message naming that type, rather than the one @ref + get_msgpack_string gives for a malformed string. + + @param[out] result created key + + @return whether key creation completed + */ + bool get_msgpack_object_key(string_t& result) + { + const char* found = nullptr; + switch (current) + { + case 0xC0: + found = "nil"; + break; + case 0xC2: + case 0xC3: + found = "a boolean"; + break; + case 0xCA: + case 0xCB: + found = "a float"; + break; + case 0xC4: + case 0xC5: + case 0xC6: + found = "a bin"; + break; + case 0xC7: + case 0xC8: + case 0xC9: + case 0xD4: + case 0xD5: + case 0xD6: + case 0xD7: + case 0xD8: + found = "an ext"; + break; + case 0xCC: + case 0xCD: + case 0xCE: + case 0xCF: + case 0xD0: + case 0xD1: + case 0xD2: + case 0xD3: + found = "an integer"; + break; + case 0xDC: + case 0xDD: + found = "an array"; + break; + case 0xDE: + case 0xDF: + found = "a map"; + break; + default: + // fixint, fixmap, and fixarray; strings, EOF, and the unused + // byte 0xC1 are left to get_msgpack_string + if (current == char_traits::eof()) + { + return get_msgpack_string(result); + } + if (current <= 0x7F || current >= 0xE0) + { + found = "an integer"; + } + else if (current <= 0x8F) + { + found = "a map"; + } + else if (current <= 0x9F) + { + found = "an array"; + } + else + { + return get_msgpack_string(result); + } + break; + } + + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(input_format_t::msgpack, concat("only string keys are supported, but found ", found, "; last byte: 0x", last_token), "object key"), nullptr)); + } + /*! @brief reads a MessagePack byte array @@ -14966,7 +15132,7 @@ class binary_reader { get(); key.clear(); - if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + if (JSON_HEDLEY_UNLIKELY(!get_msgpack_object_key(key) || !sax->key(key))) { return false; } diff --git a/tests/src/unit-cbor.cpp b/tests/src/unit-cbor.cpp index 6bd792f8a..fe0fb2644 100644 --- a/tests/src/unit-cbor.cpp +++ b/tests/src/unit-cbor.cpp @@ -1830,10 +1830,51 @@ TEST_CASE("CBOR") SECTION("invalid string in map") { json _; - CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xa1, 0xff, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xFF", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xa1, 0xff, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found a break stop code; last byte: 0xFF", json::parse_error&); CHECK(json::from_cbor(std::vector({0xa1, 0xff, 0x01}), true, false).is_discarded()); } + SECTION("non-string key (see #2766 and #3381)") + { + // only text strings map to JSON object keys; any other key is + // rejected with a message naming its type + const std::vector, std::string>> cases = + { + {{0xA1, 0x01, 0x01}, "an unsigned integer; last byte: 0x01"}, + {{0xA1, 0x20, 0x01}, "a negative integer; last byte: 0x20"}, + {{0xA1, 0x41, 0x61, 0x01}, "a byte string; last byte: 0x41"}, + {{0xA1, 0x80, 0x01}, "an array; last byte: 0x80"}, + {{0xA1, 0xA0, 0x01}, "a map; last byte: 0xA0"}, + {{0xA1, 0xC0, 0x61, 0x61, 0x01}, "a tag; last byte: 0xC0"}, + {{0xA1, 0xF4, 0x01}, "a boolean; last byte: 0xF4"}, + {{0xA1, 0xF5, 0x01}, "a boolean; last byte: 0xF5"}, + {{0xA1, 0xF6, 0x01}, "null; last byte: 0xF6"}, + {{0xA1, 0xF7, 0x01}, "undefined; last byte: 0xF7"}, + {{0xA1, 0xF9, 0x3C, 0x00, 0x01}, "a floating-point number; last byte: 0xF9"}, + {{0xA1, 0xFA, 0x3F, 0x80, 0x00, 0x00, 0x01}, "a floating-point number; last byte: 0xFA"}, + {{0xA1, 0xFB, 0x3F, 0xF0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, "a floating-point number; last byte: 0xFB"}, + {{0xA1, 0xE0, 0x01}, "a simple value; last byte: 0xE0"}, + {{0xA1, 0xF8, 0x20, 0x01}, "a simple value; last byte: 0xF8"}, + // indefinite-length map + {{0xBF, 0x01, 0x01, 0xFF}, "an unsigned integer; last byte: 0x01"}, + }; + + for (const auto& c : cases) + { + CAPTURE(c.first) + const std::string expected = "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found " + c.second; + json _; + CHECK_THROWS_WITH_AS(_ = json::from_cbor(c.first), expected.c_str(), json::parse_error&); + CHECK(json::from_cbor(c.first, true, false).is_discarded()); + } + + // a key of major type 3 with a reserved length is still reported as + // a malformed string, and a missing key as the end of input + json _; + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1})), "[json.exception.parse_error.110] parse error at byte 2: syntax error while parsing CBOR string: unexpected end of input", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0x7C, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x7C", json::parse_error&); + } + SECTION("invalid UTF-8 in string (see #5529)") { // a two-character text string (major type 3) whose bytes are not @@ -2284,7 +2325,7 @@ TEST_CASE("CBOR indefinite-length strings do not recurse per chunk") SECTION("a break marker outside an indefinite-length string is not a string") { // 0xFF only closes a string that was opened; on its own it is not one - CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0xFF, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xFF", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0xFF, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found a break stop code; last byte: 0xFF", json::parse_error&); } } diff --git a/tests/src/unit-msgpack.cpp b/tests/src/unit-msgpack.cpp index de4255b4a..498dec859 100644 --- a/tests/src/unit-msgpack.cpp +++ b/tests/src/unit-msgpack.cpp @@ -1551,10 +1551,69 @@ TEST_CASE("MessagePack") SECTION("invalid string in map") { json _; - CHECK_THROWS_WITH_AS(_ = json::from_msgpack(std::vector({0x81, 0xff, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack string: expected length specification (0xA0-0xBF, 0xD9-0xDB); last byte: 0xFF", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_msgpack(std::vector({0x81, 0xff, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack object key: only string keys are supported, but found an integer; last byte: 0xFF", json::parse_error&); CHECK(json::from_msgpack(std::vector({0x81, 0xff, 0x01}), true, false).is_discarded()); } + SECTION("non-string key (see #3381)") + { + // only strings map to JSON object keys; any other key is rejected + // with a message naming its type + const std::vector, std::string>> cases = + { + {{0x81, 0xC0, 0x01}, "nil; last byte: 0xC0"}, + {{0x81, 0xC2, 0x01}, "a boolean; last byte: 0xC2"}, + {{0x81, 0xC3, 0x01}, "a boolean; last byte: 0xC3"}, + {{0x81, 0xCA, 0x3F, 0x80, 0x00, 0x00, 0x01}, "a float; last byte: 0xCA"}, + {{0x81, 0xCB, 0x3F, 0xF0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, "a float; last byte: 0xCB"}, + {{0x81, 0xC4, 0x00, 0x01}, "a bin; last byte: 0xC4"}, + {{0x81, 0xC5, 0x00, 0x00, 0x01}, "a bin; last byte: 0xC5"}, + {{0x81, 0xC6, 0x00, 0x00, 0x00, 0x00, 0x01}, "a bin; last byte: 0xC6"}, + {{0x81, 0xC7, 0x00, 0x01, 0x01}, "an ext; last byte: 0xC7"}, + {{0x81, 0xC8, 0x00, 0x00, 0x01, 0x01}, "an ext; last byte: 0xC8"}, + {{0x81, 0xC9, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}, "an ext; last byte: 0xC9"}, + {{0x81, 0xD4, 0x01, 0x00, 0x01}, "an ext; last byte: 0xD4"}, + {{0x81, 0xD5, 0x01, 0x00, 0x00, 0x01}, "an ext; last byte: 0xD5"}, + {{0x81, 0xD6, 0x01, 0x00, 0x00, 0x00, 0x00, 0x01}, "an ext; last byte: 0xD6"}, + {{0x81, 0xD7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, "an ext; last byte: 0xD7"}, + {{0x81, 0xD8, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, "an ext; last byte: 0xD8"}, + {{0x81, 0xCC, 0x01, 0x01}, "an integer; last byte: 0xCC"}, + {{0x81, 0xCD, 0x00, 0x01, 0x01}, "an integer; last byte: 0xCD"}, + {{0x81, 0xCE, 0x00, 0x00, 0x00, 0x01, 0x01}, "an integer; last byte: 0xCE"}, + {{0x81, 0xCF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}, "an integer; last byte: 0xCF"}, + {{0x81, 0xD0, 0x01, 0x01}, "an integer; last byte: 0xD0"}, + {{0x81, 0xD1, 0x00, 0x01, 0x01}, "an integer; last byte: 0xD1"}, + {{0x81, 0xD2, 0x00, 0x00, 0x00, 0x01, 0x01}, "an integer; last byte: 0xD2"}, + {{0x81, 0xD3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01}, "an integer; last byte: 0xD3"}, + {{0x81, 0x00, 0x01}, "an integer; last byte: 0x00"}, + {{0x81, 0x7F, 0x01}, "an integer; last byte: 0x7F"}, + {{0x81, 0xE0, 0x01}, "an integer; last byte: 0xE0"}, + {{0x81, 0x80, 0x01}, "a map; last byte: 0x80"}, + {{0x81, 0x8F, 0x01}, "a map; last byte: 0x8F"}, + {{0x81, 0xDE, 0x00, 0x00, 0x01}, "a map; last byte: 0xDE"}, + {{0x81, 0xDF, 0x00, 0x00, 0x00, 0x00, 0x01}, "a map; last byte: 0xDF"}, + {{0x81, 0x90, 0x01}, "an array; last byte: 0x90"}, + {{0x81, 0x9F, 0x01}, "an array; last byte: 0x9F"}, + {{0x81, 0xDC, 0x00, 0x00, 0x01}, "an array; last byte: 0xDC"}, + {{0x81, 0xDD, 0x00, 0x00, 0x00, 0x00, 0x01}, "an array; last byte: 0xDD"}, + }; + + for (const auto& c : cases) + { + CAPTURE(c.first) + const std::string expected = "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack object key: only string keys are supported, but found " + c.second; + json _; + CHECK_THROWS_WITH_AS(_ = json::from_msgpack(c.first), expected.c_str(), json::parse_error&); + CHECK(json::from_msgpack(c.first, true, false).is_discarded()); + } + + json _; + // the unused byte 0xC1 is still reported as a malformed string + CHECK_THROWS_WITH_AS(_ = json::from_msgpack(std::vector({0x81, 0xC1, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing MessagePack string: expected length specification (0xA0-0xBF, 0xD9-0xDB); last byte: 0xC1", json::parse_error&); + // a missing key is still reported as the end of input + CHECK_THROWS_WITH_AS(_ = json::from_msgpack(std::vector({0x81})), "[json.exception.parse_error.110] parse error at byte 2: syntax error while parsing MessagePack string: unexpected end of input", json::parse_error&); + } + SECTION("invalid UTF-8 in string (see #5529)") { // a fixstr of length 2 (0xA0 | 2) whose bytes are not valid UTF-8 diff --git a/tests/src/unit-regression1.cpp b/tests/src/unit-regression1.cpp index 0529f83dd..43cd18438 100644 --- a/tests/src/unit-regression1.cpp +++ b/tests/src/unit-regression1.cpp @@ -1018,7 +1018,7 @@ TEST_CASE("regression tests 1") }; json _; - CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x98", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR object key: only string keys are supported, but found an array; last byte: 0x98", json::parse_error&); // related test case: nonempty UTF-8 string (indefinite length) std::vector const vec1 {0x7f, 0x61, 0x61}; @@ -1065,7 +1065,7 @@ TEST_CASE("regression tests 1") }; json _; - CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec1), "[json.exception.parse_error.113] parse error at byte 13: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xB4", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec1), "[json.exception.parse_error.113] parse error at byte 13: syntax error while parsing CBOR object key: only string keys are supported, but found a map; last byte: 0xB4", json::parse_error&); // related test case: double-precision std::vector const vec2 @@ -1077,7 +1077,7 @@ TEST_CASE("regression tests 1") 0x96, 0x96, 0xb4, 0xb4, 0xfa, 0x94, 0x94, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0xfb }; - CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec2), "[json.exception.parse_error.113] parse error at byte 13: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xB4", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(vec2), "[json.exception.parse_error.113] parse error at byte 13: syntax error while parsing CBOR object key: only string keys are supported, but found a map; last byte: 0xB4", json::parse_error&); } SECTION("issue #452 - Heap-buffer-overflow (OSS-Fuzz issue 585)") From fc03b9912eab296efbfc31f0b7da5568b7c9bb53 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 28 Sep 2026 17:56:11 +0200 Subject: [PATCH 2/3] Look up the locale decimal point at conversion time, not lexer construction (#5597) * Look up the locale decimal point at conversion time, not lexer construction The lexer read localeconv()->decimal_point once in its constructor and wrote that character into token_buffer in place of '.'. The strtod fallback then used the locale current at conversion time, so an LC_NUMERIC change in between (parser callback, SAX handler, another thread) truncated the value in release builds and fired the endptr assertion in debug builds. token_buffer now always holds '.'. Only the strtof/strtod/strtold fallback depends on the locale: it looks up the decimal point right before the call, restores '.' afterwards, and repeats the conversion if the locale changed in between. As a side effect, std::from_chars and Clinger's fast path now also apply under locales whose decimal point is not '.'. Fixes #5198 Signed-off-by: Niels Lohmann * Stop the strtod retry loop when the decimal point is unchanged convert_float_locale_aware() repeated the conversion until strtod consumed the whole token, assuming an early stop can only mean a locale change. Under a locale whose decimal point is not a single character (e.g. the two-byte U+066B of ar_EG.UTF-8, ar_SA.UTF-8, or fa_IR.UTF-8, all available on macOS), the in-place substitution can never succeed, so parsing any float that reaches the strtod fallback (for example 3.14159265358979323846 at C++11) hung forever. Before this branch, the same input was truncated. Retry only if the decimal point changed since the previous attempt; otherwise keep the value strtod parsed so far, as before. Add a test that parses such numbers under a multi-byte decimal point locale; it hangs without this change. Signed-off-by: Niels Lohmann * Fix -Weffc++ errors in the #5198 locale test GCC's -Weffc++ (an error in ci_test_gcc and ci_test_standards_gcc) rejected LocaleSwitchingSax: it has a pointer data member but does not declare its copy operations, and its vectors are not initialized in the member initializer list. Store the locale name as a std::string and give the vectors brace initializers, like SaxEventLogger in unit-deserialization.cpp. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- include/nlohmann/detail/input/lexer.hpp | 115 ++++++---- .../nlohmann/detail/input/number_parse.hpp | 21 +- single_include/nlohmann/json.hpp | 136 +++++++----- tests/src/unit-class_lexer.cpp | 2 +- tests/src/unit-locale-cpp.cpp | 210 ++++++++++++++++++ 5 files changed, 379 insertions(+), 105 deletions(-) diff --git a/include/nlohmann/detail/input/lexer.hpp b/include/nlohmann/detail/input/lexer.hpp index 98c0fd76a..00a964a17 100644 --- a/include/nlohmann/detail/input/lexer.hpp +++ b/include/nlohmann/detail/input/lexer.hpp @@ -206,7 +206,6 @@ class lexer : public lexer_base explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_number_values_ = false) noexcept : ia(std::move(adapter)) , ignore_comments(ignore_comments_) - , decimal_point_char(static_cast(get_decimal_point())) , discard_number_values(discard_number_values_) {} @@ -222,8 +221,7 @@ class lexer : public lexer_base // locales ///////////////////// - /// return the locale-dependent decimal point - JSON_HEDLEY_PURE + /// return the decimal point of the current locale static char get_decimal_point() noexcept { const auto* loc = localeconv(); @@ -1092,9 +1090,10 @@ class lexer : public lexer_base token_type::value_float if number could be successfully scanned, token_type::parse_error otherwise - @note The scanner is independent of the current locale. Internally, the - locale's decimal point is used instead of `.` to work with the - locale-dependent converters. + @note The scanner is independent of the current locale: token_buffer + always holds `.`. Only the std::strtod fallback of convert_number() + depends on the locale, and it looks up the decimal point right + before converting (see convert_float_locale_aware()). */ token_type scan_number() // lgtm [cpp/use-of-goto] `goto` is used in this function to implement the number-parsing state machine described above. By design, any finite input will eventually reach the "done" state or return token_type::parse_error. In each intermediate state, 1 byte of the input is appended to the token_buffer vector, and only the already initialized variables token_buffer, number_type, and error_message are manipulated. { @@ -1183,7 +1182,7 @@ scan_number_zero: { case '.': { - add(decimal_point_char); + add(current); decimal_point_position = token_buffer.size() - 1; goto scan_number_decimal1; } @@ -1220,7 +1219,7 @@ scan_number_any1: case '.': { - add(decimal_point_char); + add(current); decimal_point_position = token_buffer.size() - 1; goto scan_number_decimal1; } @@ -1462,9 +1461,9 @@ scan_number_done: // Only a number below 1 can carry further insignificant zeros, and only // while the count stays at the limit does removing them change the - // answer - so this loop is skipped for all but a few tokens. Note - // token_buffer holds the locale's decimal point, so the fraction is - // located through decimal_point_position rather than by searching '.'. + // answer - so this loop is skipped for all but a few tokens. The + // fraction is located through decimal_point_position rather than by + // searching '.'. if (lead_zero != 0) { JSON_ASSERT(has_dot != 0); // an integer "0" cannot reach the limit @@ -1482,8 +1481,8 @@ scan_number_done: @brief convert the number text in token_buffer to its value and token type The digit sequence in token_buffer has already been validated (by the - scan_number() state machine or by the contiguous fast path) and holds the - locale decimal point in place of '.'. Integers are parsed first and fall + scan_number() state machine or by the contiguous fast path) and holds '.' + as decimal point, independent of the locale. Integers are parsed first and fall back to floating point on overflow. This is shared so both scanners produce identical results. @@ -1563,7 +1562,7 @@ scan_number_done: // integer conversion above overflowed. Prefer std::from_chars // (Eisel-Lemire, locale-independent, correctly rounded) when available; // otherwise the exact Clinger fast path (double only); otherwise the - // locale-aware strtof/strtod. + // locale-aware strtof/strtod/strtold. if (parse_float_from_chars(num_begin, num_end, value_float)) { return token_type::value_float; @@ -1572,26 +1571,75 @@ scan_number_done: // extra pass over the token's bytes, which otherwise shows up on // high-precision inputs such as canada.json if (mantissa_fits_clinger(mantissa_end) - && parse_float_fast(num_begin, num_end, decimal_point_char, value_float)) + && parse_float_fast(num_begin, num_end, value_float)) { return token_type::value_float; } - char* endptr = nullptr; // NOLINT(misc-const-correctness,cppcoreguidelines-pro-type-vararg,hicpp-vararg) - strtof(value_float, token_buffer.data(), &endptr); - - // we checked the number format before - JSON_ASSERT(endptr == token_buffer.data() + token_buffer.size()); - + convert_float_locale_aware(); return token_type::value_float; } + /*! + @brief convert the float in token_buffer with strtof/strtod/strtold + + These functions expect the decimal point of the *current* locale, so it is + looked up right before the conversion instead of once when the lexer is + constructed: a locale change in between (by a parser callback, a SAX + handler, or another thread) must not truncate the value (#5198). The + token has been validated before, so if the conversion stops early and the + decimal point changed in the meantime, the locale changed between the + lookup and the call, and the conversion is repeated with the new decimal + point. If the decimal point did not change, a retry cannot succeed: the + locale's decimal point is not a single character (e.g., the two-byte + U+066B of ar_EG.UTF-8 or fa_IR.UTF-8) and cannot be substituted in place. + The value strtod parsed up to that point is kept, as before this change. + + Note that changing the locale in another thread *while* strtod runs is + undefined behavior of the C library, which this function cannot prevent. + */ + void convert_float_locale_aware() + { + const bool has_dot = decimal_point_position != std::string::npos; + char decimal_point = get_decimal_point(); + for (;;) + { + const bool substitute = has_dot && decimal_point != '.'; + if (substitute) + { + token_buffer[decimal_point_position] = static_cast(decimal_point); + } + + char* endptr = nullptr; // NOLINT(misc-const-correctness,cppcoreguidelines-pro-type-vararg,hicpp-vararg) + strtof(value_float, token_buffer.data(), &endptr); + + if (substitute) + { + // get_string() hands the token to the SAX interface with '.' + token_buffer[decimal_point_position] = '.'; + } + + if (JSON_HEDLEY_LIKELY(endptr == token_buffer.data() + token_buffer.size())) + { + return; + } + + // retry only if the locale changed; otherwise, this would loop forever + const char current_decimal_point = get_decimal_point(); + if (current_decimal_point == decimal_point) + { + return; + } + decimal_point = current_decimal_point; + } + } + /*! @brief contiguous fast path for scanning a number Parses the whole number token straight from the input buffer, avoiding the per-character get()/add() of scan_number(). On success it fills token_buffer - (with the locale decimal point substituted, as scan_number() does) and + (as scan_number() does) and returns the token type. On anything it does not fully recognize as a well-formed number it makes no state change and returns token_type::uninitialized, so the caller falls back to scan_number(), which @@ -1707,16 +1755,11 @@ scan_number_done: } #endif - // materialize the token exactly as scan_number() would, substituting the - // locale decimal point so convert_number()'s strtof fallback stays valid. - // reset() already cleared token_buffer, so append() fills it (assign() is - // avoided because custom string_t types need not provide it) + // materialize the token exactly as scan_number() would. reset() already + // cleared token_buffer, so append() fills it (assign() is avoided + // because custom string_t types need not provide it) token_buffer.append(reinterpret_cast(data), len); - if (dot_index != std::string::npos) - { - token_buffer[dot_index] = static_cast(decimal_point_char); - decimal_point_position = dot_index; - } + decimal_point_position = dot_index; ia.bulk_skip(len - 1); position.chars_read_total += (len - 1); @@ -1983,11 +2026,7 @@ scan_number_done: /// return current string value (implicitly resets the token; useful only once) string_t& get_string() { - // translate decimal points from locale back to '.' (#4084) - if (decimal_point_char != '.' && decimal_point_position != std::string::npos) - { - token_buffer[decimal_point_position] = '.'; - } + // a number token holds '.' regardless of the locale (#4084) return token_buffer; } @@ -2283,9 +2322,7 @@ scan_number_done: number_unsigned_t value_unsigned = 0; number_float_t value_float = 0; - /// the decimal point - const char_int_type decimal_point_char = '.'; - /// the position of the decimal point in the input + /// the position of the decimal point in token_buffer std::size_t decimal_point_position = std::string::npos; /// whether the caller (e.g. accept()/json_sax_acceptor) only needs the diff --git a/include/nlohmann/detail/input/number_parse.hpp b/include/nlohmann/detail/input/number_parse.hpp index e50c3f67f..25f6cac91 100644 --- a/include/nlohmann/detail/input/number_parse.hpp +++ b/include/nlohmann/detail/input/number_parse.hpp @@ -118,14 +118,12 @@ std::strtod. The parser only activates for number_float_t == double; float and long double keep the std::strtof/std::strtold paths (see the templated overload below). -@param[in] first pointer to the first character of the number -@param[in] last pointer past the last character -@param[in] decimal_point the (locale-dependent) decimal point character -@param[out] out the parsed value on success +@param[in] first pointer to the first character of the number +@param[in] last pointer past the last character +@param[out] out the parsed value on success @return true if the value was parsed exactly; false to fall back to strtod */ -template -bool parse_float_fast(const char* first, const char* last, DecimalPointType decimal_point, double& out) noexcept +inline bool parse_float_fast(const char* first, const char* last, double& out) noexcept { #if defined(FLT_EVAL_METHOD) && FLT_EVAL_METHOD != 0 // Clinger's fast path is only exact when double operations are evaluated in @@ -136,7 +134,6 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci // std::from_chars / std::strtod path. static_cast(first); static_cast(last); - static_cast(decimal_point); static_cast(out); return false; #else @@ -175,7 +172,7 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci ++num_digits; fractional_digits += static_cast(seen_dot); } - else if (static_cast(c) == decimal_point) + else if (c == '.') { if (JSON_HEDLEY_UNLIKELY(seen_dot)) { @@ -260,8 +257,8 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci } /// fast float path is only exact for `double`; decline for float/long double -template -bool parse_float_fast(const char* /*first*/, const char* /*last*/, DecimalPointType /*decimal_point*/, FloatType& /*out*/) noexcept +template +bool parse_float_fast(const char* /*first*/, const char* /*last*/, FloatType& /*out*/) noexcept { return false; } @@ -273,9 +270,7 @@ std::from_chars is locale-independent, correctly rounded, and - via the Eisel-Lemire algorithm in modern standard libraries - much faster than strtod over the whole value range (not just the Clinger subset). It is used only when __cpp_lib_to_chars indicates full floating-point support and only when it -consumes the entire token ([first, last)); a partial parse means the buffer -uses a non-'.' locale decimal point, in which case the caller falls back to the -locale-aware path. An under-/overflow (result_out_of_range) also declines, so +consumes the entire token ([first, last)). An under-/overflow (result_out_of_range) also declines, so the caller's strtod fallback supplies the well-defined ±inf/0 result the parser expects (side-stepping the P4168 divergence between implementations). diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 591a00b74..576498738 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -8605,14 +8605,12 @@ std::strtod. The parser only activates for number_float_t == double; float and long double keep the std::strtof/std::strtold paths (see the templated overload below). -@param[in] first pointer to the first character of the number -@param[in] last pointer past the last character -@param[in] decimal_point the (locale-dependent) decimal point character -@param[out] out the parsed value on success +@param[in] first pointer to the first character of the number +@param[in] last pointer past the last character +@param[out] out the parsed value on success @return true if the value was parsed exactly; false to fall back to strtod */ -template -bool parse_float_fast(const char* first, const char* last, DecimalPointType decimal_point, double& out) noexcept +inline bool parse_float_fast(const char* first, const char* last, double& out) noexcept { #if defined(FLT_EVAL_METHOD) && FLT_EVAL_METHOD != 0 // Clinger's fast path is only exact when double operations are evaluated in @@ -8623,7 +8621,6 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci // std::from_chars / std::strtod path. static_cast(first); static_cast(last); - static_cast(decimal_point); static_cast(out); return false; #else @@ -8662,7 +8659,7 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci ++num_digits; fractional_digits += static_cast(seen_dot); } - else if (static_cast(c) == decimal_point) + else if (c == '.') { if (JSON_HEDLEY_UNLIKELY(seen_dot)) { @@ -8747,8 +8744,8 @@ bool parse_float_fast(const char* first, const char* last, DecimalPointType deci } /// fast float path is only exact for `double`; decline for float/long double -template -bool parse_float_fast(const char* /*first*/, const char* /*last*/, DecimalPointType /*decimal_point*/, FloatType& /*out*/) noexcept +template +bool parse_float_fast(const char* /*first*/, const char* /*last*/, FloatType& /*out*/) noexcept { return false; } @@ -8760,9 +8757,7 @@ std::from_chars is locale-independent, correctly rounded, and - via the Eisel-Lemire algorithm in modern standard libraries - much faster than strtod over the whole value range (not just the Clinger subset). It is used only when __cpp_lib_to_chars indicates full floating-point support and only when it -consumes the entire token ([first, last)); a partial parse means the buffer -uses a non-'.' locale decimal point, in which case the caller falls back to the -locale-aware path. An under-/overflow (result_out_of_range) also declines, so +consumes the entire token ([first, last)). An under-/overflow (result_out_of_range) also declines, so the caller's strtod fallback supplies the well-defined ±inf/0 result the parser expects (side-stepping the P4168 divergence between implementations). @@ -9303,7 +9298,6 @@ class lexer : public lexer_base explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_number_values_ = false) noexcept : ia(std::move(adapter)) , ignore_comments(ignore_comments_) - , decimal_point_char(static_cast(get_decimal_point())) , discard_number_values(discard_number_values_) {} @@ -9319,8 +9313,7 @@ class lexer : public lexer_base // locales ///////////////////// - /// return the locale-dependent decimal point - JSON_HEDLEY_PURE + /// return the decimal point of the current locale static char get_decimal_point() noexcept { const auto* loc = localeconv(); @@ -10189,9 +10182,10 @@ class lexer : public lexer_base token_type::value_float if number could be successfully scanned, token_type::parse_error otherwise - @note The scanner is independent of the current locale. Internally, the - locale's decimal point is used instead of `.` to work with the - locale-dependent converters. + @note The scanner is independent of the current locale: token_buffer + always holds `.`. Only the std::strtod fallback of convert_number() + depends on the locale, and it looks up the decimal point right + before converting (see convert_float_locale_aware()). */ token_type scan_number() // lgtm [cpp/use-of-goto] `goto` is used in this function to implement the number-parsing state machine described above. By design, any finite input will eventually reach the "done" state or return token_type::parse_error. In each intermediate state, 1 byte of the input is appended to the token_buffer vector, and only the already initialized variables token_buffer, number_type, and error_message are manipulated. { @@ -10280,7 +10274,7 @@ scan_number_zero: { case '.': { - add(decimal_point_char); + add(current); decimal_point_position = token_buffer.size() - 1; goto scan_number_decimal1; } @@ -10317,7 +10311,7 @@ scan_number_any1: case '.': { - add(decimal_point_char); + add(current); decimal_point_position = token_buffer.size() - 1; goto scan_number_decimal1; } @@ -10559,9 +10553,9 @@ scan_number_done: // Only a number below 1 can carry further insignificant zeros, and only // while the count stays at the limit does removing them change the - // answer - so this loop is skipped for all but a few tokens. Note - // token_buffer holds the locale's decimal point, so the fraction is - // located through decimal_point_position rather than by searching '.'. + // answer - so this loop is skipped for all but a few tokens. The + // fraction is located through decimal_point_position rather than by + // searching '.'. if (lead_zero != 0) { JSON_ASSERT(has_dot != 0); // an integer "0" cannot reach the limit @@ -10579,8 +10573,8 @@ scan_number_done: @brief convert the number text in token_buffer to its value and token type The digit sequence in token_buffer has already been validated (by the - scan_number() state machine or by the contiguous fast path) and holds the - locale decimal point in place of '.'. Integers are parsed first and fall + scan_number() state machine or by the contiguous fast path) and holds '.' + as decimal point, independent of the locale. Integers are parsed first and fall back to floating point on overflow. This is shared so both scanners produce identical results. @@ -10660,7 +10654,7 @@ scan_number_done: // integer conversion above overflowed. Prefer std::from_chars // (Eisel-Lemire, locale-independent, correctly rounded) when available; // otherwise the exact Clinger fast path (double only); otherwise the - // locale-aware strtof/strtod. + // locale-aware strtof/strtod/strtold. if (parse_float_from_chars(num_begin, num_end, value_float)) { return token_type::value_float; @@ -10669,26 +10663,75 @@ scan_number_done: // extra pass over the token's bytes, which otherwise shows up on // high-precision inputs such as canada.json if (mantissa_fits_clinger(mantissa_end) - && parse_float_fast(num_begin, num_end, decimal_point_char, value_float)) + && parse_float_fast(num_begin, num_end, value_float)) { return token_type::value_float; } - char* endptr = nullptr; // NOLINT(misc-const-correctness,cppcoreguidelines-pro-type-vararg,hicpp-vararg) - strtof(value_float, token_buffer.data(), &endptr); - - // we checked the number format before - JSON_ASSERT(endptr == token_buffer.data() + token_buffer.size()); - + convert_float_locale_aware(); return token_type::value_float; } + /*! + @brief convert the float in token_buffer with strtof/strtod/strtold + + These functions expect the decimal point of the *current* locale, so it is + looked up right before the conversion instead of once when the lexer is + constructed: a locale change in between (by a parser callback, a SAX + handler, or another thread) must not truncate the value (#5198). The + token has been validated before, so if the conversion stops early and the + decimal point changed in the meantime, the locale changed between the + lookup and the call, and the conversion is repeated with the new decimal + point. If the decimal point did not change, a retry cannot succeed: the + locale's decimal point is not a single character (e.g., the two-byte + U+066B of ar_EG.UTF-8 or fa_IR.UTF-8) and cannot be substituted in place. + The value strtod parsed up to that point is kept, as before this change. + + Note that changing the locale in another thread *while* strtod runs is + undefined behavior of the C library, which this function cannot prevent. + */ + void convert_float_locale_aware() + { + const bool has_dot = decimal_point_position != std::string::npos; + char decimal_point = get_decimal_point(); + for (;;) + { + const bool substitute = has_dot && decimal_point != '.'; + if (substitute) + { + token_buffer[decimal_point_position] = static_cast(decimal_point); + } + + char* endptr = nullptr; // NOLINT(misc-const-correctness,cppcoreguidelines-pro-type-vararg,hicpp-vararg) + strtof(value_float, token_buffer.data(), &endptr); + + if (substitute) + { + // get_string() hands the token to the SAX interface with '.' + token_buffer[decimal_point_position] = '.'; + } + + if (JSON_HEDLEY_LIKELY(endptr == token_buffer.data() + token_buffer.size())) + { + return; + } + + // retry only if the locale changed; otherwise, this would loop forever + const char current_decimal_point = get_decimal_point(); + if (current_decimal_point == decimal_point) + { + return; + } + decimal_point = current_decimal_point; + } + } + /*! @brief contiguous fast path for scanning a number Parses the whole number token straight from the input buffer, avoiding the per-character get()/add() of scan_number(). On success it fills token_buffer - (with the locale decimal point substituted, as scan_number() does) and + (as scan_number() does) and returns the token type. On anything it does not fully recognize as a well-formed number it makes no state change and returns token_type::uninitialized, so the caller falls back to scan_number(), which @@ -10804,16 +10847,11 @@ scan_number_done: } #endif - // materialize the token exactly as scan_number() would, substituting the - // locale decimal point so convert_number()'s strtof fallback stays valid. - // reset() already cleared token_buffer, so append() fills it (assign() is - // avoided because custom string_t types need not provide it) + // materialize the token exactly as scan_number() would. reset() already + // cleared token_buffer, so append() fills it (assign() is avoided + // because custom string_t types need not provide it) token_buffer.append(reinterpret_cast(data), len); - if (dot_index != std::string::npos) - { - token_buffer[dot_index] = static_cast(decimal_point_char); - decimal_point_position = dot_index; - } + decimal_point_position = dot_index; ia.bulk_skip(len - 1); position.chars_read_total += (len - 1); @@ -11080,11 +11118,7 @@ scan_number_done: /// return current string value (implicitly resets the token; useful only once) string_t& get_string() { - // translate decimal points from locale back to '.' (#4084) - if (decimal_point_char != '.' && decimal_point_position != std::string::npos) - { - token_buffer[decimal_point_position] = '.'; - } + // a number token holds '.' regardless of the locale (#4084) return token_buffer; } @@ -11380,9 +11414,7 @@ scan_number_done: number_unsigned_t value_unsigned = 0; number_float_t value_float = 0; - /// the decimal point - const char_int_type decimal_point_char = '.'; - /// the position of the decimal point in the input + /// the position of the decimal point in token_buffer std::size_t decimal_point_position = std::string::npos; /// whether the caller (e.g. accept()/json_sax_acceptor) only needs the diff --git a/tests/src/unit-class_lexer.cpp b/tests/src/unit-class_lexer.cpp index cb8ceed6b..5d52179d7 100644 --- a/tests/src/unit-class_lexer.cpp +++ b/tests/src/unit-class_lexer.cpp @@ -666,7 +666,7 @@ TEST_CASE("parse_float_fast declines what it cannot convert exactly") // always safe: the caller then falls back to a slower, exact conversion. const auto fast = [](const std::string & s, double & out) { - return nlohmann::detail::parse_float_fast(s.data(), s.data() + s.size(), '.', out); + return nlohmann::detail::parse_float_fast(s.data(), s.data() + s.size(), out); }; double out = 0; diff --git a/tests/src/unit-locale-cpp.cpp b/tests/src/unit-locale-cpp.cpp index c2a113d06..9f62fc0ca 100644 --- a/tests/src/unit-locale-cpp.cpp +++ b/tests/src/unit-locale-cpp.cpp @@ -12,7 +12,12 @@ #include using nlohmann::json; +#include #include +#include +#include +#include +#include struct ParserImpl final: public nlohmann::json_sax { @@ -175,3 +180,208 @@ TEST_CASE("locale-dependent test (LC_NUMERIC=de_DE)") MESSAGE("locale de_DE is not usable"); } } + +namespace +{ +// records the numbers of a flat array and switches LC_NUMERIC to the given +// locale once the array opens - after the lexer was constructed, but before +// any number in the array is lexed +struct LocaleSwitchingSax final: public nlohmann::json_sax +{ + explicit LocaleSwitchingSax(const char* switch_to) + : locale_after_open(switch_to) + {} + + bool null() override + { + return true; + } + bool boolean(bool /*val*/) override + { + return true; + } + bool number_integer(json::number_integer_t /*val*/) override + { + return true; + } + bool number_unsigned(json::number_unsigned_t /*val*/) override + { + return true; + } + bool number_float(json::number_float_t val, const json::string_t& s) override + { + values.push_back(val); + strings.push_back(s); + return true; + } + bool string(json::string_t& /*val*/) override + { + return true; + } + bool binary(json::binary_t& /*val*/) override + { + return true; + } + bool start_object(std::size_t /*val*/) override + { + return true; + } + bool key(json::string_t& /*val*/) override + { + return true; + } + bool end_object() override + { + return true; + } + bool start_array(std::size_t /*val*/) override + { + switched = std::setlocale(LC_NUMERIC, locale_after_open.c_str()) != nullptr; + return true; + } + bool end_array() override + { + return true; + } + bool parse_error(std::size_t /*val*/, const std::string& /*val*/, const nlohmann::detail::exception& /*val*/) override + { + return false; + } + + std::string locale_after_open; + bool switched = false; + std::vector values {}; // NOLINT(readability-redundant-member-init) + std::vector strings {}; // NOLINT(readability-redundant-member-init) +}; +} // namespace + +TEST_CASE("locale changes between lexer construction and number conversion (#5198)") +{ + // The numbers are chosen so that the conversion also takes the strtod + // fallback, which honors the locale that is current at conversion time: + // too many significant digits for Clinger's fast path, an underflow that + // std::from_chars rejects, and a plain value. + const std::vector numbers = {"3.14159265358979323846", "1.5e-400", "12.34", "-0.000123456789012345678"}; + std::string text = "["; + for (const auto& n : numbers) + { + text += (text.size() == 1 ? "" : ",") + n; + } + text += "]"; + + using long_double_json = nlohmann::basic_json; + + // reference values, parsed without a locale switch + REQUIRE(std::setlocale(LC_NUMERIC, "C") != nullptr); + const json expected = json::parse(text); + const long_double_json expected_ld = long_double_json::parse(text); + + const std::array, 2> transitions = + { + { + {"C", "de_DE"}, + {"de_DE", "C"} + } + }; + + for (const auto& transition : transitions) + { + CAPTURE(transition.first); + CAPTURE(transition.second); + + if (std::setlocale(LC_NUMERIC, transition.first) == nullptr) + { + MESSAGE("locale is not usable"); + continue; + } + + // SAX parsing + { + LocaleSwitchingSax sax(transition.second); + CHECK(json::sax_parse(text, &sax)); + if (sax.switched) + { + CHECK(sax.values == expected.get>()); + CHECK(sax.strings == numbers); + } + } + + // DOM parsing with a callback + { + bool switched = false; + const auto cb = [&](int /*depth*/, json::parse_event_t event, json& /*parsed*/) + { + if (event == json::parse_event_t::array_start) + { + switched = std::setlocale(LC_NUMERIC, transition.second) != nullptr; + } + return true; + }; + const json j = json::parse(text, cb); + if (switched) + { + CHECK(j == expected); + } + } + + // a long double goes through std::strtold unless std::from_chars supports it + { + bool switched = false; + const auto cb = [&](int /*depth*/, long_double_json::parse_event_t event, long_double_json& /*parsed*/) + { + if (event == long_double_json::parse_event_t::array_start) + { + switched = std::setlocale(LC_NUMERIC, transition.second) != nullptr; + } + return true; + }; + const long_double_json j = long_double_json::parse(text, cb); + if (switched) + { + CHECK(j == expected_ld); + } + } + } + + std::setlocale(LC_NUMERIC, "C"); +} + +TEST_CASE("locale with a multi-byte decimal point") +{ + // Some locales use a decimal point that is not a single character, e.g. + // U+066B ARABIC DECIMAL SEPARATOR (two bytes in UTF-8). It cannot be + // substituted in place for '.', so the strtod fallback stops early. The + // conversion must still terminate rather than retry forever. + const std::array names = {{"ar_EG.UTF-8", "ar_SA.UTF-8", "fa_IR.UTF-8", "ps_AF.UTF-8", "ar_EG", "fa_IR"}}; + bool tested = false; + for (const char* name : names) + { + if (std::setlocale(LC_NUMERIC, name) == nullptr) + { + continue; + } + const std::string decimal_point = std::localeconv()->decimal_point; + if (decimal_point.size() < 2) + { + continue; + } + CAPTURE(name); + tested = true; + + // too many significant digits for Clinger's fast path, and an underflow + // that std::from_chars rejects: both reach the strtod fallback + json j; + CHECK_NOTHROW(j = json::parse("[3.14159265358979323846, 1.5e-400, -0.000123456789012345678]")); + CHECK(j.is_array()); + CHECK(json::accept("3.14159265358979323846")); + + // a value the locale-independent paths convert is not affected + CHECK(json::parse("12.5") == 12.5); + } + if (!tested) + { + MESSAGE("no locale with a multi-byte decimal point is usable"); + } + + std::setlocale(LC_NUMERIC, "C"); +} From 633de8e44bbbc29383d72f8c920343ea5d1eaa21 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 28 Sep 2026 22:20:43 +0200 Subject: [PATCH 3/3] Fix CI: clang-tidy and GCC -Wnoexcept in the locale test (#5613) #5597 was merged before all of its CI jobs had run, and two of them fail on develop now, and so on every pull request: - ci_clang_tidy: cert-err33-c for the two std::setlocale(LC_NUMERIC, "C") calls whose result was discarded. Check the result, like the other resets in the file. - ci_test_standards_gcc (20) with GCC 16: -Wnoexcept for the two parser callbacks, which cannot throw but were not declared noexcept. Signed-off-by: Niels Lohmann --- tests/src/unit-locale-cpp.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/src/unit-locale-cpp.cpp b/tests/src/unit-locale-cpp.cpp index 9f62fc0ca..14f743a66 100644 --- a/tests/src/unit-locale-cpp.cpp +++ b/tests/src/unit-locale-cpp.cpp @@ -309,7 +309,7 @@ TEST_CASE("locale changes between lexer construction and number conversion (#519 // DOM parsing with a callback { bool switched = false; - const auto cb = [&](int /*depth*/, json::parse_event_t event, json& /*parsed*/) + const auto cb = [&](int /*depth*/, json::parse_event_t event, json& /*parsed*/) noexcept { if (event == json::parse_event_t::array_start) { @@ -327,7 +327,7 @@ TEST_CASE("locale changes between lexer construction and number conversion (#519 // a long double goes through std::strtold unless std::from_chars supports it { bool switched = false; - const auto cb = [&](int /*depth*/, long_double_json::parse_event_t event, long_double_json& /*parsed*/) + const auto cb = [&](int /*depth*/, long_double_json::parse_event_t event, long_double_json& /*parsed*/) noexcept { if (event == long_double_json::parse_event_t::array_start) { @@ -343,7 +343,7 @@ TEST_CASE("locale changes between lexer construction and number conversion (#519 } } - std::setlocale(LC_NUMERIC, "C"); + CHECK(std::setlocale(LC_NUMERIC, "C") != nullptr); } TEST_CASE("locale with a multi-byte decimal point") @@ -383,5 +383,5 @@ TEST_CASE("locale with a multi-byte decimal point") MESSAGE("no locale with a multi-byte decimal point is usable"); } - std::setlocale(LC_NUMERIC, "C"); + CHECK(std::setlocale(LC_NUMERIC, "C") != nullptr); }