Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions changes/3285.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
JSON metadata validation now delegates to ``msgspec.convert`` for the type
coercions it supports (``Literal`` membership, ``int`` / ``bool`` strictness,
list-to-tuple), replacing the per-field hand-written ``parse_*`` logic. A small
fallback validates the recursive JSON values msgspec cannot, now with an
explicit nesting-depth limit, and a latent generator-exhaustion bug in
``parse_storage_transformers`` is fixed. See #3285.

As a result some metadata inputs are now parsed more strictly. The previous
per-field checks compared values with ``==``, which accepts any numerically
equal object, so a float such as ``2.0`` was accepted as ``zarr_format``; it is
now rejected because it is not an ``int``. Booleans are likewise no longer
accepted where an ``int`` is expected, since ``bool`` is an ``int`` subclass.
Metadata that conforms to the Zarr specification is unaffected.
1 change: 1 addition & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ plugins:
- https://docs.xarray.dev/en/stable/objects.inv
- https://numpy.org/doc/stable/objects.inv
- https://numcodecs.readthedocs.io/en/stable/objects.inv
- https://msgspec.dev/objects.inv
- https://developmentseed.org/obstore/latest/objects.inv
- https://filesystem-spec.readthedocs.io/en/latest/objects.inv
- https://requests.readthedocs.io/en/latest/objects.inv
Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ dependencies = [
'google-crc32c>=1.5',
'typing_extensions>=4.14',
'donfig>=0.8',
'msgspec>=0.19',
]

dynamic = [
Expand Down Expand Up @@ -269,6 +270,7 @@ extra-dependencies = [
'typing_extensions==4.14.*',
'donfig==0.8.*',
'obstore==0.5.*',
'msgspec==0.19.*',
]

[tool.hatch.envs.default]
Expand Down
26 changes: 12 additions & 14 deletions src/zarr/codecs/blosc.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from zarr.core.buffer.cpu import as_numpy_array_wrapper
from zarr.core.common import JSON, NamedRequiredConfig, parse_named_configuration
from zarr.core.dtype.common import HasItemSize
from zarr.core.json_parse import parse_field

if TYPE_CHECKING:
from typing import Self
Expand Down Expand Up @@ -104,27 +105,24 @@ class BloscCname(metaclass=_DeprecatedStrEnumMeta):


def parse_typesize(data: JSON) -> int:
if isinstance(data, int):
if data > 0:
return data
else:
raise ValueError(
f"Value must be greater than 0. Got {data}, which is less or equal to 0."
)
raise TypeError(f"Value must be an int. Got {type(data)} instead.")
parsed: int = parse_field(data, int, "typesize", error=TypeError)
if parsed > 0:
return parsed
else:
raise ValueError(
f"Value must be greater than 0. Got {parsed}, which is less or equal to 0."
)


# todo: real validation
def parse_clevel(data: JSON) -> int:
if isinstance(data, int):
return data
raise TypeError(f"Value should be an int. Got {type(data)} instead.")
parsed: int = parse_field(data, int, "clevel", error=TypeError)
return parsed


def parse_blocksize(data: JSON) -> int:
if isinstance(data, int):
return data
raise TypeError(f"Value should be an int. Got {type(data)} instead.")
parsed: int = parse_field(data, int, "blocksize", error=TypeError)
return parsed


def _parse_cname(data: object) -> BloscCnameLiteral:
Expand Down
10 changes: 5 additions & 5 deletions src/zarr/codecs/gzip.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from zarr.abc.codec import BytesBytesCodec
from zarr.core.buffer.cpu import as_numpy_array_wrapper
from zarr.core.common import JSON, parse_named_configuration
from zarr.core.json_parse import parse_field

if TYPE_CHECKING:
from typing import Self
Expand All @@ -19,13 +20,12 @@


def parse_gzip_level(data: JSON) -> int:
if not isinstance(data, (int)):
raise TypeError(f"Expected int, got {type(data)}")
if data not in range(10):
parsed: int = parse_field(data, int, "level", error=TypeError)
if parsed not in range(10):
raise ValueError(
f"Expected an integer from the inclusive range (0, 9). Got {data} instead."
f"Expected an integer from the inclusive range (0, 9). Got {parsed} instead."
)
return data
return parsed


@dataclass(frozen=True)
Expand Down
15 changes: 7 additions & 8 deletions src/zarr/codecs/zstd.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from zarr.abc.codec import BytesBytesCodec
from zarr.core.buffer.cpu import as_numpy_array_wrapper
from zarr.core.common import JSON, parse_named_configuration
from zarr.core.json_parse import parse_field

if TYPE_CHECKING:
from typing import Self
Expand All @@ -21,17 +22,15 @@


def parse_zstd_level(data: JSON) -> int:
if isinstance(data, int):
if data >= 23:
raise ValueError(f"Value must be less than or equal to 22. Got {data} instead.")
return data
raise TypeError(f"Got value with type {type(data)}, but expected an int.")
parsed: int = parse_field(data, int, "level", error=TypeError)
if parsed >= 23:
raise ValueError(f"Value must be less than or equal to 22. Got {parsed} instead.")
return parsed


def parse_checksum(data: JSON) -> bool:
if isinstance(data, bool):
return data
raise TypeError(f"Expected bool. Got {type(data)}.")
parsed: bool = parse_field(data, bool, "checksum", error=TypeError)
return parsed


@dataclass(frozen=True)
Expand Down
5 changes: 2 additions & 3 deletions src/zarr/core/chunk_key_encodings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
NamedConfig,
parse_named_configuration,
)
from zarr.core.json_parse import parse_field
from zarr.registry import get_chunk_key_encoding_class, register_chunk_key_encoding

