fix(map): reject duplicate YAML keys via a SafeLoader duplicate-check
This commit is contained in:
@@ -12,10 +12,14 @@ from typing import TYPE_CHECKING
|
|||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator
|
from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator
|
||||||
|
from yaml.nodes import ScalarNode
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
|
from collections.abc import Hashable
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from yaml.nodes import MappingNode
|
||||||
|
|
||||||
Pos = tuple[int, int] # (row, col), 0-indexed
|
Pos = tuple[int, int] # (row, col), 0-indexed
|
||||||
|
|
||||||
_ELEVATION_CHARS = frozenset("0123456789.") # digit = height, '.' = ground level
|
_ELEVATION_CHARS = frozenset("0123456789.") # digit = height, '.' = ground level
|
||||||
@@ -38,6 +42,29 @@ class MapValidationError(Exception):
|
|||||||
self.reason = reason
|
self.reason = reason
|
||||||
|
|
||||||
|
|
||||||
|
class _NoDuplicateLoader(yaml.SafeLoader):
|
||||||
|
"""SafeLoader that rejects duplicate mapping keys (PyYAML is last-wins)."""
|
||||||
|
|
||||||
|
|
||||||
|
def _construct_mapping(
|
||||||
|
loader: _NoDuplicateLoader, node: MappingNode, *, deep: bool = False
|
||||||
|
) -> dict[Hashable, object]:
|
||||||
|
seen: set[str] = set()
|
||||||
|
for key_node, _ in node.value:
|
||||||
|
if isinstance(key_node, ScalarNode):
|
||||||
|
key = loader.construct_scalar(key_node)
|
||||||
|
if key in seen:
|
||||||
|
msg = f"duplicate mapping key: {key!r}"
|
||||||
|
raise yaml.constructor.ConstructorError(None, None, msg, key_node.start_mark)
|
||||||
|
seen.add(key)
|
||||||
|
return yaml.constructor.SafeConstructor.construct_mapping(loader, node, deep)
|
||||||
|
|
||||||
|
|
||||||
|
_NoDuplicateLoader.add_constructor(
|
||||||
|
yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _construct_mapping
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TerrainType(BaseModel):
|
class TerrainType(BaseModel):
|
||||||
"""What one square means for movement and sight lines."""
|
"""What one square means for movement and sight lines."""
|
||||||
|
|
||||||
@@ -177,7 +204,13 @@ def load_map(path: Path) -> MapSpec:
|
|||||||
msg = f"cannot read file: {exc}"
|
msg = f"cannot read file: {exc}"
|
||||||
raise MapValidationError(path, msg) from exc
|
raise MapValidationError(path, msg) from exc
|
||||||
try:
|
try:
|
||||||
data: object = yaml.safe_load(text)
|
# _NoDuplicateLoader subclasses SafeLoader: no arbitrary object
|
||||||
|
# instantiation, it only adds duplicate-key rejection (S506 is a
|
||||||
|
# false positive for this loader).
|
||||||
|
data: object = yaml.load(
|
||||||
|
text,
|
||||||
|
Loader=_NoDuplicateLoader, # noqa: S506
|
||||||
|
)
|
||||||
except yaml.YAMLError as exc:
|
except yaml.YAMLError as exc:
|
||||||
msg = f"invalid YAML: {exc}"
|
msg = f"invalid YAML: {exc}"
|
||||||
raise MapValidationError(path, msg) from exc
|
raise MapValidationError(path, msg) from exc
|
||||||
|
|||||||
@@ -67,6 +67,27 @@ def test_minimal_map_without_optional_layers(tmp_path: Path) -> None:
|
|||||||
assert zone_cells(spec, "A") == []
|
assert zone_cells(spec, "A") == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejects_duplicate_yaml_keys(tmp_path: Path) -> None:
|
||||||
|
"""PyYAML is silently last-wins on duplicate mapping keys; a map that
|
||||||
|
declares `name` (or a legend entry) twice must fail, not keep one."""
|
||||||
|
body = _minimal_map() + 'name: "Test bis"\n'
|
||||||
|
with pytest.raises(MapValidationError, match="duplicate mapping key"):
|
||||||
|
load_map(_write(tmp_path, body))
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejects_duplicate_legend_keys(tmp_path: Path) -> None:
|
||||||
|
body = _minimal_map().replace(
|
||||||
|
' ".": { type: floor, move_cost: 1 }\n',
|
||||||
|
' ".": { type: floor, move_cost: 1 }\n ".": { type: wall, move_cost: null }\n',
|
||||||
|
)
|
||||||
|
with pytest.raises(MapValidationError, match="duplicate mapping key"):
|
||||||
|
load_map(_write(tmp_path, body))
|
||||||
|
|
||||||
|
|
||||||
|
def test_accepts_map_without_duplicates(tmp_path: Path) -> None:
|
||||||
|
assert load_map(_write(tmp_path, _minimal_map())).name == "Test"
|
||||||
|
|
||||||
|
|
||||||
def test_rejects_ragged_rows(tmp_path: Path) -> None:
|
def test_rejects_ragged_rows(tmp_path: Path) -> None:
|
||||||
body = _minimal_map().replace(" #.#\n", " #.##\n")
|
body = _minimal_map().replace(" #.#\n", " #.##\n")
|
||||||
with pytest.raises(MapValidationError):
|
with pytest.raises(MapValidationError):
|
||||||
|
|||||||
Reference in New Issue
Block a user