Skip to content

Commit 6af9256

Browse files
committed
fix schema validator skipping object/array checks on union types
the type gate handled a list like ["object","null"] fine but the required/properties and items checks only ran when type was exactly "object"/"array", so a union-typed schema silently passed malformed args straight through to the capability handler. gate the structural checks on the value's actual type plus union membership instead.
1 parent 81dedb7 commit 6af9256

2 files changed

Lines changed: 84 additions & 19 deletions

File tree

tests/test_validate_union_types.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""Union-type ("type": ["object", "null"]) structural validation.
2+
3+
The validator advertises that `type` may be a list of types. The primitive
4+
type gate handled that, but the structural checks (required/properties for
5+
objects, items for arrays) only fired for a bare "object"/"array" string, so
6+
a union-typed schema silently skipped them and malformed args slipped through
7+
to the capability handler. These cases pin the union path.
8+
"""
9+
10+
from zhub.validate import validate
11+
12+
13+
def test_union_object_reports_missing_required():
14+
schema = {
15+
"type": ["object", "null"],
16+
"required": ["city"],
17+
"properties": {"city": {"type": "string"}},
18+
}
19+
errs = validate({}, schema)
20+
assert any("city" in e for e in errs), errs
21+
22+
23+
def test_union_object_reports_bad_nested_type():
24+
schema = {
25+
"type": ["object", "null"],
26+
"required": ["city"],
27+
"properties": {"city": {"type": "string"}},
28+
}
29+
errs = validate({"city": 42}, schema)
30+
assert any("city" in e and "string" in e for e in errs), errs
31+
32+
33+
def test_union_object_accepts_valid_and_null():
34+
schema = {
35+
"type": ["object", "null"],
36+
"required": ["city"],
37+
"properties": {"city": {"type": "string"}},
38+
}
39+
assert validate({"city": "x"}, schema) == []
40+
# null is a permitted member of the union; it has no properties to check
41+
assert validate(None, schema) == []
42+
43+
44+
def test_union_array_reports_bad_items():
45+
schema = {"type": ["array", "null"], "items": {"type": "string"}}
46+
errs = validate([1, 2], schema)
47+
assert len(errs) == 2, errs
48+
assert validate(["a", "b"], schema) == []
49+
50+
51+
def test_single_type_and_untyped_unchanged():
52+
# regression guard for the non-union paths
53+
assert validate({}, {"type": "object", "required": ["city"]}) == [
54+
"<root>: missing required field 'city'"
55+
]
56+
assert validate("x", {"type": "string"}) == []
57+
assert validate(5, {"type": "string"}) == ["<root>: expected string, got integer"]
58+
# untyped schema still infers structure from the value
59+
assert validate({"city": 42}, {"properties": {"city": {"type": "string"}}}) == [
60+
"city: expected string, got integer"
61+
]

zhub/validate.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ def validate(value: Any, schema: dict[str, Any], path: str = "") -> list[str]:
4242
errors: list[str] = []
4343

4444
expected_type = schema.get("type")
45+
types = None
4546
if expected_type is not None:
4647
types = expected_type if isinstance(expected_type, list) else [expected_type]
4748
if not any(_TYPE_CHECKS.get(t, lambda _v: True)(value) for t in types):
@@ -50,25 +51,28 @@ def validate(value: Any, schema: dict[str, Any], path: str = "") -> list[str]:
5051
errors.append(f"{path or '<root>'}: expected {joined}, got {actual}")
5152
return errors # short-circuit; downstream checks assume the type matches
5253

53-
if expected_type == "object" or (expected_type is None and isinstance(value, dict)):
54-
if isinstance(value, dict):
55-
required = schema.get("required") or []
56-
for field in required:
57-
if field not in value:
58-
errors.append(f"{path or '<root>'}: missing required field '{field}'")
59-
properties = schema.get("properties") or {}
60-
for k, sub_schema in properties.items():
61-
if k in value:
62-
sub_path = f"{path}.{k}" if path else k
63-
errors.extend(validate(value[k], sub_schema, sub_path))
64-
65-
if expected_type == "array" or (expected_type is None and isinstance(value, list)):
66-
if isinstance(value, list):
67-
item_schema = schema.get("items")
68-
if isinstance(item_schema, dict):
69-
for i, item in enumerate(value):
70-
sub_path = f"{path}[{i}]" if path else f"[{i}]"
71-
errors.extend(validate(item, item_schema, sub_path))
54+
# Structural checks run whenever the value is actually a dict/list and the
55+
# schema permits that type. `types is None` covers untyped schemas; the
56+
# `"object"/"array" in types` membership covers both a bare "object" string
57+
# and a union like ["object", "null"] — the list form was silently skipping
58+
# required/properties/items before.
59+
if isinstance(value, dict) and (types is None or "object" in types):
60+
required = schema.get("required") or []
61+
for field in required:
62+
if field not in value:
63+
errors.append(f"{path or '<root>'}: missing required field '{field}'")
64+
properties = schema.get("properties") or {}
65+
for k, sub_schema in properties.items():
66+
if k in value:
67+
sub_path = f"{path}.{k}" if path else k
68+
errors.extend(validate(value[k], sub_schema, sub_path))
69+
70+
if isinstance(value, list) and (types is None or "array" in types):
71+
item_schema = schema.get("items")
72+
if isinstance(item_schema, dict):
73+
for i, item in enumerate(value):
74+
sub_path = f"{path}[{i}]" if path else f"[{i}]"
75+
errors.extend(validate(item, item_schema, sub_path))
7276

7377
return errors
7478

0 commit comments

Comments
 (0)