SeparatorLiteral = Literal[".", "/"]


def parse_separator(data: JSON) -> SeparatorLiteral:
if data not in (".", "/"):
raise ValueError(f"Expected an '.' or '/' separator. Got {data} instead.")
return cast("SeparatorLiteral", data)
return cast("SeparatorLiteral", parse_field(data, Literal[".", "/"], "separator"))


class ChunkKeyEncodingParams(TypedDict):
Expand Down
22 changes: 10 additions & 12 deletions src/zarr/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from typing_extensions import ReadOnly

from zarr.core.config import config as zarr_config
from zarr.core.json_parse import convert, parse_field
from zarr.errors import ZarrRuntimeWarning

if TYPE_CHECKING:
Expand Down Expand Up @@ -147,12 +148,13 @@ def parse_enum[E: Enum](data: object, cls: type[E]) -> E:


def parse_name(data: JSON, expected: str | None = None) -> str:
if isinstance(data, str):
if expected is None or data == expected:
return data
raise ValueError(f"Expected '{expected}'. Got {data} instead.")
else:
raise TypeError(f"Expected a string, got an instance of {type(data)}.")
try:
data = cast("str", convert(data, str))
except (ValueError, TypeError) as exc:
raise TypeError(f"Expected a string, got an instance of {type(data)}.") from exc
if expected is None or data == expected:
return data
raise ValueError(f"Expected '{expected}'. Got {data} instead.")


def parse_configuration(data: JSON) -> JSON:
Expand Down Expand Up @@ -227,15 +229,11 @@ def parse_fill_value(data: Any) -> Any:


def parse_order(data: Any) -> Literal["C", "F"]:
if data in ("C", "F"):
return cast("Literal['C', 'F']", data)
raise ValueError(f"Expected one of ('C', 'F'), got {data} instead.")
return cast("Literal['C', 'F']", parse_field(data, Literal["C", "F"], "order"))


def parse_bool(data: Any) -> bool:
if isinstance(data, bool):
return data
raise ValueError(f"Expected bool, got {data} instead.")
return cast("bool", convert(data, bool))


def parse_int(data: Any) -> int:
Expand Down
7 changes: 3 additions & 4 deletions src/zarr/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

from donfig import Config as DConfig

from zarr.core.json_parse import parse_field

if TYPE_CHECKING:
from donfig.config_obj import ConfigSet

Expand Down Expand Up @@ -159,7 +161,4 @@ def enable_gpu(self) -> ConfigSet:


def parse_indexing_order(data: Any) -> Literal["C", "F"]:
if data in ("C", "F"):
return cast("Literal['C', 'F']", data)
msg = f"Expected one of ('C', 'F'), got {data} instead."
raise ValueError(msg)
return cast("Literal['C', 'F']", parse_field(data, Literal["C", "F"], "order"))
14 changes: 6 additions & 8 deletions src/zarr/core/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
parse_shapelike,
)
from zarr.core.config import config
from zarr.core.json_parse import parse_field
from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
from zarr.core.metadata.io import save_metadata
from zarr.core.sync import SyncMixin, sync
Expand Down Expand Up @@ -85,18 +86,15 @@

def parse_zarr_format(data: Any) -> ZarrFormat:
"""Parse the zarr_format field from metadata."""
if data in (2, 3):
return cast("ZarrFormat", data)
msg = f"Invalid zarr_format. Expected one of 2 or 3. Got {data}."
raise ValueError(msg)
return cast("ZarrFormat", parse_field(data, Literal[2, 3], "zarr_format"))


def parse_node_type(data: Any) -> NodeType:
"""Parse the node_type field from metadata."""
if data in ("array", "group"):
return cast("Literal['array', 'group']", data)
msg = f"Invalid value for 'node_type'. Expected 'array' or 'group'. Got '{data}'."
raise MetadataValidationError(msg)
return cast(
"Literal['array', 'group']",
parse_field(data, Literal["array", "group"], "node_type", error=MetadataValidationError),
)


# todo: convert None to empty dict
Expand Down
103 changes: 103 additions & 0 deletions src/zarr/core/json_parse.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Helpers for validating JSON-decoded metadata.

