For data that can be NULL, JsonSchemaReflectionTrait::getJsonSchemaForMethod() adds a ['type' => 'null'] option to the schemas in a oneOf.

A oneOf accepts a value only when exactly one of its options matches it. So when the declared schema already accepts NULL, and a ['type' => 'null'] option gets added, NULL matches two options, and the schema rejects it. See comment #2 for details on how oneOf behaves.

A schema can accept NULL in various ways, and each of these incorrectly get a second null option:

  • An enum that lists null
  • A const of null
  • A nested oneOf or anyOf with a null option
  • A schema that accepts NULL without mentioning it, like ['not' => ['type' => 'integer']]

Example:

#[JsonSchema(['enum' => ['a', NULL]])]

// Now:
['oneOf' => [['enum' => ['a', NULL]], ['type' => 'null']]]

// Expected:
['enum' => ['a', NULL]]

Issue fork drupal-3634887

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dries created an issue. See original summary.

dries’s picture

dries commented

I've been trying to figure out what goes wrong.

As shown in the issue summary, core outputs: {"oneOf": [{"enum": ["a", null]}, {"type": "null"}]}.

The schema has two options. The first, {"enum": ["a", null]}, is the one the method declared. The second, {"type": "null"}, is added automatically by core, because the data is optional and so can be NULL.

oneOf means "exactly one of these". A validator checks the value against every option and counts how many match.

Take the value "a". It matches the first option but not the second. That makes for exactly one match, so "a" passes.

Now take NULL. It matches the first option, because the enum includes null. But it also matches the second option. That adds up to two matches, so NULL fails.

To double check, I asked Claude Code to write me a short check, in PHP and in Python, using external validators.

The PHP version uses justinrainbow/json-schema, the validator Drupal Core ships with.

For NULL:

ddev php -r 'require "vendor/autoload.php"; $value = json_decode($argv[2]); $validator = new JsonSchema\Validator(); $validator->validate($value, json_decode($argv[1])); echo $validator->isValid() ? "passes\n" : "fails\n";' '{"oneOf": [{"enum": ["a", null]}, {"type": "null"}]}' 'null'

It prints "fails".

For "a":

ddev php -r 'require "vendor/autoload.php"; $value = json_decode($argv[2]); $validator = new JsonSchema\Validator(); $validator->validate($value, json_decode($argv[1])); echo $validator->isValid() ? "passes\n" : "fails\n";' '{"oneOf": [{"enum": ["a", null]}, {"type": "null"}]}' '"a"'

It prints "passes".

The Python version uses a JSON Schema 2020-12 validator, which is more up-to-date.

For NULL:

uv run --quiet --with jsonschema python -I -c 'import json,sys,jsonschema; print("passes" if jsonschema.Draft202012Validator(json.loads(sys.argv[1])).is_valid(json.loads(sys.argv[2])) else "fails")' '{"oneOf": [{"enum": ["a", null]}, {"type": "null"}]}' 'null'

It prints "fails".

For "a":

uv run --quiet --with jsonschema python -I -c 'import json,sys,jsonschema; print("passes" if jsonschema.Draft202012Validator(json.loads(sys.argv[1])).is_valid(json.loads(sys.argv[2])) else "fails")' '{"oneOf": [{"enum": ["a", null]}, {"type": "null"}]}' '"a"'

It prints "passes".

In short: core added the null option to let NULL through, but adding it is what makes NULL fail.

dries opened merge request !17693

dries’s picture

dries commented
Issue summary: View changes

It turns out the enum is just one example of the problem. Here are some other ways to get a second, incorrect NULL option in the output JSON Schema:

  • A const of null. {"const": null} becomes {"oneOf": [{"const": null}, {"type": "null"}]}.
  • A nested oneOf with a null option. {"oneOf": [{"type": "string"}, {"type": "null"}]} becomes {"oneOf": [{"oneOf": [{"type": "string"}, {"type": "null"}]}, {"type": "null"}]}.
  • A nested anyOf with a null option.
  • And an even trickier one, a schema that accepts NULL without mentioning NULL. {"not": {"type": "integer"}} means "anything except an integer", so it accepts NULL. It becomes {"oneOf": [{"not": {"type": "integer"}}, {"type": "null"}]}.

I've added tests for these cases in the MR. They should fail on main. I've also updated the issue summary based on what I've learned so far.

Knowing these cases, especially the "tricky one", helps us think through the right solution, because we can't just "scan for NULL" ...

dries’s picture

dries commented

I think we should just ask the JSON Schema validator, justinrainbow/json-schema, whether a schema already accepts NULL or not. With all the ways a schema can accept NULL, we kind of have to.

So my proposed fix simply validates NULL against the schema and only adds {"type": "null"} when NULL doesn't pass. Let's see if the tests pass.

dries’s picture

