Fix nullable allOf ref type check

[tomakehurst]

Fixes issue where a nullable (OpenAPI dialect) on an element containing allOf isn't honoured when a null value is passed.

Closes #1278

[Copilot]

Pull request overview

This PR fixes OpenAPI 3.0 nullable: true handling when nullable is applied to a schema composed via allOf that only contains a $ref, ensuring null values are accepted as intended (per issue #1278).

Changes:

• Update nullable type-check logic to detect nullable: true across $ref evaluation context (not just lexical parentage).
• Add an OpenAPI 3.0 regression test covering nullable + allOf + $ref.
• Add a Draft-07 test asserting nullable remains ignored outside nullable-enabled dialects.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/main/java/com/networknt/schema/utils/JsonNodeTypes.java	Enhances nullable detection for null type mismatches by walking the evaluation schema stack to account for $ref caching/parentage.
src/test/java/com/networknt/schema/oas/OpenApi30Test.java	Adds regression test reproducing issue #1278 (nullable on allOf with $ref should allow null).
src/test/java/com/networknt/schema/TypeValidatorTest.java	Adds a Draft-07 test confirming nullable is not honored when the dialect does not enable the nullable keyword.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[stevehu]

PR review: Fix nullable allOf ref type check

Risk: HIGH — request changes

Finding

1. [High] The change incorrectly honors nullable beside a direct $ref.

   In isNullableAncestor
   (

   json-schema-validator/src/main/java/com/networknt/schema/utils/JsonNodeTypes.java

   Lines 95 to 126 in 4307fda

   	private static boolean isNullableAncestor(Schema schema, ExecutionContext executionContext) {
   	Schema current = schema;
   	while (current != null) {
   	if (isNodeNullable(current.getSchemaNode())) {
   	return true;
   	}
   	Schema parentSchema = current.getParentSchema();
   	if (parentSchema != null && isNodeNullable(parentSchema.getSchemaNode())) {
   	return true;
   	}
   	current = findReferencingSchema(current, executionContext);
   	}
   	return false;
   	}
   	/**
   	* Finds the schema that referenced the given schema through {@code $ref}, if
   	* any, by looking at the schema evaluated immediately before it on the
   	* dynamic evaluation stack.
   	*
   	* @param schema the schema that may have been reached through {@code $ref}
   	* @param executionContext the execution context
   	* @return the referencing schema, or null if none is found
   	*/
   	private static Schema findReferencingSchema(Schema schema, ExecutionContext executionContext) {
   	Iterator<Schema> ancestors = executionContext.getEvaluationSchema().descendingIterator();
   	while (ancestors.hasNext()) {
   	if (ancestors.next() == schema) {
   	if (ancestors.hasNext()) {
   	Schema candidate = ancestors.next();
   	if (candidate.getSchemaNode().get(REF) != null) {
   	return candidate;

   ), traversal returns
   the referencing $ref schema and then checks that schema’s own nullable property. Consequently, this now accepts null:

   {
   "$ref": "#/components/schemas/Money",
   "nullable": true
   }

   OpenAPI 3.0 requires additional properties on a Reference Object to be ignored, including nullable. The validator already implements that rule by discarding $ref siblings in
   Schema.read(). OpenAPI 3.0.3 Reference Object specification (https://spec.openapis.org/oas/v3.0.3.html#reference-object)

   I reproduced the behavioral regression:

   • Base a9c9638: correctly reports one type error.
   • PR head 4307fda: incorrectly reports zero errors.
   • The intended nullable + allOf + $ref case still passes.

   The traversal should inspect the lexical parent of a $ref wrapper—the allOf-owning Schema Object—without honoring nullable on the Reference Object itself. Add a regression test proving
   direct $ref siblings remain ignored.

Verification

• mvn -q test: passed on PR head.
• Added focused cached-reference and chained-$ref probes: passed.
• Direct $ref sibling regression probe: failed as described.
• The linked Copilot review (Fix nullable allOf ref type check #1279 (review)) generated no comments and missed this behavior change.

Recommendation: REQUEST CHANGES before merge.

[stevehu]

Re-review: PR #1279

No blocking findings. Risk: MEDIUM. Recommendation: APPROVE.

The new commit 5d3d506 resolves the previous regression correctly:

• Direct "$ref" + "nullable": true now remains rejected because the Reference Object’s own nullable is skipped.
• "nullable": true on the allOf-owning schema still accepts null.
• The contributor added a focused regression test that also asserts the resulting error is specifically a type error.
• OpenAPI 3.1 remains unaffected.

Verification on the current head:

• Full mvn -q test: passed.
• Cached target used from nullable and non-nullable reference sites: passed.
• Multiple chained $ref schemas: passed.
• Direct $ref sibling through a chained reference: correctly rejected.
• OpenAPI 3.1 null union and ignored legacy nullable: passed.
• GitNexus reports medium blast radius because JsonNodeTypes is shared validation infrastructure, but the changed branch remains gated by the nullable-enabled dialect.

One non-blocking cleanup note: OpenApi30Test.java was normalized from CRLF to LF, making the diff look much larger than its functional change. That does not affect correctness.

GitHub currently reports the PR (#1279) as mergeable, although no remote status checks are listed.

[tomakehurst]

Thanks!

[tomakehurst]

Should I open a 2.x PR also?

We're still on v2, so it'd be great to have the fix there too.

[stevehu]

Yes. Please.