From 19595b5e19098240fa82ba0e85bc9c3a6c674aa7 Mon Sep 17 00:00:00 2001 From: Julian Berman Date: Tue, 28 Jun 2022 10:00:46 -0400 Subject: [PATCH] Gut the meat of the recursiveRef implementation for draft2019 It's buggy, and can lead to blowing up the stack with recursion errors. Done by (redundantly) checking schemas during test runs, even though the upstream test suite 'guarantees' they're valid. (Given that it uses this implementation to make that guarantee, bugs like the above went unnoticed.) Closes: #847 --- jsonschema/_legacy_validators.py | 6 +++++- jsonschema/tests/_suite.py | 1 + jsonschema/tests/test_jsonschema_test_suite.py | 12 ++++++++++++ jsonschema/tests/test_validators.py | 2 +- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/jsonschema/_legacy_validators.py b/jsonschema/_legacy_validators.py index c8eff2cc5..5be628fe7 100644 --- a/jsonschema/_legacy_validators.py +++ b/jsonschema/_legacy_validators.py @@ -221,4 +221,8 @@ def recursiveRef(validator, recursiveRef, instance, schema): fragment = recursiveRef.lstrip("#") subschema = validator.resolver.resolve_fragment(target, fragment) - yield from validator.descend(instance, subschema) + # FIXME: This is gutted (and not calling .descend) because it can trigger + # recursion errors, so there's a bug here. Re-enable the tests to + # see it. + subschema + return [] diff --git a/jsonschema/tests/_suite.py b/jsonschema/tests/_suite.py index 870304dbe..ff3e3af49 100644 --- a/jsonschema/tests/_suite.py +++ b/jsonschema/tests/_suite.py @@ -197,6 +197,7 @@ def fn(this): return unittest.skipIf(reason is not None, reason)(fn) def validate(self, Validator, **kwargs): + Validator.check_schema(self.schema) resolver = jsonschema.RefResolver.from_schema( schema=self.schema, store=self._remotes, diff --git a/jsonschema/tests/test_jsonschema_test_suite.py b/jsonschema/tests/test_jsonschema_test_suite.py index e428cc38a..1a5bd7a40 100644 --- a/jsonschema/tests/test_jsonschema_test_suite.py +++ b/jsonschema/tests/test_jsonschema_test_suite.py @@ -360,6 +360,18 @@ def leap_second(test): message="dynamicRef support isn't working yet.", subject="recursiveRef", )(test) + or skip( + message="These tests depends on dynamicRef working.", + subject="id", + case_description=( + "Invalid use of fragments in location-independent $id" + ), + )(test) + or skip( + message="These tests depends on dynamicRef working.", + subject="defs", + description="invalid definition schema", + )(test) or skip( message="These tests depends on dynamicRef working.", subject="anchor", diff --git a/jsonschema/tests/test_validators.py b/jsonschema/tests/test_validators.py index 3aabd3370..4167435f3 100644 --- a/jsonschema/tests/test_validators.py +++ b/jsonschema/tests/test_validators.py @@ -1410,7 +1410,7 @@ class MetaSchemaTestsMixin(object): # TODO: These all belong upstream def test_invalid_properties(self): with self.assertRaises(exceptions.SchemaError): - self.Validator.check_schema({"properties": {"test": object()}}) + self.Validator.check_schema({"properties": 12}) def test_minItems_invalid_string(self): with self.assertRaises(exceptions.SchemaError):