dries commented
Related issues: +#3634179: JSON:API generates invalid JSON Schemas for field types with a single optional property

The Drupal.org test infrastructure seems to be having some issues, so I did some extra manual testing on my localhost:

Declares Before After
An enum with null {"oneOf": [{"enum": ["a", null]}, {"type": "null"}]} {"enum": ["a", null]}
A null const {"oneOf": [{"const": null}, {"type": "null"}]} {"const": null}
A nested oneOf with a null option {"oneOf": [{"oneOf": [{"type": "string"}, {"type": "null"}]}, {"type": "null"}]} {"oneOf": [{"type": "string"}, {"type": "null"}]}
A nested anyOf with a null option {"oneOf": [{"anyOf": [{"type": "string"}, {"type": "null"}]}, {"type": "null"}]} {"anyOf": [{"type": "string"}, {"type": "null"}]}
A not, which accepts NULL without naming it {"oneOf": [{"not": {"type": "integer"}}, {"type": "null"}]} {"not": {"type": "integer"}}

We can maybe look at #3634179: JSON:API generates invalid JSON Schemas for field types with a single optional property next. It looks like the same bug, and we might be able to reuse the new acceptsNull().

ahmad-khalil-imagex’s picture

ahmad-khalil-imagex
He/Him
Primary language English
Location London, Ontario, Canada
commented
Assigned: Unassigned » ahmad-khalil-imagex
ahmad-khalil-imagex’s picture

ahmad-khalil-imagex
He/Him
Primary language English
Location London, Ontario, Canada
commented
Assigned: ahmad-khalil-imagex » Unassigned
Status: Active » Needs review
dries’s picture

dries commented

Thanks for the thorough review, and for the end-to-end check.

I made one change. I put the throw in acceptsNull() back. If there are typos in the schema, or unresolved $refs, it's good to throw an exception so broken schemas don't go unnoticed. I replaced your $ref row with a test that expects the exception. Happy to undo that change if others disagree.

I'll follow up on your other points separately.

dries’s picture

dries commented

On justinrainbow/json-schema 5.x: main requires ^6.5.2. 11.x still allows ^5.2 so `{"const": "a"}` would look like it accepts NULL. I think we're good TBH.

We're tracking the JSON:API field schema bug in #3634179: JSON:API generates invalid JSON Schemas for field types with a single optional property, and we can work on that after this lands as it will likely reuse the new acceptsNull().

The new tests "a type gets the null option", "a required type gets no null option" and "a required repeated schema stays a flat oneOf" look good to me. They pass with or without this fix, so they are slightly out of scope. That said, they are useful tests for common scenarios. I checked and nothing in core seem to assert these today. I think they are worth keeping.

As far as I can tell, this looks good.

ahmad-khalil-imagex’s picture

ahmad-khalil-imagex
He/Him
Primary language English
Location London, Ontario, Canada
commented
Assigned: Unassigned » ahmad-khalil-imagex
ahmad-khalil-imagex’s picture

ahmad-khalil-imagex
He/Him
Primary language English
Location London, Ontario, Canada
commented
Assigned: ahmad-khalil-imagex » Unassigned
dries’s picture

dries commented
Issue summary: View changes
dries’s picture

dries commented

@ahmad-khalil-imagex Yes, please push your test commit.

I updated the issue summary with the new exception. I also updated the MR.

dries’s picture

dries commented
Issue summary: View changes

This all looks good to me. You convinced me on the exception handling. I stand corrected. :)

I feel this is very robust now and most likely RTBC.

I updated the issue summary.

f0ns’s picture

f0ns commented

One thing to look at (minor, not blocking) before RTBC:

acceptsNull() follows $refs. So a nullable schema like ['$ref' => 'https://example.com/s.json'] now makes an HTTP request every time the schema is built. If the host doesn't respond, PHP waits up to 60 seconds by default. A file:// ref reads a file from disk.

Fix: give the validator a URI retriever that refuses every fetch. I tested it: remote refs then fall back to keeping the null option, and internal $defs refs still work.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Reviewed & tested by the community

Asking the validator whether the declared schema already accepts NULL is the right fix. It handles enum, const, nested oneOf/anyOf, and not, and scanning the schema for null can't catch the not case.

If the validator can't read a schema, like a $ref it can't resolve or an unknown type, acceptsNull() returns FALSE. That schema keeps the null option, the same output as before this change. No core data type declares a schema that accepts NULL, so the schemas core generates don't change.

The new rows in nullableSchemaDataProvider() assert the exact schema for optional and required values, for repeated attributes, and for each exception the validator can throw.

#3634179: JSON:API generates invalid JSON Schemas for field types with a single optional property should follow. With the property schema fixed, the outer oneOf in FieldNormalizer::getNormalizationSchema() is what rejects an empty field for a schema that accepts NULL.

Generated with the help of an LLM.