Always enable metaschema format checking

In the implementation prior to this change, passing
`--disable-formats` would impact not only the "actual" schema
validator, but also the validator built to evaluate the schema against
its metaschema. As a result, `--disable-formats *` and similar would
enable schemas to run which previously should have been caught as
invalid.

Furthermore, the customized format checker which had extensions for
date and time evaluation added was used, and any other customizations
to format checking would implicitly be shared with the metaschema
check.

To resolve, refactor format checker building to allow it to be used
more directly for the metaschema check, and add test cases to confirm
that a bad regex in a `pattern` is always rejected, even when
`--disable-formats regex` or similar is used.

Author: sirosen

src/check_jsonschema/formats/__init__.py

@@ -66,13 +66,10 @@ def make_format_checker(
     if not opts.enabled:
         return None
 
-    # copy the base checker
-    base_checker = get_base_format_checker(schema_dialect)
-    checker = copy.deepcopy(base_checker)
+    # customize around regex checking first
+    checker = format_checker_for_regex_impl(opts.regex_impl)
 
-    # replace the regex check
-    del checker.checkers["regex"]
-    checker.checks("regex")(opts.regex_impl.check_format)
+    # add other custom format checks
     checker.checks("date-time")(validate_rfc3339)
     checker.checks("time")(validate_time)
 
@@ -83,3 +80,18 @@ def make_format_checker(
         del checker.checkers[checkname]
 
     return checker
+
+
+def format_checker_for_regex_impl(
+    regex_impl: RegexImplementation, schema_dialect: str | None = None
+) -> jsonschema.FormatChecker:
+    # convert to a schema-derived format checker, and copy it
+    # for safe modification
+    base_checker = get_base_format_checker(schema_dialect)
+    checker = copy.deepcopy(base_checker)
+
+    # replace the regex check
+    del checker.checkers["regex"]
+    checker.checks("regex")(regex_impl.check_format)
+
+    return checker

src/check_jsonschema/schema_loader/main.py

@@ -9,7 +9,7 @@
 import jsonschema
 
 from ..builtin_schemas import get_builtin_schema
-from ..formats import FormatOptions, make_format_checker
+from ..formats import FormatOptions, format_checker_for_regex_impl, make_format_checker
 from ..parsers import ParserSet
 from ..regex_variants import RegexImplementation
 from ..utils import is_url_ish
@@ -153,10 +153,7 @@ def _get_validator(
     ) -> jsonschema.protocols.Validator:
         retrieval_uri = self.get_schema_retrieval_uri()
         schema = self.get_schema()
-
-        schema_dialect = schema.get("$schema")
-        if schema_dialect is not None and not isinstance(schema_dialect, str):
-            schema_dialect = None
+        schema_dialect = _dialect_of_schema(schema)
 
         # format checker (which may be None)
         format_checker = make_format_checker(format_opts, schema_dialect)
@@ -170,12 +167,8 @@ def _get_validator(
         if self.validator_class is None:
             # get the correct validator class and check the schema under its metaschema
             validator_cls = jsonschema.validators.validator_for(schema)
-            _check_schema(
-                validator_cls,
-                schema,
-                format_checker=format_checker,
-                regex_impl=regex_impl,
-            )
+
+            _check_schema(validator_cls, schema, regex_impl=regex_impl)
         else:
             # for a user-provided validator class, don't check_schema
             # on the grounds that it might *not* be valid but the user wants to use
@@ -206,28 +199,40 @@ def _check_schema(
     validator_cls: type[jsonschema.protocols.Validator],
     schema: dict[str, t.Any],
     *,
-    format_checker: jsonschema.FormatChecker | None,
     regex_impl: RegexImplementation,
 ) -> None:
     """A variant definition of Validator.check_schema which uses the regex
     implementation and format checker specified."""
+    # construct the metaschema validator class (with customized regex impl)
     schema_validator_cls = jsonschema.validators.validator_for(
         validator_cls.META_SCHEMA, default=validator_cls
     )
     schema_validator_cls = _extend_with_pattern_implementation(
         schema_validator_cls, regex_impl
     )
 
-    if format_checker is None:
-        format_checker = schema_validator_cls.FORMAT_CHECKER
+    # construct a specialized format checker (again, customized regex impl)
+    metaschema_dialect = _dialect_of_schema(validator_cls.META_SCHEMA)
+    format_checker = format_checker_for_regex_impl(regex_impl, metaschema_dialect)
 
+    # now, construct and apply the actual validator
     schema_validator = schema_validator_cls(
         validator_cls.META_SCHEMA, format_checker=format_checker
     )
     for error in schema_validator.iter_errors(schema):
         raise jsonschema.exceptions.SchemaError.create_from(error)
 
 
+def _dialect_of_schema(schema: dict[str, t.Any] | bool) -> str | None:
+    if not isinstance(schema, dict):
+        return None
+
+    schema_dialect = schema.get("$schema")
+    if schema_dialect is not None and not isinstance(schema_dialect, str):
+        schema_dialect = None
+    return schema_dialect
+
+
 class BuiltinSchemaLoader(SchemaLoader):
     def __init__(self, schema_name: str, *, base_uri: str | None = None) -> None:
         self.schema_name = schema_name

tests/acceptance/test_invalid_schema_files.py

@@ -1,3 +1,6 @@
+import pytest
+
+
 def test_checker_non_json_schemafile(run_line, tmp_path):
     foo = tmp_path / "foo.json"
     bar = tmp_path / "bar.json"
@@ -31,13 +34,22 @@ def test_checker_invalid_schemafile_scheme(run_line, tmp_path):
     assert "only supports http, https" in res.stderr
 
 
-def test_checker_invalid_schemafile_due_to_bad_regex(run_line, tmp_path):
+@pytest.mark.parametrize(
+    "add_args",
+    [
+        pytest.param([], id="noargs"),
+        # ensure that this works even when regex checking is disabled
+        pytest.param(["--disable-formats", "*"], id="all-formats-disabled"),
+        pytest.param(["--disable-formats", "regex"], id="regex-format-disabled"),
+    ],
+)
+def test_checker_invalid_schemafile_due_to_bad_regex(run_line, tmp_path, add_args):
     foo = tmp_path / "foo.json"
     bar = tmp_path / "bar.json"
     # too many backslash escapes -- not a valid Unicode-mode regex
     foo.write_text(r'{"properties": {"foo": {"pattern": "\\\\p{N}"}}}')
     bar.write_text("{}")
 
-    res = run_line(["check-jsonschema", "--schemafile", str(foo), str(bar)])
+    res = run_line(["check-jsonschema", "--schemafile", str(foo), str(bar), *add_args])
     assert res.exit_code == 1
     assert "schemafile was not valid" in res.stderr