forked from weaselab/weaseljson
schemagen: reject required keys with no properties entry
A `required` entry without a matching `properties` key made 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. Since no seen bit is emitted for such keys, the
generated parser silently accepted every document instead of rejecting
them as the README contract requires ("missing required property").
Follow the pattern from issue #55 and raise GenError at generation time
when set(required) is not a subset of properties.keys(), for root,
nested, and $defs object schemas alike. Update the README's
not-supported list accordingly.
Closes #63
This commit is contained in:
@@ -62,7 +62,8 @@ into the result, so it is non-movable.
|
||||
`oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`,
|
||||
`patternProperties`, `additionalProperties` absent or set to `true`,
|
||||
`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
|
||||
`["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)
|
||||
|
||||
|
||||
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):
|
||||
"""Regression tests for issue #33: cyclic array $ref targets."""
|
||||
|
||||
|
||||
@@ -432,6 +432,22 @@ class Builder:
|
||||
)
|
||||
required = set(node.get("required", []))
|
||||
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()
|
||||
for key, sub in props.items():
|
||||
ty, nullable = self._unpack(self.build_type(sub, key))
|
||||
|
||||
Reference in New Issue
Block a user