From ab0696df1097da2bb80140b705b4ba4107691891 Mon Sep 17 00:00:00 2001 From: barlowa124 Date: Wed, 7 Oct 2026 17:07:10 -0400 Subject: [PATCH 1/5] fix: validate consolidated metadata listing keys and hierarchy --- src/zarr/core/group.py | 39 ++++++++++++++++ tests/test_metadata/test_consolidated.py | 58 +++++++++++++++++++++++- 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index 7792fa58de..6a2cdb7d2d 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -140,6 +140,41 @@ def _parse_async_node( raise TypeError(f"Unknown node type, got {type(node)}") +def _validate_consolidated_key(key: str) -> None: + """A consolidated metadata key is a node path: it must be non-empty, and every + segment must be a valid node name (non-empty, not ``.``/``..``/``zarr.json``, + not ``__``-prefixed).""" + segments = key.split("/") + if any(not seg or seg in (".", "..", ZARR_JSON) or seg.startswith("__") for seg in segments): + msg = f"Invalid node path in consolidated metadata: {key!r}" + raise MetadataValidationError(msg) + + +def _validate_consolidated_hierarchy( + metadata: dict[str, ArrayV2Metadata | ArrayV3Metadata | GroupMetadata], +) -> None: + """Every non-root listing key must have all of its ancestors present as + group entries. A missing ancestor makes the group unreadable, and a + listing below an array is invalid: array nodes may not have children.""" + for key in metadata: + parts = key.split("/") + for i in range(1, len(parts)): + prefix = "/".join(parts[:i]) + parent = metadata.get(prefix) + if parent is None: + msg = ( + f"Consolidated metadata lists {key!r} without a listing " + f"for its parent {prefix!r}" + ) + raise MetadataValidationError(msg) + if not isinstance(parent, GroupMetadata): + msg = ( + f"Consolidated metadata lists {key!r} below {prefix!r}, " + "which is an array; array nodes may not have children" + ) + raise MetadataValidationError(msg) + + @dataclass(frozen=True) class ConsolidatedMetadata: """ @@ -190,6 +225,9 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli metadata: dict[str, ArrayV2Metadata | ArrayV3Metadata | GroupMetadata] = {} if raw_metadata: + for k in raw_metadata: + _validate_consolidated_key(k) + for k, v in raw_metadata.items(): member = k if path is None else _join_paths([path, k]) if not isinstance(v, dict): @@ -216,6 +254,7 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli else: assert_never(zarr_format) + _validate_consolidated_hierarchy(metadata) cls._flat_to_nested(metadata) return cls(metadata=metadata) diff --git a/tests/test_metadata/test_consolidated.py b/tests/test_metadata/test_consolidated.py index cd0fd92d74..c2a9a68c23 100644 --- a/tests/test_metadata/test_consolidated.py +++ b/tests/test_metadata/test_consolidated.py @@ -22,7 +22,7 @@ from zarr.core.group import ConsolidatedMetadata, GroupMetadata from zarr.core.metadata import ArrayV3Metadata from zarr.core.metadata.v2 import ArrayV2Metadata -from zarr.errors import ZarrUserWarning +from zarr.errors import MetadataValidationError, ZarrUserWarning from zarr.storage import StorePath if TYPE_CHECKING: @@ -901,3 +901,59 @@ async def test_consolidated_metadata_case_differing_siblings(memory_store: Store "obs/study/categories", "obs/study/codes", ] + + +# Regression tests for https://github.com/zarr-developers/zarr-python/issues/4453 +# item 2: consolidated metadata accepted invalid listing keys, a listing with no +# parent entry, and a listing below an array. + +_ARRAY_V3 = { + "zarr_format": 3, + "node_type": "array", + "shape": [1], + "data_type": "int8", + "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [1]}}, + "chunk_key_encoding": {"name": "default"}, + "fill_value": 0, + "codecs": [{"name": "bytes", "configuration": {"endian": "little"}}], +} +_GROUP_V3 = {"zarr_format": 3, "node_type": "group", "attributes": {}} + + +def _consolidated_doc(metadata: dict[str, Any]) -> dict[str, Any]: + return {"kind": "inline", "must_understand": False, "metadata": metadata} + + +@pytest.mark.parametrize( + "key", + ["", ".", "..", "zarr.json", "__a", "a/zarr.json", "a/../b", "a//b"], +) +def test_consolidated_metadata_rejects_invalid_keys(key: str) -> None: + """Listing keys are node paths; every segment must be a valid node name.""" + with pytest.raises(MetadataValidationError, match="Invalid node path"): + ConsolidatedMetadata.from_dict(_consolidated_doc({key: _GROUP_V3})) + + +def test_consolidated_metadata_rejects_child_without_parent() -> None: + """A listing for 'a/b' with no 'a' listing made the root group unopenable.""" + with pytest.raises(MetadataValidationError, match="without a listing"): + ConsolidatedMetadata.from_dict(_consolidated_doc({"a/b": _GROUP_V3})) + + +def test_consolidated_metadata_rejects_node_below_array() -> None: + """Array nodes may not have children; 'arr/child' below an 'arr' array is + invalid.""" + with pytest.raises(MetadataValidationError, match="array"): + ConsolidatedMetadata.from_dict( + _consolidated_doc({"arr": _ARRAY_V3, "arr/child": _GROUP_V3}) + ) + + +def test_consolidated_metadata_accepts_nested_listings() -> None: + consolidated = ConsolidatedMetadata.from_dict( + _consolidated_doc({"a": _GROUP_V3, "a/b": _GROUP_V3, "a/b/c": _ARRAY_V3}) + ) + a = consolidated.metadata["a"] + assert isinstance(a, GroupMetadata) + assert a.consolidated_metadata is not None + assert sorted(a.consolidated_metadata.metadata) == ["b"] From 7b01078c9b49e43ab3d885372474200e333a2c3b Mon Sep 17 00:00:00 2001 From: barlowa124 Date: Wed, 7 Oct 2026 17:08:18 -0400 Subject: [PATCH 2/5] docs: add changelog entry for #4485 --- changes/4485.bugfix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/4485.bugfix.md diff --git a/changes/4485.bugfix.md b/changes/4485.bugfix.md new file mode 100644 index 0000000000..fe80c8c1aa --- /dev/null +++ b/changes/4485.bugfix.md @@ -0,0 +1,5 @@ +Reject invalid consolidated metadata listings at parse time. Listing keys +with empty or reserved path segments (``.``, ``..``, ``zarr.json``, +``__``-prefixed) now raise ``MetadataValidationError``, as do listings +whose parent group entry is missing or that place a node below an array. +Both previously surfaced as ``KeyError`` or left the group unopenable. From fc0e8cffab2b203aeaf9a5ddd8f5f0ff300b96f3 Mon Sep 17 00:00:00 2001 From: Angus Date: Fri, 9 Oct 2026 11:21:39 -0400 Subject: [PATCH 3/5] warn (not raise) on reserved node names in v3 consolidated listings --- changes/4485.bugfix.md | 15 ++++--- src/zarr/core/group.py | 27 +++++++++---- tests/test_metadata/test_consolidated.py | 51 ++++++++++++++++++++++-- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/changes/4485.bugfix.md b/changes/4485.bugfix.md index fe80c8c1aa..0079f4775b 100644 --- a/changes/4485.bugfix.md +++ b/changes/4485.bugfix.md @@ -1,5 +1,10 @@ -Reject invalid consolidated metadata listings at parse time. Listing keys -with empty or reserved path segments (``.``, ``..``, ``zarr.json``, -``__``-prefixed) now raise ``MetadataValidationError``, as do listings -whose parent group entry is missing or that place a node below an array. -Both previously surfaced as ``KeyError`` or left the group unopenable. +Validate consolidated metadata listings at parse time. In v3 listings, +keys containing a reserved node name (`.`, `..`, `zarr.json`, or a +`__`-prefixed segment) now emit a `ZarrDeprecationWarning` instead of +being accepted silently; stores written before these names were rejected +still open. Keys with empty path segments still raise +`MetadataValidationError`, as do listings whose parent group entry is +missing or that place a node below an array; those cases previously +surfaced as `KeyError` or left the group unopenable. The reserved-name +check does not apply to v2 listings, whose specification does not +reserve node names. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index 6a2cdb7d2d..43dc20d901 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -64,6 +64,7 @@ GroupNotFoundError, MetadataValidationError, NodeTypeValidationError, + ZarrDeprecationWarning, ZarrUserWarning, ) from zarr.storage import StoreLike, StorePath @@ -140,14 +141,26 @@ def _parse_async_node( raise TypeError(f"Unknown node type, got {type(node)}") -def _validate_consolidated_key(key: str) -> None: - """A consolidated metadata key is a node path: it must be non-empty, and every - segment must be a valid node name (non-empty, not ``.``/``..``/``zarr.json``, - not ``__``-prefixed).""" +def _validate_consolidated_key(key: str, zarr_format: ZarrFormat) -> None: + """A consolidated metadata key is a node path: every segment must be non-empty. + + The v3 specification also reserves the names ``.``, ``..``, ``zarr.json``, and + every ``__``-prefixed name. Stores written before those names were rejected on + creation still open today, so for now such keys only warn. The v2 specification + does not reserve any node names, so no name checks apply to v2 listings.""" segments = key.split("/") - if any(not seg or seg in (".", "..", ZARR_JSON) or seg.startswith("__") for seg in segments): + if any(not seg for seg in segments): msg = f"Invalid node path in consolidated metadata: {key!r}" raise MetadataValidationError(msg) + if zarr_format == 3 and any( + seg in (".", "..", ZARR_JSON) or seg.startswith("__") for seg in segments + ): + msg = ( + f"Consolidated metadata lists {key!r}, which contains a name reserved " + "by the v3 specification. Rename the node, as reading such listings " + "will become an error." + ) + warnings.warn(msg, category=ZarrDeprecationWarning, stacklevel=1) def _validate_consolidated_hierarchy( @@ -225,9 +238,6 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli metadata: dict[str, ArrayV2Metadata | ArrayV3Metadata | GroupMetadata] = {} if raw_metadata: - for k in raw_metadata: - _validate_consolidated_key(k) - for k, v in raw_metadata.items(): member = k if path is None else _join_paths([path, k]) if not isinstance(v, dict): @@ -237,6 +247,7 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli # zarr_format is present in v2 and v3. zarr_format = parse_zarr_format(v["zarr_format"]) + _validate_consolidated_key(k, zarr_format) if zarr_format == 3: node_type = parse_node_type(v.get("node_type", None)) diff --git a/tests/test_metadata/test_consolidated.py b/tests/test_metadata/test_consolidated.py index c2a9a68c23..c2a53d3479 100644 --- a/tests/test_metadata/test_consolidated.py +++ b/tests/test_metadata/test_consolidated.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import warnings from typing import TYPE_CHECKING, Any import numpy as np @@ -22,7 +23,7 @@ from zarr.core.group import ConsolidatedMetadata, GroupMetadata from zarr.core.metadata import ArrayV3Metadata from zarr.core.metadata.v2 import ArrayV2Metadata -from zarr.errors import MetadataValidationError, ZarrUserWarning +from zarr.errors import MetadataValidationError, ZarrDeprecationWarning, ZarrUserWarning from zarr.storage import StorePath if TYPE_CHECKING: @@ -918,6 +919,7 @@ async def test_consolidated_metadata_case_differing_siblings(memory_store: Store "codecs": [{"name": "bytes", "configuration": {"endian": "little"}}], } _GROUP_V3 = {"zarr_format": 3, "node_type": "group", "attributes": {}} +_GROUP_V2 = {"zarr_format": 2} def _consolidated_doc(metadata: dict[str, Any]) -> dict[str, Any]: @@ -926,14 +928,57 @@ def _consolidated_doc(metadata: dict[str, Any]) -> dict[str, Any]: @pytest.mark.parametrize( "key", - ["", ".", "..", "zarr.json", "__a", "a/zarr.json", "a/../b", "a//b"], + ["", "a//b", "/a", "a/"], ) def test_consolidated_metadata_rejects_invalid_keys(key: str) -> None: - """Listing keys are node paths; every segment must be a valid node name.""" + """Listing keys are node paths; every segment must be non-empty.""" with pytest.raises(MetadataValidationError, match="Invalid node path"): ConsolidatedMetadata.from_dict(_consolidated_doc({key: _GROUP_V3})) +@pytest.mark.parametrize( + "key", + [".", "..", "zarr.json", "__a", "a/zarr.json", "a/__x"], +) +def test_consolidated_metadata_warns_on_reserved_v3_keys(key: str) -> None: + """v3 reserves '.', '..', 'zarr.json', and '__'-prefixed names, but stores + written before the names were rejected still open, so they only warn.""" + with pytest.warns(ZarrDeprecationWarning, match="reserved"): + consolidated = ConsolidatedMetadata.from_dict( + _consolidated_doc({key: _GROUP_V3, "a": _GROUP_V3}) + ) + if "/" in key: + parent, leaf = key.split("/") + nested = consolidated.metadata[parent].consolidated_metadata + assert nested is not None + assert leaf in nested.metadata + else: + assert key in consolidated.metadata + + +def test_consolidated_metadata_reserved_name_below_missing_parent() -> None: + """A reserved name warns before the missing-parent error is raised.""" + with pytest.warns(ZarrDeprecationWarning, match="reserved"): + with pytest.raises(MetadataValidationError, match="without a listing"): + ConsolidatedMetadata.from_dict( + _consolidated_doc({"a": _GROUP_V3, "a/../b": _GROUP_V3}) + ) + + +@pytest.mark.parametrize( + "key", + [".", "..", "zarr.json", "__a"], +) +def test_consolidated_metadata_ignores_reserved_v2_keys(key: str) -> None: + """The v2 specification does not reserve node names.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + consolidated = ConsolidatedMetadata.from_dict( + _consolidated_doc({key: _GROUP_V2}) + ) + assert key in consolidated.metadata + + def test_consolidated_metadata_rejects_child_without_parent() -> None: """A listing for 'a/b' with no 'a' listing made the root group unopenable.""" with pytest.raises(MetadataValidationError, match="without a listing"): From 1913880be418427ff1060bf58df859746694e3c5 Mon Sep 17 00:00:00 2001 From: Angus Date: Fri, 9 Oct 2026 13:24:31 -0400 Subject: [PATCH 4/5] ruff format --- tests/test_metadata/test_consolidated.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_metadata/test_consolidated.py b/tests/test_metadata/test_consolidated.py index c2a53d3479..174cca2de3 100644 --- a/tests/test_metadata/test_consolidated.py +++ b/tests/test_metadata/test_consolidated.py @@ -960,9 +960,7 @@ def test_consolidated_metadata_reserved_name_below_missing_parent() -> None: """A reserved name warns before the missing-parent error is raised.""" with pytest.warns(ZarrDeprecationWarning, match="reserved"): with pytest.raises(MetadataValidationError, match="without a listing"): - ConsolidatedMetadata.from_dict( - _consolidated_doc({"a": _GROUP_V3, "a/../b": _GROUP_V3}) - ) + ConsolidatedMetadata.from_dict(_consolidated_doc({"a": _GROUP_V3, "a/../b": _GROUP_V3})) @pytest.mark.parametrize( @@ -973,9 +971,7 @@ def test_consolidated_metadata_ignores_reserved_v2_keys(key: str) -> None: """The v2 specification does not reserve node names.""" with warnings.catch_warnings(): warnings.simplefilter("error") - consolidated = ConsolidatedMetadata.from_dict( - _consolidated_doc({key: _GROUP_V2}) - ) + consolidated = ConsolidatedMetadata.from_dict(_consolidated_doc({key: _GROUP_V2})) assert key in consolidated.metadata From 9156127264e3955092cc868e894f375cf4edde00 Mon Sep 17 00:00:00 2001 From: barlowa124 Date: Fri, 9 Oct 2026 13:43:09 -0400 Subject: [PATCH 5/5] narrow group metadata type in reserved-key test --- tests/test_metadata/test_consolidated.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_metadata/test_consolidated.py b/tests/test_metadata/test_consolidated.py index 174cca2de3..367465f7d6 100644 --- a/tests/test_metadata/test_consolidated.py +++ b/tests/test_metadata/test_consolidated.py @@ -949,7 +949,9 @@ def test_consolidated_metadata_warns_on_reserved_v3_keys(key: str) -> None: ) if "/" in key: parent, leaf = key.split("/") - nested = consolidated.metadata[parent].consolidated_metadata + parent_meta = consolidated.metadata[parent] + assert isinstance(parent_meta, GroupMetadata) + nested = parent_meta.consolidated_metadata assert nested is not None assert leaf in nested.metadata else: