From 69a0c1b82ca0d5c7e3e82524ce09abd407b4a0d6 Mon Sep 17 00:00:00 2001 From: Alex Prabhat Bara Date: Fri, 9 Oct 2026 16:52:07 +0530 Subject: [PATCH] Avoid allocating temporary basic_json for cbor and msgpack object keys (#5328) * avoid allocating temporary basic_json for CBOR and MessagePack object keys Signed-off-by: alexprabhat99 * add size() to the custom object key test type UBJSON and BJData access object keys through size() and c_str() directly, so the key type now provides both and the comment says why. Signed-off-by: alexprabhat99 * address review: drop key size()/c_str(), test keys below the depth limit Nothing in the library calls size() or c_str() on an object key, so the test key type only keeps data(), which JSON_DIAGNOSTICS needs. The CBOR and MessagePack custom key tests now also nest objects deeper than detail::recursion_depth_limit(), so keys written by write_cbor_iterative and write_msgpack_iterative are covered as well. Signed-off-by: alexprabhat99 --------- Signed-off-by: alexprabhat99 --- docs/mkdocs/docs/api/basic_json/object_t.md | 3 +- .../nlohmann/detail/output/binary_writer.hpp | 180 ++++++++++-------- single_include/nlohmann/json.hpp | 180 ++++++++++-------- tests/src/custom_object_key_type.hpp | 75 ++++++++ tests/src/unit-cbor.cpp | 53 ++++++ tests/src/unit-msgpack.cpp | 53 ++++++ 6 files changed, 393 insertions(+), 151 deletions(-) create mode 100644 tests/src/custom_object_key_type.hpp diff --git a/docs/mkdocs/docs/api/basic_json/object_t.md b/docs/mkdocs/docs/api/basic_json/object_t.md index 204e5ab28..5e7cab9e9 100644 --- a/docs/mkdocs/docs/api/basic_json/object_t.md +++ b/docs/mkdocs/docs/api/basic_json/object_t.md @@ -26,7 +26,8 @@ To store objects in C++, a type is defined by the template parameters described `StringType` : the type of the keys or names (e.g., `std::string`). The comparison function `std::less` is used to - order elements inside the container. + order elements inside the container. `object_t::key_type` must be implicitly convertible to `string_t` (required by the + binary formats). `AllocatorType` : the allocator to use for objects (e.g., `std::allocator`) diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 1cb64f08d..4af3ff9fb 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -244,16 +244,7 @@ class binary_writer case value_t::string: { - string_t storage; - const string_t& value = sanitize_utf8_for_write(*j.m_data.m_value.string, j, storage); - - // step 1: write control byte and the string length - write_cbor_head(0x60, value.size()); - - // step 2: write the string - oa.write_characters( - reinterpret_cast(value.data()), - value.size()); + write_cbor_string(*j.m_data.m_value.string, j); break; } @@ -316,23 +307,20 @@ class binary_writer case value_t::object: { + static_assert( + std::is_convertible < + typename BasicJsonType::object_t::key_type, + string_t >::value, + "object_t::key_type must be implicitly convertible to string_t"); + // step 1: write control byte and the object size write_cbor_head(0xA0, j.m_data.m_value.object->size()); for (const auto& el : *j.m_data.m_value.object) { - // el.first is checked here, against the object as - // diagnostics context, because write_cbor(el.first) - // converts it to a temporary basic_json that would be - // used as the context instead; for error_handler_t::keep - // and ::replace/::ignore the recursive write_cbor(el.first) - // call below handles the key like any other string, so no - // separate check is needed here for those - if (error_handler == error_handler_t::strict) - { - check_utf8(el.first, j); - } - write_cbor(el.first); + // el.first is written directly (not via a temporary + // basic_json), with the object as diagnostics context + write_cbor_string(el.first, j); write_cbor(el.second, depth + 1); } break; @@ -491,39 +479,7 @@ class binary_writer case value_t::string: { - string_t storage; - const string_t& value = sanitize_utf8_for_write(*j.m_data.m_value.string, j, storage); - - // step 1: write control byte and the string length - const auto N = to_msgpack_length(value.size(), j); - if (N <= 31) - { - // fixstr - write_number(static_cast(0xA0 | N)); - } - else if (N <= (std::numeric_limits::max)()) - { - // str 8 - oa.write_character(to_char_type(0xD9)); - write_number(static_cast(N)); - } - else if (N <= (std::numeric_limits::max)()) - { - // str 16 - oa.write_character(to_char_type(0xDA)); - write_number(static_cast(N)); - } - else - { - // str 32 - oa.write_character(to_char_type(0xDB)); - write_number(static_cast(N)); - } - - // step 2: write the string - oa.write_characters( - reinterpret_cast(value.data()), - value.size()); + write_msgpack_string(*j.m_data.m_value.string, j); break; } @@ -629,19 +585,20 @@ class binary_writer case value_t::object: { + static_assert( + std::is_convertible < + typename BasicJsonType::object_t::key_type, + string_t >::value, + "object_t::key_type must be implicitly convertible to string_t"); + // step 1: write control byte and the object size write_msgpack_object_prefix(j.m_data.m_value.object->size(), j); for (const auto& el : *j.m_data.m_value.object) { - // as in write_cbor, el.first is checked here against the - // object as diagnostics context; the recursive call below - // handles keep/replace/ignore like any other string - if (error_handler == error_handler_t::strict) - { - check_utf8(el.first, j); - } - write_msgpack(el.first); + // as in write_cbor, el.first is written directly with the + // object as diagnostics context + write_msgpack_string(el.first, j); write_msgpack(el.second, depth + 1); } break; @@ -1025,13 +982,9 @@ class binary_writer continue; } - // el.first is checked here, against the object as diagnostics - // context, like the matching check in write_cbor's object case - if (error_handler == error_handler_t::strict) - { - check_utf8(current.object_it->first, *current.value); - } - write_cbor(current.object_it->first); + // the key is written directly (not via a temporary basic_json), + // with the object as diagnostics context, as in write_cbor + write_cbor_string(current.object_it->first, *current.value); const BasicJsonType* child = &(current.object_it->second); ++stack.back().object_it; write_cbor_value_or_push(*child, stack); @@ -1106,11 +1059,9 @@ class binary_writer continue; } - if (error_handler == error_handler_t::strict) - { - check_utf8(current.object_it->first, *current.value); - } - write_msgpack(current.object_it->first); + // as in write_cbor_iterative, the key is written directly with + // the object as diagnostics context + write_msgpack_string(current.object_it->first, *current.value); const BasicJsonType* child = &(current.object_it->second); ++stack.back().object_it; write_msgpack_value_or_push(*child, stack); @@ -1988,6 +1939,85 @@ class binary_writer } } + /*! + @brief write a CBOR text string + + @a value is checked or sanitized according to @ref error_handler, with + @a context (the string value itself, or the object a key belongs to) used + as diagnostics context; this avoids converting object keys to a temporary + basic_json just to write them + + @note When object_t::key_type is not string_t, @a value is a temporary + string_t converted from the key, which lives only until the end of + the caller's statement. The reference returned by + @ref sanitize_utf8_for_write may refer to it, so it must not escape + this function. + */ + void write_cbor_string(const string_t& value, const BasicJsonType& context) + { + string_t storage; + const string_t& sanitized = sanitize_utf8_for_write(value, context, storage); + + // step 1: write control byte and the string length + write_cbor_head(0x60, sanitized.size()); + + // step 2: write the string + oa.write_characters( + reinterpret_cast(sanitized.data()), + sanitized.size()); + } + + ///////////// + // MsgPack // + ///////////// + + /*! + @brief write a MessagePack str + + @a value is checked or sanitized according to @ref error_handler, with + @a context used as diagnostics context, as in @ref write_cbor_string + + @note As in @ref write_cbor_string, @a value may be a temporary string_t + converted from a key, so the reference returned by + @ref sanitize_utf8_for_write must not escape this function. + */ + void write_msgpack_string(const string_t& value, const BasicJsonType& context) + { + string_t storage; + const string_t& sanitized = sanitize_utf8_for_write(value, context, storage); + + // step 1: write control byte and the string length + const auto N = to_msgpack_length(sanitized.size(), context); + if (N <= 31) + { + // fixstr + write_number(static_cast(0xA0 | N)); + } + else if (N <= (std::numeric_limits::max)()) + { + // str 8 + oa.write_character(to_char_type(0xD9)); + write_number(static_cast(N)); + } + else if (N <= (std::numeric_limits::max)()) + { + // str 16 + oa.write_character(to_char_type(0xDA)); + write_number(static_cast(N)); + } + else + { + // str 32 + oa.write_character(to_char_type(0xDB)); + write_number(static_cast(N)); + } + + // step 2: write the string + oa.write_characters( + reinterpret_cast(sanitized.data()), + sanitized.size()); + } + //////////// // UBJSON // //////////// diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 72c95feea..428a6abbf 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -21749,16 +21749,7 @@ class binary_writer case value_t::string: { - string_t storage; - const string_t& value = sanitize_utf8_for_write(*j.m_data.m_value.string, j, storage); - - // step 1: write control byte and the string length - write_cbor_head(0x60, value.size()); - - // step 2: write the string - oa.write_characters( - reinterpret_cast(value.data()), - value.size()); + write_cbor_string(*j.m_data.m_value.string, j); break; } @@ -21821,23 +21812,20 @@ class binary_writer case value_t::object: { + static_assert( + std::is_convertible < + typename BasicJsonType::object_t::key_type, + string_t >::value, + "object_t::key_type must be implicitly convertible to string_t"); + // step 1: write control byte and the object size write_cbor_head(0xA0, j.m_data.m_value.object->size()); for (const auto& el : *j.m_data.m_value.object) { - // el.first is checked here, against the object as - // diagnostics context, because write_cbor(el.first) - // converts it to a temporary basic_json that would be - // used as the context instead; for error_handler_t::keep - // and ::replace/::ignore the recursive write_cbor(el.first) - // call below handles the key like any other string, so no - // separate check is needed here for those - if (error_handler == error_handler_t::strict) - { - check_utf8(el.first, j); - } - write_cbor(el.first); + // el.first is written directly (not via a temporary + // basic_json), with the object as diagnostics context + write_cbor_string(el.first, j); write_cbor(el.second, depth + 1); } break; @@ -21996,39 +21984,7 @@ class binary_writer case value_t::string: { - string_t storage; - const string_t& value = sanitize_utf8_for_write(*j.m_data.m_value.string, j, storage); - - // step 1: write control byte and the string length - const auto N = to_msgpack_length(value.size(), j); - if (N <= 31) - { - // fixstr - write_number(static_cast(0xA0 | N)); - } - else if (N <= (std::numeric_limits::max)()) - { - // str 8 - oa.write_character(to_char_type(0xD9)); - write_number(static_cast(N)); - } - else if (N <= (std::numeric_limits::max)()) - { - // str 16 - oa.write_character(to_char_type(0xDA)); - write_number(static_cast(N)); - } - else - { - // str 32 - oa.write_character(to_char_type(0xDB)); - write_number(static_cast(N)); - } - - // step 2: write the string - oa.write_characters( - reinterpret_cast(value.data()), - value.size()); + write_msgpack_string(*j.m_data.m_value.string, j); break; } @@ -22134,19 +22090,20 @@ class binary_writer case value_t::object: { + static_assert( + std::is_convertible < + typename BasicJsonType::object_t::key_type, + string_t >::value, + "object_t::key_type must be implicitly convertible to string_t"); + // step 1: write control byte and the object size write_msgpack_object_prefix(j.m_data.m_value.object->size(), j); for (const auto& el : *j.m_data.m_value.object) { - // as in write_cbor, el.first is checked here against the - // object as diagnostics context; the recursive call below - // handles keep/replace/ignore like any other string - if (error_handler == error_handler_t::strict) - { - check_utf8(el.first, j); - } - write_msgpack(el.first); + // as in write_cbor, el.first is written directly with the + // object as diagnostics context + write_msgpack_string(el.first, j); write_msgpack(el.second, depth + 1); } break; @@ -22530,13 +22487,9 @@ class binary_writer continue; } - // el.first is checked here, against the object as diagnostics - // context, like the matching check in write_cbor's object case - if (error_handler == error_handler_t::strict) - { - check_utf8(current.object_it->first, *current.value); - } - write_cbor(current.object_it->first); + // the key is written directly (not via a temporary basic_json), + // with the object as diagnostics context, as in write_cbor + write_cbor_string(current.object_it->first, *current.value); const BasicJsonType* child = &(current.object_it->second); ++stack.back().object_it; write_cbor_value_or_push(*child, stack); @@ -22611,11 +22564,9 @@ class binary_writer continue; } - if (error_handler == error_handler_t::strict) - { - check_utf8(current.object_it->first, *current.value); - } - write_msgpack(current.object_it->first); + // as in write_cbor_iterative, the key is written directly with + // the object as diagnostics context + write_msgpack_string(current.object_it->first, *current.value); const BasicJsonType* child = &(current.object_it->second); ++stack.back().object_it; write_msgpack_value_or_push(*child, stack); @@ -23493,6 +23444,85 @@ class binary_writer } } + /*! + @brief write a CBOR text string + + @a value is checked or sanitized according to @ref error_handler, with + @a context (the string value itself, or the object a key belongs to) used + as diagnostics context; this avoids converting object keys to a temporary + basic_json just to write them + + @note When object_t::key_type is not string_t, @a value is a temporary + string_t converted from the key, which lives only until the end of + the caller's statement. The reference returned by + @ref sanitize_utf8_for_write may refer to it, so it must not escape + this function. + */ + void write_cbor_string(const string_t& value, const BasicJsonType& context) + { + string_t storage; + const string_t& sanitized = sanitize_utf8_for_write(value, context, storage); + + // step 1: write control byte and the string length + write_cbor_head(0x60, sanitized.size()); + + // step 2: write the string + oa.write_characters( + reinterpret_cast(sanitized.data()), + sanitized.size()); + } + + ///////////// + // MsgPack // + ///////////// + + /*! + @brief write a MessagePack str + + @a value is checked or sanitized according to @ref error_handler, with + @a context used as diagnostics context, as in @ref write_cbor_string + + @note As in @ref write_cbor_string, @a value may be a temporary string_t + converted from a key, so the reference returned by + @ref sanitize_utf8_for_write must not escape this function. + */ + void write_msgpack_string(const string_t& value, const BasicJsonType& context) + { + string_t storage; + const string_t& sanitized = sanitize_utf8_for_write(value, context, storage); + + // step 1: write control byte and the string length + const auto N = to_msgpack_length(sanitized.size(), context); + if (N <= 31) + { + // fixstr + write_number(static_cast(0xA0 | N)); + } + else if (N <= (std::numeric_limits::max)()) + { + // str 8 + oa.write_character(to_char_type(0xD9)); + write_number(static_cast(N)); + } + else if (N <= (std::numeric_limits::max)()) + { + // str 16 + oa.write_character(to_char_type(0xDA)); + write_number(static_cast(N)); + } + else + { + // str 32 + oa.write_character(to_char_type(0xDB)); + write_number(static_cast(N)); + } + + // step 2: write the string + oa.write_characters( + reinterpret_cast(sanitized.data()), + sanitized.size()); + } + //////////// // UBJSON // //////////// diff --git a/tests/src/custom_object_key_type.hpp b/tests/src/custom_object_key_type.hpp new file mode 100644 index 000000000..fa23736d4 --- /dev/null +++ b/tests/src/custom_object_key_type.hpp @@ -0,0 +1,75 @@ +// __ _____ _____ _____ +// __| | __| | | | JSON for Modern C++ (supporting code) +// | | |__ | | | | | | version 3.12.0 +// |_____|_____|_____|_|___| https://github.com/nlohmann/json +// +// SPDX-FileCopyrightText: 2013-2026 Niels Lohmann +// SPDX-License-Identifier: MIT + +#pragma once + +#include +#include +#include +#include +#include + +namespace custom_object_key_test +{ +class key +{ + public: + key() = default; + + key(const char* value) + : m_value(value) + {} + + key(std::string value) + : m_value(std::move(value)) + {} + + operator std::string() const + { + return m_value; + } + + // Required by JSON_DIAGNOSTICS, which reads object keys through data() + // when building the path of an exception. + const char* data() const noexcept + { + return m_value.data(); + } + + friend bool operator<(const key& lhs, const key& rhs) + { + return lhs.m_value < rhs.m_value; + } + + private: + std::string m_value; +}; + +template +class object + : public std::map < + key, + Value, + std::less, // NOLINT(modernize-use-transparent-functors) + typename std::allocator_traits::template rebind_alloc < + std::pair>> +{ + private: + using allocator_type = + typename std::allocator_traits::template rebind_alloc < + std::pair>; + + using base_type = + std::map, allocator_type>; // NOLINT(modernize-use-transparent-functors) + + public: + using base_type::base_type; +}; + +using json = nlohmann::basic_json; +} // namespace custom_object_key_test diff --git a/tests/src/unit-cbor.cpp b/tests/src/unit-cbor.cpp index 9ee371495..9b57d0664 100644 --- a/tests/src/unit-cbor.cpp +++ b/tests/src/unit-cbor.cpp @@ -28,6 +28,7 @@ using nlohmann::json; #include "make_test_data_available.hpp" #include "round_trip_corpus.hpp" #include "test_utils.hpp" +#include "custom_object_key_type.hpp" #include "sax_countdown.hpp" using utils::SaxCountdown; @@ -3357,3 +3358,55 @@ TEST_CASE("CBOR large strings and binaries (chunked reader)") } } } + +TEST_CASE("CBOR supports custom object key types") +{ + using custom_json = custom_object_key_test::json; + using custom_key = custom_object_key_test::key; + + custom_json::object_t object; + object.emplace(custom_key{"short"}, 1); + object.emplace( + custom_key{"a key longer than twenty-three characters"}, + 2); + + const custom_json value(std::move(object)); + const auto encoded = custom_json::to_cbor(value); + + CHECK(nlohmann::json::from_cbor(encoded) == nlohmann::json + { + {"short", 1}, + {"a key longer than twenty-three characters", 2} + }); +} + +TEST_CASE("CBOR supports custom object key types nested deeper than the recursion depth limit") +{ + // below detail::recursion_depth_limit(), keys are written by + // write_cbor_iterative instead of write_cbor + using custom_json = custom_object_key_test::json; + using custom_key = custom_object_key_test::key; + + const std::size_t depth = nlohmann::detail::recursion_depth_limit() + 10; + + custom_json value = 1; + nlohmann::json expected = 1; + for (std::size_t i = 0; i < depth; ++i) + { + // alternate short keys with ones long enough to need a length byte + const std::string name = (i % 2 == 0) ? "k" + std::to_string(i) + : "a key longer than thirty-one characters " + std::to_string(i); + + custom_json::object_t object; + object.emplace(custom_key{name}, std::move(value)); + value = custom_json(std::move(object)); + + nlohmann::json::object_t expected_object; + expected_object.emplace(name, std::move(expected)); + expected = nlohmann::json(std::move(expected_object)); + } + + const auto encoded = custom_json::to_cbor(value); + CHECK(encoded == nlohmann::json::to_cbor(expected)); + CHECK(nlohmann::json::from_cbor(encoded) == expected); +} diff --git a/tests/src/unit-msgpack.cpp b/tests/src/unit-msgpack.cpp index d29769738..0e07c8ea4 100644 --- a/tests/src/unit-msgpack.cpp +++ b/tests/src/unit-msgpack.cpp @@ -31,6 +31,7 @@ using nlohmann::json; #include "make_test_data_available.hpp" #include "round_trip_corpus.hpp" #include "test_utils.hpp" +#include "custom_object_key_type.hpp" #include "sax_countdown.hpp" using utils::SaxCountdown; @@ -2522,3 +2523,55 @@ TEST_CASE("MessagePack large strings and binaries (chunked reader)") } } } + +TEST_CASE("MessagePack supports custom object key types") +{ + using custom_json = custom_object_key_test::json; + using custom_key = custom_object_key_test::key; + + custom_json::object_t object; + object.emplace(custom_key{"short"}, 1); + object.emplace( + custom_key{"a key longer than thirty-one characters"}, + 2); + + const custom_json value(std::move(object)); + const auto encoded = custom_json::to_msgpack(value); + + CHECK(nlohmann::json::from_msgpack(encoded) == nlohmann::json + { + {"short", 1}, + {"a key longer than thirty-one characters", 2} + }); +} + +TEST_CASE("MessagePack supports custom object key types nested deeper than the recursion depth limit") +{ + // below detail::recursion_depth_limit(), keys are written by + // write_msgpack_iterative instead of write_msgpack + using custom_json = custom_object_key_test::json; + using custom_key = custom_object_key_test::key; + + const std::size_t depth = nlohmann::detail::recursion_depth_limit() + 10; + + custom_json value = 1; + nlohmann::json expected = 1; + for (std::size_t i = 0; i < depth; ++i) + { + // alternate short keys with ones long enough to need a length byte + const std::string name = (i % 2 == 0) ? "k" + std::to_string(i) + : "a key longer than thirty-one characters " + std::to_string(i); + + custom_json::object_t object; + object.emplace(custom_key{name}, std::move(value)); + value = custom_json(std::move(object)); + + nlohmann::json::object_t expected_object; + expected_object.emplace(name, std::move(expected)); + expected = nlohmann::json(std::move(expected_object)); + } + + const auto encoded = custom_json::to_msgpack(value); + CHECK(encoded == nlohmann::json::to_msgpack(expected)); + CHECK(nlohmann::json::from_msgpack(encoded) == expected); +}