Skip resolving recursive references in resolve_all_refs

Author: Stranger6667

src/hypothesis_jsonschema/_canonicalise.py

@@ -18,6 +18,7 @@
 import re
 from copy import deepcopy
 from typing import Any, Dict, List, NoReturn, Optional, Tuple, Union
+from urllib.parse import urljoin
 
 import jsonschema
 from hypothesis.errors import InvalidArgument
@@ -559,16 +560,25 @@ def resolve_remote(self, uri: str) -> NoReturn:
         )
 
 
+def is_recursive_reference(reference: str, resolver: LocalResolver) -> bool:
+    """Detect if the given reference is recursive."""
+    # Special case: a reference to the schema's root is always recursive
+    if reference == "#":
+        return True
+    # During reference resolving the scope might go to external schemas. `hypothesis-jsonschema` does not support
+    # schemas behind remote references, but the underlying `jsonschema` library includes meta schemas for
+    # different JSON Schema drafts that are available transparently, and they count as external schemas in this context.
+    # For this reason we need to check the reference relatively to the base uri.
+    full_reference = urljoin(resolver.base_uri, reference)
+    # If a fully-qualified reference is in the resolution stack, then we encounter it for the second time.
+    # Therefore it is a recursive reference.
+    return full_reference in resolver._scopes_stack
+
+
 def resolve_all_refs(
     schema: Union[bool, Schema], *, resolver: LocalResolver = None
 ) -> Schema:
-    """
-    Resolve all references in the given schema.
-
-    This handles nested definitions, but not recursive definitions.
-    The latter require special handling to convert to strategies and are much
-    less common, so we just ignore them (and error out) for now.
-    """
+    """Resolve all non-recursive references in the given schema."""
     if isinstance(schema, bool):
         return canonicalish(schema)
     assert isinstance(schema, dict), schema
@@ -580,35 +590,41 @@ def resolve_all_refs(
         )
 
     if "$ref" in schema:
-        s = dict(schema)
-        ref = s.pop("$ref")
-        with resolver.resolving(ref) as got:
-            if s == {}:
-                return resolve_all_refs(got, resolver=resolver)
-            m = merged([s, got])
-            if m is None:  # pragma: no cover
-                msg = f"$ref:{ref!r} had incompatible base schema {s!r}"
-                raise HypothesisRefResolutionError(msg)
-            return resolve_all_refs(m, resolver=resolver)
-    assert "$ref" not in schema
+        # Recursive references are skipped to avoid infinite recursion.
+        if not is_recursive_reference(schema["$ref"], resolver):
+            s = dict(schema)
+            ref = s.pop("$ref")
+            with resolver.resolving(ref) as got:
+                if s == {}:
+                    return resolve_all_refs(deepcopy(got), resolver=resolver)
+                m = merged([s, got])
+                if m is None:  # pragma: no cover
+                    msg = f"$ref:{ref!r} had incompatible base schema {s!r}"
+                    raise HypothesisRefResolutionError(msg)
+                # `deepcopy` is not needed, because, the schemas are copied inside the `merged` call above
+                return resolve_all_refs(m, resolver=resolver)
 
     for key in SCHEMA_KEYS:
         val = schema.get(key, False)
         if isinstance(val, list):
             schema[key] = [
-                resolve_all_refs(v, resolver=resolver) if isinstance(v, dict) else v
+                resolve_all_refs(deepcopy(v), resolver=resolver)
+                if isinstance(v, dict)
+                else v
                 for v in val
             ]
         elif isinstance(val, dict):
-            schema[key] = resolve_all_refs(val, resolver=resolver)
+            schema[key] = resolve_all_refs(deepcopy(val), resolver=resolver)
         else:
             assert isinstance(val, bool)
     for key in SCHEMA_OBJECT_KEYS:  # values are keys-to-schema-dicts, not schemas
         if key in schema:
             subschema = schema[key]
             assert isinstance(subschema, dict)
             schema[key] = {
-                k: resolve_all_refs(v, resolver=resolver) if isinstance(v, dict) else v
+                k: resolve_all_refs(deepcopy(v), resolver=resolver)
+                if isinstance(v, dict)
+                else v
                 for k, v in subschema.items()
             }
     assert isinstance(schema, dict)

tests/test_canonicalise.py

@@ -522,3 +522,133 @@ def test_validators_use_proper_draft():
     }
     cc = canonicalish(schema)
     jsonschema.validators.validator_for(cc).check_schema(cc)