Most JSON metadata validation is delegated to
[`msgspec.convert`][msgspec.convert], which handles the type coercions Zarr
needs (``Literal`` membership, ``int``/``bool`` strictness, list-to-tuple,
``TypedDict`` with ``NotRequired``). ``convert`` is a thin wrapper that
translates [`msgspec.ValidationError`][msgspec.ValidationError] into the
``TypeError`` the rest of the codebase already raises.

msgspec cannot handle two things in Zarr's metadata types:

* the recursive ``JSON`` / ``JSONValue`` aliases, which it rejects at
schema-build time, and
* PEP 728 ``extra_items=`` extension fields, which it silently drops.

``validate_json_value`` is the small hand-written fallback for the first of
those. See https://github.com/zarr-developers/zarr-python/issues/3285.
"""

from __future__ import annotations

from collections.abc import Mapping
from typing import TYPE_CHECKING, Any, Final, cast, get_origin

import msgspec

if TYPE_CHECKING:
from zarr.core.common import JSON

__all__ = ["MAX_JSON_DEPTH", "convert", "parse_field", "validate_json_value"]

MAX_JSON_DEPTH: Final = 64
"""Maximum nesting depth accepted by ``validate_json_value``."""


def _type_name(type_: Any) -> str:
"""Render ``type_`` for an error message.

Parameterized types keep their arguments, so a ``Literal`` reports its
members (``Literal[2, 3]``) rather than the bare origin name. ``__name__``
would drop them, which loses the most useful part of the message.
"""
if get_origin(type_) is not None:
return str(type_).replace("typing.", "")
return getattr(type_, "__name__", None) or str(type_).replace("typing.", "")


def convert(value: object, type_: Any, *, strict: bool = True) -> Any:
"""Validate and coerce ``value`` against ``type_`` via [`msgspec.convert`][msgspec.convert].

On a mismatch msgspec raises
[`msgspec.ValidationError`][msgspec.ValidationError]; this re-raises
a plain, field-agnostic ``ValueError`` naming the expected type, so callers
can add their own field context (see ``parse_field``).
"""
try:
return msgspec.convert(value, type_, strict=strict)
except msgspec.ValidationError as exc:
raise ValueError(f"Expected instance of {_type_name(type_)}, got {value!r}.") from exc


def parse_field(
data: object, type_: Any, field: str, *, error: type[Exception] = ValueError
) -> Any:
"""Validate ``data`` for metadata field ``field`` against ``type_``.

Wraps ``convert`` and, on failure, re-raises ``error`` with field
context, chaining the underlying type error. This keeps the
``convert``-then-re-raise pattern in one place rather than repeating it in
every per-field parser.
"""
try:
return convert(data, type_)
except ValueError as exc:
raise error(
f"Failed to parse input for {field!r}: expected {_type_name(type_)}, got {data!r}."
) from exc


def validate_json_value(value: object, *, max_depth: int = MAX_JSON_DEPTH, _depth: int = 0) -> JSON:
"""Check that ``value`` is a JSON value and return it unchanged.

msgspec cannot build a schema for Zarr's recursive ``JSON`` / ``JSONValue``
aliases, so this covers the fields typed that way (``attributes``,
``fill_value``, extension-field values). Unlike the previous per-field
parsers it also enforces ``max_depth``: a pathologically nested document
could otherwise exhaust the interpreter stack.
"""
if _depth > max_depth:
raise ValueError(f"JSON value nesting exceeds the maximum depth of {max_depth}.")
if value is None or isinstance(value, (bool, int, float, str)):
return cast("JSON", value)
if isinstance(value, (list, tuple)):
for item in value:
validate_json_value(item, max_depth=max_depth, _depth=_depth + 1)
return cast("JSON", value)
if isinstance(value, Mapping):
for key, item in value.items():
if not isinstance(key, str):
raise TypeError(f"JSON object keys must be str, got {type(key).__name__}.")
validate_json_value(item, max_depth=max_depth, _depth=_depth + 1)
return cast("JSON", value)
raise TypeError(f"Value {value!r} is not a valid JSON value.")
7 changes: 4 additions & 3 deletions src/zarr/core/metadata/v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
parse_shapelike,
)
from zarr.core.config import config, parse_indexing_order
from zarr.core.json_parse import parse_field
from zarr.core.metadata.common import parse_attributes


Expand Down Expand Up @@ -278,9 +279,9 @@ def parse_dtype(data: npt.DTypeLike) -> np.dtype[Any]:


def parse_zarr_format(data: object) -> Literal[2]:
if data == 2:
return 2
raise ValueError(f"Invalid value. Expected 2. Got {data}.")
from typing import Literal

return cast("Literal[2]", parse_field(data, Literal[2], "zarr_format"))


def parse_filters(data: object) -> tuple[Numcodec, ...] | None:
Expand Down
Loading
Loading