From 1555e4742192b961da8880e062ff71d983902b16 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Fri, 19 Jun 2026 09:12:08 -0400 Subject: [PATCH 1/3] Restrict `disallow` on Draft 3 canonicalizer in more cases Signed-off-by: Juan Cruz Viotti --- schemas/canonical-draft3.json | 118 +++++++++++------- .../canonicalizer/disallow_double_negation.h | 26 +++- .../alterschema_canonicalize_draft3_test.cc | 34 ++--- 3 files changed, 108 insertions(+), 70 deletions(-) diff --git a/schemas/canonical-draft3.json b/schemas/canonical-draft3.json index f9a350759..1e0836b55 100644 --- a/schemas/canonical-draft3.json +++ b/schemas/canonical-draft3.json @@ -70,7 +70,56 @@ } } }, - "schema": { + "extends": { + "x-lint-exclude": "simple_properties_identifiers", + "type": "object", + "allOf": [ + { + "$ref": "#/$defs/metadata" + }, + { + "$ref": "#/$defs/core" + } + ], + "required": [ "extends" ], + "properties": { + "extends": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/schema" + } + }, + "required": { + "type": "boolean" + } + }, + "unevaluatedProperties": false + }, + "union": { + "x-lint-exclude": "simple_properties_identifiers", + "type": "object", + "allOf": [ + { + "$ref": "#/$defs/metadata" + }, + { + "$ref": "#/$defs/core" + } + ], + "required": [ "type" ], + "properties": { + "type": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/schema" + } + } + }, + "unevaluatedProperties": false + }, + "leaf": { "anyOf": [ { "const": {} @@ -336,55 +385,19 @@ } }, "unevaluatedProperties": false + } + ] + }, + "schema": { + "anyOf": [ + { + "$ref": "#/$defs/leaf" }, { - "x-lint-exclude": "simple_properties_identifiers", - "type": "object", - "allOf": [ - { - "$ref": "#/$defs/metadata" - }, - { - "$ref": "#/$defs/core" - } - ], - "required": [ "type" ], - "properties": { - "type": { - "type": "array", - "minItems": 1, - "items": { - "$ref": "#/$defs/schema" - } - } - }, - "unevaluatedProperties": false + "$ref": "#/$defs/union" }, { - "x-lint-exclude": "simple_properties_identifiers", - "type": "object", - "allOf": [ - { - "$ref": "#/$defs/metadata" - }, - { - "$ref": "#/$defs/core" - } - ], - "required": [ "extends" ], - "properties": { - "extends": { - "type": "array", - "minItems": 1, - "items": { - "$ref": "#/$defs/schema" - } - }, - "required": { - "type": "boolean" - } - }, - "unevaluatedProperties": false + "$ref": "#/$defs/extends" }, { "x-lint-exclude": "simple_properties_identifiers", @@ -404,7 +417,18 @@ "maxItems": 1, "minItems": 1, "items": { - "$ref": "#/$defs/schema" + "$comment": "TODO: `disallow` should only ever wrap a single-kind leaf. We additionally permit `extends` and `type` unions here as an interim escape hatch: negating a conjunction or disjunction whose wrapper is targeted by a `$ref` cannot be pushed to the leaves, because doing so would dissolve the referenced node. The proper fix is to invert such references so the `disallow` wraps a `$ref` leaf instead, after which both can be dropped from this list", + "anyOf": [ + { + "$ref": "#/$defs/leaf" + }, + { + "$ref": "#/$defs/union" + }, + { + "$ref": "#/$defs/extends" + } + ] } } }, diff --git a/src/alterschema/canonicalizer/disallow_double_negation.h b/src/alterschema/canonicalizer/disallow_double_negation.h index f0bb5b557..d79b9a098 100644 --- a/src/alterschema/canonicalizer/disallow_double_negation.h +++ b/src/alterschema/canonicalizer/disallow_double_negation.h @@ -35,8 +35,13 @@ class DisallowDoubleNegation final : public SchemaTransformRule { ONLY_CONTINUE_IF( wraps_single_constraint(schema, "disallow", walker, vocabularies)); - ONLY_CONTINUE_IF(!frame.has_references_through( - location.pointer, WeakPointer::Token{std::cref(KEYWORD)})); + // The doubly-negated schema relocates up (handled by `rereference`), but + // the inner `disallow` wrapper itself dissolves, so a reference straight at + // it has no new home: bail only in that case + auto wrapper{location.pointer}; + wrapper.push_back(std::cref(KEYWORD)); + wrapper.push_back(static_cast(0)); + ONLY_CONTINUE_IF(!frame.has_references_to(wrapper)); return true; } @@ -55,6 +60,23 @@ class DisallowDoubleNegation final : public SchemaTransformRule { } } + [[nodiscard]] auto rereference(const std::string_view, const Pointer &, + const Pointer &target, + const Pointer ¤t) const + -> Pointer override { + auto old_prefix{current.concat({"disallow", 0, "disallow", 0})}; + while ( + target.starts_with(old_prefix.concat({"disallow", 0, "disallow", 0}))) { + old_prefix = old_prefix.concat({"disallow", 0, "disallow", 0}); + } + + if (!target.starts_with(old_prefix)) { + return target; + } + + return target.rebase(old_prefix, current); + } + private: static auto is_single_negation(const sourcemeta::core::JSON &schema) -> bool { return schema.is_object() && schema.size() == 1 && diff --git a/test/alterschema/alterschema_canonicalize_draft3_test.cc b/test/alterschema/alterschema_canonicalize_draft3_test.cc index 8f703cb59..7f0d0decd 100644 --- a/test/alterschema/alterschema_canonicalize_draft3_test.cc +++ b/test/alterschema/alterschema_canonicalize_draft3_test.cc @@ -1456,7 +1456,7 @@ TEST_F(CanonicalizerDraft3Test, disallow_quadruple_negation) { } TEST_F(CanonicalizerDraft3Test, - disallow_double_negation_with_reference_into_subtree_not_eliminated) { + disallow_double_negation_with_reference_into_subtree_rereferenced) { auto document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-03/schema#", "type": "object", @@ -1490,34 +1490,26 @@ TEST_F(CanonicalizerDraft3Test, "a": { "extends": [ { - "disallow": [ - { - "disallow": [ + "type": "object", + "properties": { + "x": { + "extends": [ { - "type": "object", - "properties": { - "x": { - "extends": [ - { - "type": "string", - "minLength": 0 - } - ], - "required": false - } - }, - "patternProperties": {}, - "additionalProperties": {} + "type": "string", + "minLength": 0 } - ] + ], + "required": false } - ] + }, + "patternProperties": {}, + "additionalProperties": {} } ], "required": false }, "b": { - "$ref": "#/properties/a/extends/0/disallow/0/disallow/0/properties/x" + "$ref": "#/properties/a/extends/0/properties/x" } }, "patternProperties": {}, From fe49b696d6ecc2c03443f43abe227314e20b6bf2 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Fri, 19 Jun 2026 09:43:15 -0400 Subject: [PATCH 2/3] Fix Signed-off-by: Juan Cruz Viotti --- .../canonicalizer/disallow_double_negation.h | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/alterschema/canonicalizer/disallow_double_negation.h b/src/alterschema/canonicalizer/disallow_double_negation.h index d79b9a098..4a6ffa816 100644 --- a/src/alterschema/canonicalizer/disallow_double_negation.h +++ b/src/alterschema/canonicalizer/disallow_double_negation.h @@ -35,13 +35,22 @@ class DisallowDoubleNegation final : public SchemaTransformRule { ONLY_CONTINUE_IF( wraps_single_constraint(schema, "disallow", walker, vocabularies)); - // The doubly-negated schema relocates up (handled by `rereference`), but - // the inner `disallow` wrapper itself dissolves, so a reference straight at - // it has no new home: bail only in that case + // Collapsing the chain dissolves every intermediate `disallow` wrapper, so + // a reference targeting any of them has no valid new home: bail. A + // reference into the surviving innermost schema relocates with it (handled + // by `rereference`) auto wrapper{location.pointer}; - wrapper.push_back(std::cref(KEYWORD)); - wrapper.push_back(static_cast(0)); - ONLY_CONTINUE_IF(!frame.has_references_to(wrapper)); + const sourcemeta::core::JSON *node{&disallow->at(0)}; + while (is_single_negation(*node)) { + wrapper.push_back(std::cref(KEYWORD)); + wrapper.push_back(static_cast(0)); + if (frame.has_references_to(wrapper)) { + return false; + } + + node = &node->at(KEYWORD).at(0); + } + return true; } From 1efde888c01b9728b24a97375b6b196f04ea2782 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Fri, 19 Jun 2026 10:03:57 -0400 Subject: [PATCH 3/3] Fix Signed-off-by: Juan Cruz Viotti --- .../alterschema_canonicalize_draft3_test.cc | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/alterschema/alterschema_canonicalize_draft3_test.cc b/test/alterschema/alterschema_canonicalize_draft3_test.cc index 7f0d0decd..b413be8ca 100644 --- a/test/alterschema/alterschema_canonicalize_draft3_test.cc +++ b/test/alterschema/alterschema_canonicalize_draft3_test.cc @@ -1643,6 +1643,78 @@ TEST_F(CanonicalizerDraft3Test, CANONICALIZE_AND_VALIDATE(document, expected, *compiled_meta_); } +TEST_F(CanonicalizerDraft3Test, + disallow_double_negation_deep_with_reference_into_subtree_rereferenced) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-03/schema#", + "type": "object", + "properties": { + "a": { + "disallow": [ + { + "disallow": [ + { + "disallow": [ + { + "disallow": [ + { + "type": "object", + "properties": { + "x": { + "type": "string" + } + } + } + ] + } + ] + } + ] + } + ] + }, + "b": { + "$ref": "#/properties/a/disallow/0/disallow/0/disallow/0/disallow/0/properties/x" + } + } + })JSON"); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-03/schema#", + "type": "object", + "properties": { + "a": { + "extends": [ + { + "type": "object", + "properties": { + "x": { + "extends": [ + { + "type": "string", + "minLength": 0 + } + ], + "required": false + } + }, + "patternProperties": {}, + "additionalProperties": {} + } + ], + "required": false + }, + "b": { + "$ref": "#/properties/a/extends/0/properties/x" + } + }, + "patternProperties": {}, + "additionalProperties": {} + })JSON"); + + CANONICALIZE_AND_VALIDATE(document, expected, *compiled_meta_); +} + TEST_F(CanonicalizerDraft3Test, disallow_double_negation_over_type_union) { auto document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-03/schema#",