+
+
+# Reference to itself
+ROOT_REFERENCE = {"$ref": "#"}
+# One extra nesting level
+NESTED = {"not": {"$ref": "#/not"}}
+# The same as above, but includes "$id".
+NESTED_WITH_ID = {
+    "not": {"$ref": "#/not"},
+    "$id": "http://json-schema.org/draft-07/schema#",
+}
+SELF_REFERENTIAL = {"foo": {"$ref": "#foo"}, "not": {"$ref": "#foo"}}
+
+
+@pytest.mark.parametrize(
+    "schema, expected",
+    (
+        (ROOT_REFERENCE, ROOT_REFERENCE),
+        (NESTED, NESTED),
+        (NESTED_WITH_ID, NESTED_WITH_ID),
+        # "foo" content should be inlined as is, because "#" is recursive (special case)
+        (
+            {"foo": {"$ref": "#"}, "not": {"$ref": "#foo"}},
+            {"foo": {"$ref": "#"}, "not": {"$ref": "#"}},
+        ),
+        # "foo" content should be inlined as is, because it points to itself
+        (
+            SELF_REFERENTIAL,
+            SELF_REFERENTIAL,
+        ),
+        # The same as above, but with one extra nesting level
+        (
+            {"foo": {"not": {"$ref": "#foo"}}, "not": {"$ref": "#foo"}},
+            # 1. We start from resolving "$ref" in "not"
+            # 2. at this point we don't know this path is recursive, so we follow to "foo"
+            # 3. inside "foo" we found a reference to "foo", which means it is recursive
+            {"foo": {"not": {"$ref": "#foo"}}, "not": {"not": {"$ref": "#foo"}}},
+        ),
+        # Circular reference between two schemas
+        (
+            {"foo": {"$ref": "#bar"}, "bar": {"$ref": "#foo"}, "not": {"$ref": "#foo"}},
+            # 1. We start in "not" and follow to "foo"
+            # 2. In "foo" we follow to "bar"
+            # 3. Here we see a reference to previously seen scope, which means it is a recursive path
+            # We take the schema where we stop and inline it to the starting point (therefore it is `{"$ref": "#foo"}`)
+            {"foo": {"$ref": "#bar"}, "bar": {"$ref": "#foo"}, "not": {"$ref": "#foo"}},
+        ),
+    ),
+)
+def test_skip_recursive_references_simple_schemas(schema, expected):
+    # When there is a recursive reference, it should not be resolved
+    assert resolve_all_refs(schema) == expected
+
+
+@pytest.mark.parametrize(
+    "schema, resolved",
+    (
+        # NOTE. The `resolved` fixture does not include "definitions" to save visual space here, but it is extended
+        # with it in the test body.
+        # The reference target is behind two references, that share the same definition path. Not a recursive reference
+        (
+            {
+                "definitions": {
+                    "properties": {
+                        "foo": {"type": "string"},
+                        "bar": {"$ref": "#/definitions/properties/foo"},
+                    },
+                },
+                "not": {"$ref": "#/definitions/properties/bar"},
+            },
+            {
+                "not": {"type": "string"},
+            },
+        ),
+        # Here we need to resolve multiple references while being on the same resolution scope:
+        # "#/definitions/foo" contains two references
+        (
+            {
+                "definitions": {
+                    "foo": {
+                        "properties": {
+                            "bar": {"$ref": "#/definitions/spam"},
+                            "baz": {"$ref": "#/definitions/spam"},
+                        }
+                    },
+                    "spam": {"type": "string"},
+                },
+                "properties": {"foo": {"$ref": "#/definitions/foo"}},
+            },
+            {
+                "properties": {
+                    "foo": {
+                        "properties": {
+                            "bar": {"type": "string"},
+                            "baz": {"type": "string"},
+                        }
+                    }
+                },
+            },
+        ),
+        # Similar to the one above, but recursive
+        (
+            {
+                "definitions": {
+                    "foo": {
+                        "properties": {
+                            "bar": {"$ref": "#/definitions/spam"},
+                            "baz": {"$ref": "#/definitions/spam"},
+                        }
+                    },
+                    "spam": {"$ref": "#/definitions/foo"},
+                },
+                "properties": {"foo": {"$ref": "#/definitions/foo"}},
+            },
+            {
+                "properties": {
+                    "foo": {
+                        "properties": {
+                            "bar": {"$ref": "#/definitions/foo"},
+                            "baz": {"$ref": "#/definitions/foo"},
+                        }
+                    }
+                },
+            },
+        ),
+    ),
+)
+def test_skip_recursive_references_complex_schemas(schema, resolved):
+    resolved["definitions"] = schema["definitions"]
+    assert resolve_all_refs(schema) == resolved