Hide a discarded container's content from the parser callback

When a parser callback rejects an object's or array's start event,
json_sax_dom_callback_parser kept calling it for everything inside
that container anyway: nested keys, values, and the start/end events
of containers below it. This contradicts parser_callback_t's own
documentation, which promises that discarding a container at its
start event also hides its content from the callback.

The same code path also kept a full copy of every key inside such a
discarded container in key_stack until the whole parse finished,
because the early return for values that are not stored skipped the
matching pop. Filtering out a large subtree is the main reason to use
a callback, so this made peak memory during the parse scale with the
size of the very subtree the callback was trying to skip.

Fix start_object(), start_array(), and key() so that a container
whose own start event was discarded, or that is nested inside one, is
never handed to the callback, and no longer pushes onto the key
stacks. A container whose start event was accepted but whose key was
rejected still gets its content reported, as documented ("the
callback is still called for the associated value, but its return
value has no further effect"); only its own bookkeeping is skipped
since it will not be stored.

Fixes #5643.

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

docs/mkdocs/docs/api/basic_json/parser_callback_t.md

@@ -100,3 +100,6 @@ the latter case, it is skipped completely, or replaced by `null` if it is the to
 - Added in version 1.0.0.
 - Fixed in version 3.13.0 to also remove discarded values from a parent object; before, discarding an array or a value
   stored under an object key left a discarded member behind, which made the parse result serialize to invalid JSON.
+- Fixed in version 3.13.0 so that discarding an array or object at its start event also hides its content from the
+  callback, as documented above; before, the callback was still called for the content, and the key of every member of
+  a discarded object was kept in memory until the parse ended.

include/nlohmann/detail/input/json_sax.hpp

@@ -582,8 +582,8 @@ class json_sax_dom_callback_parser
 
     bool start_object(std::size_t len)
     {
-        // check callback for object start
-        const bool keep = callback(static_cast<int>(ref_stack.size()), parse_event_t::object_start, discarded);
+        // check callback for object start; not called inside a discarded container
+        const bool keep = keep_stack.back() && callback(static_cast<int>(ref_stack.size()), parse_event_t::object_start, discarded);
         keep_stack.push_back(keep);
 
         // the key this object will be stored under, read before handle_value()
@@ -619,6 +619,18 @@ class json_sax_dom_callback_parser
 
     bool key(string_t& val)
     {
+        if (!keep_stack.back() || !ref_stack.back())
+        {
+            // the object is not stored: the value of this key is dropped in
+            // handle_value() without touching the key stacks
+            if (keep_stack.back())
+            {
+                BasicJsonType k = BasicJsonType(val);
+                static_cast<void>(callback(static_cast<int>(ref_stack.size()), parse_event_t::key, k));
+            }
+            return true;
+        }
+
         BasicJsonType k = BasicJsonType(val);
 
         // check callback for the key
@@ -704,7 +716,7 @@ class json_sax_dom_callback_parser
 
     bool start_array(std::size_t len)
     {
-        const bool keep = callback(static_cast<int>(ref_stack.size()), parse_event_t::array_start, discarded);
+        const bool keep = keep_stack.back() && callback(static_cast<int>(ref_stack.size()), parse_event_t::array_start, discarded);
         keep_stack.push_back(keep);
 
         // see start_object()

include/nlohmann/detail/recursion_depth_limit.hpp

@@ -19,17 +19,11 @@ namespace detail
 /*!
 @brief the number of nesting levels an operation recurses into
 
-Operations that walk a value (copying, comparing, serializing, hashing, merging,
-...) recurse once
+Operations that walk a value (serializing, hashing, merging, ...) recurse once
 per nesting level, which is fastest, but a value nested deeply enough would
 exhaust the call stack. So they recurse only this many levels deep and finish
 whatever lies below with an explicit stack. All of them share this limit.
 
-Most of them pass the depth down as an argument. The copy constructor and the
-comparison operators cannot, as their signatures are fixed, so they count it
-in basic_json::nesting_depth() instead, a byte per thread; the limit must
-therefore stay below 255.
-
 @sa https://github.com/nlohmann/json/issues/5387
 */
 constexpr std::size_t recursion_depth_limit() noexcept

include/nlohmann/json.hpp

@@ -898,8 +898,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
     }
 
 #ifndef JSON_NO_THREAD_LOCAL
-    // nesting_depth() is a byte and may exceed the limit by one level
-    static_assert(detail::recursion_depth_limit() < 255, "the nesting depth count must fit in a byte");
+    /// the number of levels an operation descends into before it finishes the
+    /// value below it without the call stack
+    static constexpr std::uint8_t nesting_depth_limit()
+    {
+        return 128;
+    }
 
     /*!
     @brief how many levels the operation going on in this thread has descended into
@@ -941,7 +945,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
         static_cast<void>(may_descend);
         return true;
 #else
-        return !may_descend || nesting_depth() >= detail::recursion_depth_limit();
+        return !may_descend || nesting_depth() >= nesting_depth_limit();
 #endif
     }
 
@@ -965,7 +969,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 #ifdef JSON_NO_THREAD_LOCAL
             : m_okay(false)
 #else
-            : m_okay(nesting_depth() < detail::recursion_depth_limit())
+            : m_okay(nesting_depth() < nesting_depth_limit())
 #endif
         {
 #ifndef JSON_NO_THREAD_LOCAL
@@ -1169,7 +1173,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
     The values whose copy has not been created yet are kept on an explicit
     worklist rather than on the call stack. This is only reached for values
-    nested deeper than @ref detail::recursion_depth_limit levels, which is why it copies
+    nested deeper than @ref nesting_depth_limit levels, which is why it copies
     every container by hand instead of letting the container do it: the fast
     ways of doing so would descend into the elements and defeat the purpose.
     */
@@ -1235,7 +1239,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
     Copying a container copies its elements, so a value nested deeply enough
     used to exhaust the call stack. The descent is bounded here: the first
-    @ref detail::recursion_depth_limit levels are copied by the containers themselves, just
+    @ref nesting_depth_limit levels are copied by the containers themselves, just
     as they always were, and anything below that is copied without the call
     stack by @ref copy_iteratively. Copying a value can therefore no longer
     exhaust the stack, however deeply it is nested, just like destroying one
@@ -1373,7 +1377,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
     /*!
     @brief compare @a lhs and @a rhs without descending into them
 
-    Reached once a comparison has descended @ref detail::recursion_depth_limit levels, so
+    Reached once a comparison has descended @ref nesting_depth_limit levels, so
     that comparing values cannot exhaust the call stack however deeply they are
     nested. The two values are walked in lockstep on an explicit stack and
     compared lexicographically, element by element in the order the containers

single_include/nlohmann/json.hpp

@@ -7281,17 +7281,11 @@ namespace detail
 /*!
 @brief the number of nesting levels an operation recurses into
 
-Operations that walk a value (copying, comparing, serializing, hashing, merging,
-...) recurse once
+Operations that walk a value (serializing, hashing, merging, ...) recurse once
 per nesting level, which is fastest, but a value nested deeply enough would
 exhaust the call stack. So they recurse only this many levels deep and finish
 whatever lies below with an explicit stack. All of them share this limit.
 
-Most of them pass the depth down as an argument. The copy constructor and the
-comparison operators cannot, as their signatures are fixed, so they count it
-in basic_json::nesting_depth() instead, a byte per thread; the limit must
-therefore stay below 255.
-
 @sa https://github.com/nlohmann/json/issues/5387
 */
 constexpr std::size_t recursion_depth_limit() noexcept
@@ -12002,8 +11996,8 @@ class json_sax_dom_callback_parser
 
     bool start_object(std::size_t len)
     {
-        // check callback for object start
-        const bool keep = callback(static_cast<int>(ref_stack.size()), parse_event_t::object_start, discarded);
+        // check callback for object start; not called inside a discarded container
+        const bool keep = keep_stack.back() && callback(static_cast<int>(ref_stack.size()), parse_event_t::object_start, discarded);
         keep_stack.push_back(keep);
 
         // the key this object will be stored under, read before handle_value()
@@ -12039,6 +12033,18 @@ class json_sax_dom_callback_parser
 
     bool key(string_t& val)
     {
+        if (!keep_stack.back() || !ref_stack.back())
+        {
+            // the object is not stored: the value of this key is dropped in
+            // handle_value() without touching the key stacks
+            if (keep_stack.back())
+            {
+                BasicJsonType k = BasicJsonType(val);
+                static_cast<void>(callback(static_cast<int>(ref_stack.size()), parse_event_t::key, k));
+            }
+            return true;
+        }
+
         BasicJsonType k = BasicJsonType(val);
 
         // check callback for the key
@@ -12124,7 +12130,7 @@ class json_sax_dom_callback_parser
 
     bool start_array(std::size_t len)
     {
-        const bool keep = callback(static_cast<int>(ref_stack.size()), parse_event_t::array_start, discarded);
+        const bool keep = keep_stack.back() && callback(static_cast<int>(ref_stack.size()), parse_event_t::array_start, discarded);
         keep_stack.push_back(keep);
 
         // see start_object()
@@ -26985,8 +26991,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
     }
 
 #ifndef JSON_NO_THREAD_LOCAL
-    // nesting_depth() is a byte and may exceed the limit by one level
-    static_assert(detail::recursion_depth_limit() < 255, "the nesting depth count must fit in a byte");
+    /// the number of levels an operation descends into before it finishes the
+    /// value below it without the call stack
+    static constexpr std::uint8_t nesting_depth_limit()
+    {
+        return 128;
+    }
 
     /*!
     @brief how many levels the operation going on in this thread has descended into
@@ -27028,7 +27038,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
         static_cast<void>(may_descend);
         return true;
 #else
-        return !may_descend || nesting_depth() >= detail::recursion_depth_limit();
+        return !may_descend || nesting_depth() >= nesting_depth_limit();
 #endif
     }
 
@@ -27052,7 +27062,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 #ifdef JSON_NO_THREAD_LOCAL
             : m_okay(false)
 #else
-            : m_okay(nesting_depth() < detail::recursion_depth_limit())
+            : m_okay(nesting_depth() < nesting_depth_limit())
 #endif
         {
 #ifndef JSON_NO_THREAD_LOCAL
@@ -27256,7 +27266,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
     The values whose copy has not been created yet are kept on an explicit
     worklist rather than on the call stack. This is only reached for values
-    nested deeper than @ref detail::recursion_depth_limit levels, which is why it copies
+    nested deeper than @ref nesting_depth_limit levels, which is why it copies
     every container by hand instead of letting the container do it: the fast
     ways of doing so would descend into the elements and defeat the purpose.
     */
@@ -27322,7 +27332,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
     Copying a container copies its elements, so a value nested deeply enough
     used to exhaust the call stack. The descent is bounded here: the first
-    @ref detail::recursion_depth_limit levels are copied by the containers themselves, just
+    @ref nesting_depth_limit levels are copied by the containers themselves, just
     as they always were, and anything below that is copied without the call
     stack by @ref copy_iteratively. Copying a value can therefore no longer
     exhaust the stack, however deeply it is nested, just like destroying one
@@ -27460,7 +27470,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
     /*!
     @brief compare @a lhs and @a rhs without descending into them
 
-    Reached once a comparison has descended @ref detail::recursion_depth_limit levels, so
+    Reached once a comparison has descended @ref nesting_depth_limit levels, so
     that comparing values cannot exhaust the call stack however deeply they are
     nested. The two values are walked in lockstep on an explicit stack and
     compared lexicographically, element by element in the order the containers

tests/src/unit-class_parser.cpp

@@ -1966,6 +1966,102 @@ TEST_CASE("parser class")
             }
         }
 
+        SECTION("no callback for the content of a discarded container (#5643)")
+        {
+            // discarding a container at its start event must also hide
+            // everything inside it from the callback: none of the nested
+            // keys, values, or nested containers' own start/end events may
+            // be reported
+            std::vector<std::string> log;
+            bool first = true;
+            const json j = json::parse(R"({"skip": {"k1": 1, "k2": [2, {"k3": 3}]}, "keep": 1})",
+                                       [&](int depth, json::parse_event_t event, json & parsed)
+            {
+                static const char* const names[] = {"object_start", "object_end", "array_start", "array_end", "key", "value"};
+                log.push_back(std::to_string(depth) + " " + names[static_cast<int>(event)] + " " + parsed.dump());
+
+                if (depth == 1 && event == json::parse_event_t::object_start && first)
+                {
+                    // discard "skip" right at its object_start event
+                    first = false;
+                    return false;
+                }
+                return true;
+            });
+
+            CHECK(log == std::vector<std::string>
+            {
+                "0 object_start <discarded>",
+                "1 key \"skip\"",
+                "1 object_start <discarded>",
+                "1 key \"keep\"",
+                "1 value 1",
+                "0 object_end {\"keep\":1}"
+            });
+            CHECK(j == json({{"keep", 1}}));
+        }
+
+        SECTION("callback still called inside a container whose key was rejected (#5643)")
+        {
+            // rejecting a key does not discard its value's container at the
+            // container's own start event, so the callback is still called
+            // for that container's content; only storing the container
+            // under the rejected key is skipped
+            // (documented for parser_callback_t: "the callback is still
+            // called for the associated value, but its return value has no
+            // further effect")
+            const auto record = [](std::vector<std::string>& log, int depth, json::parse_event_t event, const json & parsed)
+            {
+                static const char* const names[] = {"object_start", "object_end", "array_start", "array_end", "key", "value"};
+                log.push_back(std::to_string(depth) + " " + names[static_cast<int>(event)] + " " + parsed.dump());
+            };
+
+            std::vector<std::string> log_object;
+            const json j_object = json::parse(R"({"skip": {"k1": 1}, "keep": 2})",
+                                              [&](int depth, json::parse_event_t event, json & parsed)
+            {
+                record(log_object, depth, event, parsed);
+                return !(event == json::parse_event_t::key && parsed == json("skip"));
+            });
+
+            CHECK(log_object == std::vector<std::string>
+            {
+                "0 object_start <discarded>",
+                "1 key \"skip\"",
+                "1 object_start <discarded>",
+                "2 key \"k1\"",
+                "2 value 1",
+                "1 key \"keep\"",
+                "1 value 2",
+                "0 object_end {\"keep\":2}"
+            });
+            CHECK(j_object == json({{"keep", 2}}));
+
+            // same for a rejected key whose value is an array rather than an object
+            std::vector<std::string> log_array;
+            const json j_array = json::parse(R"({"skip": [1, {"k1": 2}], "keep": 2})",
+                                             [&](int depth, json::parse_event_t event, json & parsed)
+            {
+                record(log_array, depth, event, parsed);
+                return !(event == json::parse_event_t::key && parsed == json("skip"));
+            });
+
+            CHECK(log_array == std::vector<std::string>
+            {
+                "0 object_start <discarded>",
+                "1 key \"skip\"",
+                "1 array_start <discarded>",
+                "2 value 1",
+                "2 object_start <discarded>",
+                "3 key \"k1\"",
+                "3 value 2",
+                "1 key \"keep\"",
+                "1 value 2",
+                "0 object_end {\"keep\":2}"
+            });
+            CHECK(j_array == json({{"keep", 2}}));
+        }
+
         SECTION("special cases")
         {
             // the following test cases cover the situation in which an empty