diff --git a/include/nlohmann/detail/input/json_sax.hpp b/include/nlohmann/detail/input/json_sax.hpp index c8a3da5c3..d1ab4f83a 100644 --- a/include/nlohmann/detail/input/json_sax.hpp +++ b/include/nlohmann/detail/input/json_sax.hpp @@ -9,6 +9,7 @@ #pragma once #include // copy, find_if, min +#include // array #include #include // hash #include // numeric_limits @@ -400,7 +401,7 @@ class object_key_index /// the hash of each element, by position std::vector hashes; /// open addressing, a power of two in size: position + 1 of an element, or 0 for a free slot - std::vector table {}; + std::vector table {}; // NOLINT(readability-redundant-member-init) std::size_t mask = 0; }; @@ -413,15 +414,21 @@ hierarchy of the JSON value is managed by the stack `frames` which contains an entry for each open array or object. The parser of the text formats, for which the size of a container is not known up front, collects the elements of an array and the members of an `ordered_json` object on a stack and moves them -into a container of the exact size; the binary readers use the plain +into a container of the exact size (with @a Collect, as basic_json's own parse +functions use it); the binary readers use the plain @ref json_sax_dom_binary_parser instead. After successful parsing, the value that is passed by reference to the -constructor contains the parsed value. +constructor contains the parsed value. Without @a Collect, every value is +stored in place as it arrives, so after a parse error the value holds what was +read up to it. @tparam BasicJsonType the JSON type +@tparam Collect whether to collect the values of containers of unknown size on + a stack (only for a caller that does not use the value after a + parse error) */ -template +template class json_sax_dom_parser { public: @@ -531,9 +538,10 @@ class json_sax_dom_parser // pointers and positions refer to it), so no buffering start_stored_array(len); #else - if (len != detail::unknown_size()) + if (!collects(len)) { - // the number of elements is known: reserve and store them directly + // the number of elements is known (or the elements must be in + // place as they arrive): store them directly start_stored_array(len); } else @@ -567,7 +575,7 @@ class json_sax_dom_parser { // the elements are on the stack: make them the array const auto first = elements.begin() + static_cast(f.start); - array_t* const arr = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(elements.end())); + auto* const arr = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(elements.end())); BasicJsonType value; value.m_data.m_type = BasicJsonType::value_t::array; value.m_data.m_value.array = arr; @@ -671,11 +679,14 @@ class json_sax_dom_parser class frame_stack { public: - frame_stack() = default; + frame_stack() noexcept + : local() // NOLINT(readability-redundant-member-init) + {} frame_stack(const frame_stack&) = delete; frame_stack& operator=(const frame_stack&) = delete; frame_stack(frame_stack&& other) noexcept + : local() // NOLINT(readability-redundant-member-init) { take(other); } @@ -690,10 +701,7 @@ class json_sax_dom_parser return *this; } - ~frame_stack() - { - release(); - } + ~frame_stack() = default; bool empty() const noexcept { @@ -735,37 +743,35 @@ class json_sax_dom_parser void grow() { const std::size_t new_capacity = capacity * 2; - frame* const new_data = new frame[new_capacity]; - std::copy(data, data + count, new_data); - release(); - data = new_data; + std::unique_ptr new_heap(new frame[new_capacity]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + std::copy(data, data + count, new_heap.get()); + heap = std::move(new_heap); + data = heap.get(); capacity = new_capacity; } void release() noexcept { - if (data != local) - { - delete[] data; - data = local; - capacity = local_capacity; - } + heap.reset(); + data = local.data(); + capacity = local_capacity; } /// make the contents of @a other our own void take(frame_stack& other) noexcept { - if (other.data == other.local) + if (other.heap == nullptr) { - std::copy(other.local, other.local + other.count, local); - data = local; + std::copy(other.local.begin(), other.local.begin() + static_cast(other.count), local.begin()); + data = local.data(); capacity = local_capacity; } else { - data = other.data; + heap = std::move(other.heap); + data = heap.get(); capacity = other.capacity; - other.data = other.local; + other.data = other.local.data(); other.capacity = local_capacity; } count = other.count; @@ -773,8 +779,19 @@ class json_sax_dom_parser } static constexpr std::size_t local_capacity = 8; - frame local[local_capacity]; - frame* data = local; + + /// the first frames, left uninitialized: a frame is written before it is + /// read (the user-provided constructor keeps the member initialization, + /// which -Weffc++ asks for, from zeroing them) + struct local_frames : std::array + { + local_frames() noexcept {} // NOLINT(modernize-use-equals-default,hicpp-use-equals-default,cppcoreguidelines-pro-type-member-init,hicpp-member-init): see above + }; + + local_frames local; + /// the frames once there are more than local_capacity of them + std::unique_ptr heap {}; // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays,readability-redundant-member-init) + frame* data = local.data(); std::size_t count = 0; std::size_t capacity = local_capacity; }; @@ -948,7 +965,7 @@ class json_sax_dom_parser /// when the closing brace arrives. bool start_object(std::size_t len, std::true_type /*indexed*/) { - if (len != detail::unknown_size()) + if (!collects(len)) { return start_stored_object(len); } @@ -1020,7 +1037,7 @@ class json_sax_dom_parser object_t* members_to_object(const frame& f, BasicJsonType& value) { const auto first = ordered_data.members.begin() + static_cast(f.start); - object_t* const obj = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(ordered_data.members.end())); + auto* const obj = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(ordered_data.members.end())); value.m_data.m_type = BasicJsonType::value_t::object; value.m_data.m_value.object = obj; ordered_data.members.erase(first, ordered_data.members.end()); @@ -1035,6 +1052,9 @@ class json_sax_dom_parser f.object = members_to_object(f, value); f.ordered.buffered = false; in_object = true; + // false positive: an object without an enclosing container is the root, + // which has a slot, so place_container() does not use the nullptr + // @infer-ignore NULLPTR_DEREFERENCE place_container(f, frames.size() >= 2 ? &frames[frames.size() - 2] : nullptr, std::move(value), false); } @@ -1232,6 +1252,22 @@ class json_sax_dom_parser member = BasicJsonType(std::forward(v)); } + /// whether the values of a container of @a len elements are collected on the stack + static bool collects(const std::size_t len) noexcept + { + return collects(len, std::integral_constant {}); + } + + static bool collects(const std::size_t len, std::true_type /*collect*/) noexcept + { + return len == detail::unknown_size(); + } + + static bool collects(const std::size_t /*len*/, std::false_type /*collect*/) noexcept + { + return false; + } + /// after a container was closed: the one it was in is the one that gets the next value void enter_enclosing_container() { @@ -1251,12 +1287,11 @@ class json_sax_dom_parser template void handle_value(Value&& v) { - BasicJsonType* const stored = store(std::forward(v)); - #if JSON_DIAGNOSTIC_POSITIONS + BasicJsonType* const stored = store(std::forward(v)); diagnostic_positions::set_from_lexer(*stored, m_lexer_ref); #else - static_cast(stored); + store(std::forward(v)); #endif } @@ -1275,7 +1310,7 @@ class json_sax_dom_parser /// the member of the innermost such object that key() added or found last std::size_t member_index = 0; /// the key indexes of the open objects that have one, innermost last - std::vector> key_indexes {}; + std::vector> key_indexes {}; // NOLINT(readability-redundant-member-init) }; struct no_ordered_data_t {}; typename std::conditional::type ordered_data {}; diff --git a/include/nlohmann/detail/input/parser.hpp b/include/nlohmann/detail/input/parser.hpp index b03aafd10..bfa70b01c 100644 --- a/include/nlohmann/detail/input/parser.hpp +++ b/include/nlohmann/detail/input/parser.hpp @@ -56,8 +56,8 @@ using parser_callback_t = template struct sax_ignores_number_text : std::false_type {}; -template -struct sax_ignores_number_text> : std::true_type {}; +template +struct sax_ignores_number_text> : std::true_type {}; template struct sax_ignores_number_text> : std::true_type {}; @@ -131,7 +131,7 @@ class parser } else { - json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); + json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); // in case of an error, return a discarded value if (!parse_dom(sdp, strict)) diff --git a/include/nlohmann/detail/view/materialize.hpp b/include/nlohmann/detail/view/materialize.hpp index b15bec551..93ce02f5c 100644 --- a/include/nlohmann/detail/view/materialize.hpp +++ b/include/nlohmann/detail/view/materialize.hpp @@ -40,7 +40,7 @@ template BasicJsonType materialize(const document_data& d, const node* n) { using string_t = typename BasicJsonType::string_t; - using sax_t = json_sax_dom_parser>; + using sax_t = json_sax_dom_parser, true>; using nav = navigation; // the open containers: their next element (or key of the next member), diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 09e72af6e..2dd35fdeb 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -158,7 +158,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec friend class ::nlohmann::detail::binary_writer; template friend class ::nlohmann::detail::binary_reader; - template + template friend class ::nlohmann::detail::json_sax_dom_parser; template friend class ::nlohmann::detail::json_sax_dom_binary_parser; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index caefe88fc..27f8b8a3c 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -9694,6 +9694,7 @@ NLOHMANN_JSON_NAMESPACE_END #include // copy, find_if, min +#include // array #include #include // hash #include // numeric_limits @@ -15333,7 +15334,7 @@ class object_key_index /// the hash of each element, by position std::vector hashes; /// open addressing, a power of two in size: position + 1 of an element, or 0 for a free slot - std::vector table {}; + std::vector table {}; // NOLINT(readability-redundant-member-init) std::size_t mask = 0; }; @@ -15346,15 +15347,21 @@ hierarchy of the JSON value is managed by the stack `frames` which contains an entry for each open array or object. The parser of the text formats, for which the size of a container is not known up front, collects the elements of an array and the members of an `ordered_json` object on a stack and moves them -into a container of the exact size; the binary readers use the plain +into a container of the exact size (with @a Collect, as basic_json's own parse +functions use it); the binary readers use the plain @ref json_sax_dom_binary_parser instead. After successful parsing, the value that is passed by reference to the -constructor contains the parsed value. +constructor contains the parsed value. Without @a Collect, every value is +stored in place as it arrives, so after a parse error the value holds what was +read up to it. @tparam BasicJsonType the JSON type +@tparam Collect whether to collect the values of containers of unknown size on + a stack (only for a caller that does not use the value after a + parse error) */ -template +template class json_sax_dom_parser { public: @@ -15464,9 +15471,10 @@ class json_sax_dom_parser // pointers and positions refer to it), so no buffering start_stored_array(len); #else - if (len != detail::unknown_size()) + if (!collects(len)) { - // the number of elements is known: reserve and store them directly + // the number of elements is known (or the elements must be in + // place as they arrive): store them directly start_stored_array(len); } else @@ -15500,7 +15508,7 @@ class json_sax_dom_parser { // the elements are on the stack: make them the array const auto first = elements.begin() + static_cast(f.start); - array_t* const arr = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(elements.end())); + auto* const arr = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(elements.end())); BasicJsonType value; value.m_data.m_type = BasicJsonType::value_t::array; value.m_data.m_value.array = arr; @@ -15604,11 +15612,14 @@ class json_sax_dom_parser class frame_stack { public: - frame_stack() = default; + frame_stack() noexcept + : local() // NOLINT(readability-redundant-member-init) + {} frame_stack(const frame_stack&) = delete; frame_stack& operator=(const frame_stack&) = delete; frame_stack(frame_stack&& other) noexcept + : local() // NOLINT(readability-redundant-member-init) { take(other); } @@ -15623,10 +15634,7 @@ class json_sax_dom_parser return *this; } - ~frame_stack() - { - release(); - } + ~frame_stack() = default; bool empty() const noexcept { @@ -15668,37 +15676,35 @@ class json_sax_dom_parser void grow() { const std::size_t new_capacity = capacity * 2; - frame* const new_data = new frame[new_capacity]; - std::copy(data, data + count, new_data); - release(); - data = new_data; + std::unique_ptr new_heap(new frame[new_capacity]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + std::copy(data, data + count, new_heap.get()); + heap = std::move(new_heap); + data = heap.get(); capacity = new_capacity; } void release() noexcept { - if (data != local) - { - delete[] data; - data = local; - capacity = local_capacity; - } + heap.reset(); + data = local.data(); + capacity = local_capacity; } /// make the contents of @a other our own void take(frame_stack& other) noexcept { - if (other.data == other.local) + if (other.heap == nullptr) { - std::copy(other.local, other.local + other.count, local); - data = local; + std::copy(other.local.begin(), other.local.begin() + static_cast(other.count), local.begin()); + data = local.data(); capacity = local_capacity; } else { - data = other.data; + heap = std::move(other.heap); + data = heap.get(); capacity = other.capacity; - other.data = other.local; + other.data = other.local.data(); other.capacity = local_capacity; } count = other.count; @@ -15706,8 +15712,19 @@ class json_sax_dom_parser } static constexpr std::size_t local_capacity = 8; - frame local[local_capacity]; - frame* data = local; + + /// the first frames, left uninitialized: a frame is written before it is + /// read (the user-provided constructor keeps the member initialization, + /// which -Weffc++ asks for, from zeroing them) + struct local_frames : std::array + { + local_frames() noexcept {} // NOLINT(modernize-use-equals-default,hicpp-use-equals-default,cppcoreguidelines-pro-type-member-init,hicpp-member-init): see above + }; + + local_frames local; + /// the frames once there are more than local_capacity of them + std::unique_ptr heap {}; // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays,readability-redundant-member-init) + frame* data = local.data(); std::size_t count = 0; std::size_t capacity = local_capacity; }; @@ -15881,7 +15898,7 @@ class json_sax_dom_parser /// when the closing brace arrives. bool start_object(std::size_t len, std::true_type /*indexed*/) { - if (len != detail::unknown_size()) + if (!collects(len)) { return start_stored_object(len); } @@ -15953,7 +15970,7 @@ class json_sax_dom_parser object_t* members_to_object(const frame& f, BasicJsonType& value) { const auto first = ordered_data.members.begin() + static_cast(f.start); - object_t* const obj = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(ordered_data.members.end())); + auto* const obj = BasicJsonType::template create(std::make_move_iterator(first), std::make_move_iterator(ordered_data.members.end())); value.m_data.m_type = BasicJsonType::value_t::object; value.m_data.m_value.object = obj; ordered_data.members.erase(first, ordered_data.members.end()); @@ -15968,6 +15985,9 @@ class json_sax_dom_parser f.object = members_to_object(f, value); f.ordered.buffered = false; in_object = true; + // false positive: an object without an enclosing container is the root, + // which has a slot, so place_container() does not use the nullptr + // @infer-ignore NULLPTR_DEREFERENCE place_container(f, frames.size() >= 2 ? &frames[frames.size() - 2] : nullptr, std::move(value), false); } @@ -16165,6 +16185,22 @@ class json_sax_dom_parser member = BasicJsonType(std::forward(v)); } + /// whether the values of a container of @a len elements are collected on the stack + static bool collects(const std::size_t len) noexcept + { + return collects(len, std::integral_constant {}); + } + + static bool collects(const std::size_t len, std::true_type /*collect*/) noexcept + { + return len == detail::unknown_size(); + } + + static bool collects(const std::size_t /*len*/, std::false_type /*collect*/) noexcept + { + return false; + } + /// after a container was closed: the one it was in is the one that gets the next value void enter_enclosing_container() { @@ -16184,12 +16220,11 @@ class json_sax_dom_parser template void handle_value(Value&& v) { - BasicJsonType* const stored = store(std::forward(v)); - #if JSON_DIAGNOSTIC_POSITIONS + BasicJsonType* const stored = store(std::forward(v)); diagnostic_positions::set_from_lexer(*stored, m_lexer_ref); #else - static_cast(stored); + store(std::forward(v)); #endif } @@ -16208,7 +16243,7 @@ class json_sax_dom_parser /// the member of the innermost such object that key() added or found last std::size_t member_index = 0; /// the key indexes of the open objects that have one, innermost last - std::vector> key_indexes {}; + std::vector> key_indexes {}; // NOLINT(readability-redundant-member-init) }; struct no_ordered_data_t {}; typename std::conditional::type ordered_data {}; @@ -21785,8 +21820,8 @@ using parser_callback_t = template struct sax_ignores_number_text : std::false_type {}; -template -struct sax_ignores_number_text> : std::true_type {}; +template +struct sax_ignores_number_text> : std::true_type {}; template struct sax_ignores_number_text> : std::true_type {}; @@ -21860,7 +21895,7 @@ class parser } else { - json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); + json_sax_dom_parser sdp(result, allow_exceptions, &m_lexer); // in case of an error, return a discarded value if (!parse_dom(sdp, strict)) @@ -33496,7 +33531,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec friend class ::nlohmann::detail::binary_writer; template friend class ::nlohmann::detail::binary_reader; - template + template friend class ::nlohmann::detail::json_sax_dom_parser; template friend class ::nlohmann::detail::json_sax_dom_binary_parser; diff --git a/single_include/nlohmann/json_view.hpp b/single_include/nlohmann/json_view.hpp index 6220270c7..1b20b598e 100644 --- a/single_include/nlohmann/json_view.hpp +++ b/single_include/nlohmann/json_view.hpp @@ -5770,7 +5770,7 @@ template BasicJsonType materialize(const document_data& d, const node* n) { using string_t = typename BasicJsonType::string_t; - using sax_t = json_sax_dom_parser>; + using sax_t = json_sax_dom_parser, true>; using nav = navigation; // the open containers: their next element (or key of the next member), diff --git a/tests/src/unit-class_parser.cpp b/tests/src/unit-class_parser.cpp index 079e80085..c82cdebc7 100644 --- a/tests/src/unit-class_parser.cpp +++ b/tests/src/unit-class_parser.cpp @@ -2981,3 +2981,52 @@ TEST_CASE("accept() agrees with parse() on floats around the overflow limit") } } } + +namespace +{ +// a DOM parser that reports parse errors without throwing, as in the sax_no_exception example +template +class quiet_dom_parser : public nlohmann::detail::json_sax_dom_parser +{ + public: + explicit quiet_dom_parser(BasicJsonType& j) + : nlohmann::detail::json_sax_dom_parser(j, false) + {} + + bool parse_error(std::size_t /*unused*/, const std::string& /*unused*/, const nlohmann::detail::exception& /*unused*/) + { + return false; + } +}; + +template +std::string read_before_error(const std::string& text) +{ + BasicJsonType j; + quiet_dom_parser sax(j); + CHECK(!BasicJsonType::sax_parse(text, &sax)); + return j.dump(); +} +} // namespace + +TEST_CASE("a json_sax_dom_parser keeps the values read before a parse error") +{ + // a json_sax_dom_parser that a user constructs stores every value in place, so a + // handler that does not throw on a parse error sees what was read before it + const std::vector> cases = + { + {"[1,2,3,]", "[1,2,3]"}, + {"[[1,[2,3],4],[5,", "[[1,[2,3],4],[5]]"}, + {R"({"a":1,"b":[1,2,{"c":)", R"({"a":1,"b":[1,2,{"c":null}]})"}, + {R"({"a":1,"a":)", R"({"a":1})"}, + {"[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,]", "[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]"}, + {R"({"k0":0,"k1":1,"k2":{"x":[true,)", R"({"k0":0,"k1":1,"k2":{"x":[true]}})"}, + {R"([{},[],{"a":[]},x)", R"([{},[],{"a":[]}])"}, + }; + for (const auto& c : cases) + { + CAPTURE(c.first) + CHECK(read_before_error(c.first) == c.second); + CHECK(read_before_error(c.first) == c.second); + } +}