Merge pull request 'schemagen: reject required entries with no matching properties key' (#65) from weaselbot/weaseljson:weaselbot/issue-63 into main
CI / pre-commit (push) Successful in 56s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (push) Successful in 1m2s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (push) Successful in 53s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (push) Successful in 1m43s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (push) Successful in 1m33s
CI / pre-commit (push) Successful in 56s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (push) Successful in 1m2s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (push) Successful in 53s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (push) Successful in 1m43s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (push) Successful in 1m33s
Reviewed-on: #65
This commit was merged in pull request #65.
This commit is contained in:
@@ -62,7 +62,8 @@ into the result, so it is non-movable.
|
|||||||
`oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`,
|
`oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`,
|
||||||
`patternProperties`, `additionalProperties` absent or set to `true`,
|
`patternProperties`, `additionalProperties` absent or set to `true`,
|
||||||
`additionalProperties` with a schema (typed map), `prefixItems` (tuples),
|
`additionalProperties` with a schema (typed map), `prefixItems` (tuples),
|
||||||
`const`,
|
`const`, `required` entries with no matching `properties` key (the schema
|
||||||
|
would be unsatisfiable),
|
||||||
`dependentSchemas`/`dependentRequired`, union `type` lists other than
|
`dependentSchemas`/`dependentRequired`, union `type` lists other than
|
||||||
`["T", "null"]`, non-string enums, and remote (`$ref` to other documents).
|
`["T", "null"]`, non-string enums, and remote (`$ref` to other documents).
|
||||||
|
|
||||||
|
|||||||
@@ -287,6 +287,94 @@ class SchemagenAdditionalPropertiesTest(unittest.TestCase):
|
|||||||
self.assertEqual(rc, 0, msg=stderr)
|
self.assertEqual(rc, 0, msg=stderr)
|
||||||
|
|
||||||
|
|
||||||
|
class SchemagenRequiredPropertyTest(unittest.TestCase):
|
||||||
|
"""Regression tests for issue #63: `required` entries with no matching
|
||||||
|
`properties` key must be rejected at generation time. With the mandatory
|
||||||
|
`additionalProperties: false` the schema is unsatisfiable, and the
|
||||||
|
generated parser has no seen bit for such keys, so it would otherwise
|
||||||
|
silently accept documents missing the required key."""
|
||||||
|
|
||||||
|
def run_schemagen(self, schema, args=None):
|
||||||
|
"""Run schemagen on a schema dict. Returns (returncode, stdout, stderr)."""
|
||||||
|
with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as fp:
|
||||||
|
json.dump(schema, fp)
|
||||||
|
schema_path = fp.name
|
||||||
|
try:
|
||||||
|
cmd = [sys.executable, SCRIPT, schema_path]
|
||||||
|
if args:
|
||||||
|
cmd.extend(args)
|
||||||
|
result = subprocess.run(cmd, capture_output=True, text=True, check=False)
|
||||||
|
return result.returncode, result.stdout, result.stderr
|
||||||
|
finally:
|
||||||
|
os.unlink(schema_path)
|
||||||
|
|
||||||
|
def test_required_key_without_properties_entry_rejected(self):
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {"a": {"type": "integer"}},
|
||||||
|
"required": ["b"],
|
||||||
|
}
|
||||||
|
rc, stdout, stderr = self.run_schemagen(schema)
|
||||||
|
self.assertNotEqual(rc, 0)
|
||||||
|
self.assertIn("required key 'b' has no matching properties entry", stderr)
|
||||||
|
|
||||||
|
def test_multiple_dangling_required_keys_reported(self):
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {"a": {"type": "integer"}},
|
||||||
|
"required": ["a", "x", "y"],
|
||||||
|
}
|
||||||
|
rc, stdout, stderr = self.run_schemagen(schema)
|
||||||
|
self.assertNotEqual(rc, 0)
|
||||||
|
self.assertIn(
|
||||||
|
"required keys 'x', 'y' have no matching properties entries",
|
||||||
|
stderr,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_required_without_properties_rejected(self):
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"required": ["any"],
|
||||||
|
}
|
||||||
|
rc, stdout, stderr = self.run_schemagen(schema)
|
||||||
|
self.assertNotEqual(rc, 0)
|
||||||
|
self.assertIn("required key 'any' has no matching properties entry", stderr)
|
||||||
|
|
||||||
|
def test_nested_required_key_without_properties_entry_rejected(self):
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {"child": {"$ref": "#/$defs/Child"}},
|
||||||
|
"$defs": {
|
||||||
|
"Child": {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {"x": {"type": "string"}},
|
||||||
|
"required": ["misspelled"],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
rc, stdout, stderr = self.run_schemagen(schema)
|
||||||
|
self.assertNotEqual(rc, 0)
|
||||||
|
self.assertIn(
|
||||||
|
"required key 'misspelled' has no matching properties entry",
|
||||||
|
stderr,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_required_key_with_properties_entry_accepted(self):
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {"a": {"type": "integer"}, "b": {"type": "integer"}},
|
||||||
|
"required": ["b"],
|
||||||
|
}
|
||||||
|
rc, stdout, stderr = self.run_schemagen(schema)
|
||||||
|
self.assertEqual(rc, 0, msg=stderr)
|
||||||
|
|
||||||
|
|
||||||
class SchemagenCyclicArrayTest(unittest.TestCase):
|
class SchemagenCyclicArrayTest(unittest.TestCase):
|
||||||
"""Regression tests for issue #33: cyclic array $ref targets."""
|
"""Regression tests for issue #33: cyclic array $ref targets."""
|
||||||
|
|
||||||
|
|||||||
@@ -432,6 +432,22 @@ class Builder:
|
|||||||
)
|
)
|
||||||
required = set(node.get("required", []))
|
required = set(node.get("required", []))
|
||||||
props = node.get("properties", {})
|
props = node.get("properties", {})
|
||||||
|
# A `required` entry with no matching `properties` key makes the schema
|
||||||
|
# unsatisfiable: `additionalProperties: false` (the only supported
|
||||||
|
# object mode) rejects any key not listed in `properties`, so the
|
||||||
|
# required key can never be present. No seen bit is emitted for such a
|
||||||
|
# key, so the generated parser would silently accept every document.
|
||||||
|
# Follow the other unsupported constructs and reject at generation time.
|
||||||
|
dangling = sorted(required - set(props))
|
||||||
|
if dangling:
|
||||||
|
quoted = ", ".join(f"'{k}'" for k in dangling)
|
||||||
|
if len(dangling) == 1:
|
||||||
|
raise GenError(
|
||||||
|
f"required key {quoted} has no matching properties entry"
|
||||||
|
)
|
||||||
|
raise GenError(
|
||||||
|
f"required keys {quoted} have no matching properties entries"
|
||||||
|
)
|
||||||
seen_cpp = set()
|
seen_cpp = set()
|
||||||
for key, sub in props.items():
|
for key, sub in props.items():
|
||||||
ty, nullable = self._unpack(self.build_type(sub, key))
|
ty, nullable = self._unpack(self.build_type(sub, key))
|
||||||
|
|||||||
Reference in New Issue
Block a user