Skip to content

remove discarded array from parent object in end_array - #5342

Open
Angadi56 wants to merge 1 commit into
nlohmann:developfrom
Angadi56:callback-discarded-array-object
Open

remove discarded array from parent object in end_array#5342
Angadi56 wants to merge 1 commit into
nlohmann:developfrom
Angadi56:callback-discarded-array-object

Conversation

@Angadi56

Copy link
Copy Markdown
Contributor

json::parse with a callback drops a container when the callback returns false, and the parser then takes that value back out of whatever it was being built into, so that, as parser_callback_t documents, the parser behaves as if the discarded value was never read. end_object() does this for any structured parent, but end_array() only handles an array parent, so an array discarded under an object key is never removed and stays behind as a discarded member. The result is that parse hands back a value containing a discarded entry and dump() then prints {"a":<discarded>,"b":3}, which is no longer valid JSON. I noticed it while filling a gap in the callback tests: there was coverage for an object inside an array but none for an array inside an object, and that turned out to be the one combination that was broken. The fix gives end_array() the object case end_object() already has, and keeps the existing pop_back() for array parents so filtering a long array stays linear instead of becoming a scan per element. It also has to run when the array was rejected at array_start, since nothing was stored in that case and only the placeholder written by the key event is left, so the condition covers both.

  • The changes are described in detail, both the what and why.
  • If applicable, an existing issue is referenced.
  • The Code coverage remained at 100%. A test case for every new line of code.
  • If applicable, the documentation is updated.
  • The source code is amalgamated by running make amalgamate.

Read the Contribution Guidelines for detailed information.

Signed-off-by: Angadi Yashaswini <angadi@digiscrypt.com>

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Not sure since when this is broken - good catch!

One thing worth flagging: there's a sibling bug in the same area that this PR doesn't cover. Rejecting a plain scalar's value event (not an array) when it's nested under an object key leaves the exact same kind of <discarded> placeholder behind, e.g.:

auto j = json::parse(R"({"foo": 2, "bar": 3})",
    [](int, json::parse_event_t e, const json& v) {
        return !(e == json::parse_event_t::value && v == json(2));
    });
// j.dump() == {"bar":3,"foo":<discarded>}   -- same symptom as the array case

This happens because handle_value() returns early (return {false, nullptr};) when a scalar's own callback rejects it, before it ever reaches the code that would clean up the placeholder key() wrote — scalars don't have an end_* event to hook a fix into the way arrays/objects do.
Would you be open to extending this fix to cover that case too, so end_array/end_object/handle_value are all consistent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants