Skip to content

Fix crash when validating null values in objects with additionalProperties #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
orgads wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix crash when validating null values in objects with additionalProperties #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/json-validator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,6 @@ class type_schema : public schema
else_->validate(ptr, instance, patch, e);
}
}
if (instance.is_null()) {
patch.add(nlohmann::json::json_pointer{}, default_value_);
}
}

protected:
Expand Down
4 changes: 4 additions & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,7 @@ add_test(NAME issue-255-error-message-limit-precision COMMAND issue-255-error-me
add_executable(issue-105-verbose-combination-errors issue-105-verbose-combination-errors.cpp)
target_link_libraries(issue-105-verbose-combination-errors nlohmann_json_schema_validator)
add_test(NAME issue-105-verbose-combination-errors COMMAND issue-105-verbose-combination-errors)

add_executable(issue-null-values-in-object issue-null-values-in-object.cpp)
target_link_libraries(issue-null-values-in-object nlohmann_json_schema_validator)
add_test(NAME issue-null-values-in-object COMMAND issue-null-values-in-object)
33 changes: 33 additions & 0 deletions test/issue-null-values-in-object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include <iostream>
#include <nlohmann/json-schema.hpp>

using nlohmann::json;
using nlohmann::json_schema::json_validator;

// Test: object with additionalProperties containing null value should validate
// additionalProperties causes traversal into the object where null value triggers the bug
static const json schema = R"(
{
"properties": {
"x": {"type": "object", "additionalProperties": {}}
Copy link
Owner

@pboettch pboettch Dec 24, 2025

Choose a reason for hiding this comment

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

additionalProperties is defaulting to be an empty schema. So can be removed.

Copy link
Author

@orgads orgads Dec 24, 2025

Choose a reason for hiding this comment

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

I'm not sure why, but if I remove this, the test succeeds even without the fix.

Copy link
Owner

@pboettch pboettch Dec 24, 2025

Choose a reason for hiding this comment

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

Empty schema is seen as maybe seen as false. So additionalProperties are not allowed.

Not sure whether this is expected.

Copy link
Owner

@pboettch pboettch Dec 24, 2025

Choose a reason for hiding this comment

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

Definitely something wrong here, additionalProperties: {} should behave the same as if omitted.

Copy link
Author

@orgads orgads Dec 24, 2025

Choose a reason for hiding this comment

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

🤷‍♂️

Without the fix:

  • Without additionalProperties, the test passes
  • With additionalProperties, I get this:
ctest -R issue-null --output-on-failure
Test project /home/orgads/Projects/json-schema-validator/build
    Start 18: issue-null-values-in-object
1/1 Test #18: issue-null-values-in-object ......Subprocess aborted***Exception:   0.02 sec
terminate called after throwing an instance of 'nlohmann::json_abi_v3_12_0::detail::type_error'
  what():  [json.exception.type_error.305] cannot use operator[] with a string argument with null


0% tests passed, 1 tests failed out of 1

Copy link
Owner

@pboettch pboettch Dec 24, 2025

Choose a reason for hiding this comment

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

It's late, but I read a little bit further: The default-patch handling seems flawed:

		if (instance.is_null()) {
			patch.add(nlohmann::json::json_pointer{}, default_value_);
		}

I don't understand how the, seemingly, global instance patch, can use the root-pointer. Such a patch, when applied, will overwrite everything from the instance.

This is what you see in your test.

The incoherence of your test result regarding additionalProperties (present as {} or omitted) comes from the fact that nested is a null value, and when additionalProperties is {} it enters validation nested, this succeeds, but as it is a null-value a patch is added, with in my eyes a wrong json-path.

I think removing the if as you did, or at least change it:

if (instance.is_null() && default_value_) {
	patch.add(ptr, default_value_);
}

This would still create a patch for a possible default value of the instance is null. So this is wrong.

Actually a default value would need to be added only if the json-path does not exist in the validated instance.

As it is stands, default values patch creation seems flawed...

Not sure what to do, is the original author still available to check and explain? @Finkman

This change 59c9d62 was adding these lines.

}
})"_json;

int main(void)
{
json_validator validator{};
validator.set_root_schema(schema);

json instance = R"({"x": {"nested": null}})"_json;

const auto patch = validator.validate(instance);
const auto result = instance.patch(patch);

// null value should be preserved
if (!result["x"]["nested"].is_null()) {
std::cerr << "Failed: nested should be null, got: " << result["x"]["nested"].dump() << std::endl;
return 1;
}

return 0;
}
Loading