Skip to content

Commit 57e66d9

Browse files
authored
fix: reject malformed chunk keys in DefaultChunkKeyEncoding (#4219)
* fix: reject malformed chunk keys in DefaultChunkKeyEncoding decode_chunk_key stripped a single leading character and split the rest, so any key at all decoded to something. "0/1" silently became (1,) -- the "0" was eaten as if it were the "c" prefix -- and a key written with one separator decoded wrongly under an encoding configured with the other. Validate the "c<separator>" prefix and raise ValueError when it is absent, so a key that is not a chunk key for this encoding is reported rather than silently misread. Adds the tests this method never had, covering the round trip for both separators and each way a key can fail to carry the prefix. Assisted-by: ClaudeCode:claude-fable-5 * Rename 250.bugfix.md to 4219.bugfix.md
1 parent ec8e70a commit 57e66d9

3 files changed

Lines changed: 73 additions & 1 deletion

File tree

‎changes/4219.bugfix.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
`DefaultChunkKeyEncoding.decode_chunk_key` now validates that a chunk key
2+
starts with the configured `c<separator>` prefix and raises `ValueError` for
3+
malformed keys, instead of silently decoding them incorrectly.

‎src/zarr/core/chunk_key_encodings.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,11 @@ def __post_init__(self) -> None:
7979
def decode_chunk_key(self, chunk_key: str) -> tuple[int, ...]:
8080
if chunk_key == "c":
8181
return ()
82-
return tuple(map(int, chunk_key[1:].split(self.separator)))
82+
# Strip the "c<sep>" prefix (e.g. "c/" or "c.") before splitting.
83+
prefix = "c" + self.separator
84+
if chunk_key.startswith(prefix):
85+
return tuple(map(int, chunk_key[len(prefix) :].split(self.separator)))
86+
raise ValueError(f"Invalid chunk key for default encoding: {chunk_key!r}")
8387

8488
def encode_chunk_key(self, chunk_coords: tuple[int, ...]) -> str:
8589
return self.separator.join(map(str, ("c",) + chunk_coords))

‎tests/test_chunk_key_encodings.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
from __future__ import annotations
2+
3+
import pytest
4+
5+
from zarr.core.chunk_key_encodings import DefaultChunkKeyEncoding, V2ChunkKeyEncoding
6+
7+
8+
@pytest.mark.parametrize("separator", ["/", "."])
9+
@pytest.mark.parametrize(
10+
"coords",
11+
[(), (0,), (1, 2), (10, 0, 3)],
12+
)
13+
def test_default_encoding_round_trips(separator: str, coords: tuple[int, ...]) -> None:
14+
"""Encoding coordinates and decoding the result returns the coordinates."""
15+
encoding = DefaultChunkKeyEncoding(separator=separator) # type: ignore[arg-type]
16+
17+
key = encoding.encode_chunk_key(coords)
18+
assert encoding.decode_chunk_key(key) == coords
19+
20+
21+
@pytest.mark.parametrize("separator", ["/", "."])
22+
@pytest.mark.parametrize("coords", [(0,), (1, 2), (10, 0, 3)])
23+
def test_v2_encoding_round_trips(separator: str, coords: tuple[int, ...]) -> None:
24+
"""The v2 encoding round-trips coordinates for either separator."""
25+
encoding = V2ChunkKeyEncoding(separator=separator) # type: ignore[arg-type]
26+
27+
key = encoding.encode_chunk_key(coords)
28+
assert encoding.decode_chunk_key(key) == coords
29+
30+
31+
@pytest.mark.parametrize("separator", ["/", "."])
32+
def test_v2_zero_dimensional_key_is_ambiguous(separator: str) -> None:
33+
"""A 0-d v2 array stores its sole chunk under `"0"`, the same key a 1-d
34+
array uses for chunk 0, so decoding cannot recover the empty tuple on its
35+
own -- the array's dimensionality is what disambiguates it."""
36+
encoding = V2ChunkKeyEncoding(separator=separator) # type: ignore[arg-type]
37+
38+
assert encoding.encode_chunk_key(()) == "0"
39+
assert encoding.decode_chunk_key("0") == (0,)
40+
41+
42+
@pytest.mark.parametrize(
43+
"chunk_key",
44+
[
45+
"0/1", # no "c" prefix at all
46+
"c0/1", # "c" not followed by the separator
47+
"x/0/1", # wrong prefix character
48+
"",
49+
],
50+
)
51+
def test_default_encoding_rejects_key_without_prefix(chunk_key: str) -> None:
52+
"""A key that does not carry the `c<separator>` prefix is not a chunk key
53+
for this encoding, and must be rejected rather than silently decoded."""
54+
encoding = DefaultChunkKeyEncoding(separator="/")
55+
56+
with pytest.raises(ValueError, match="Invalid chunk key"):
57+
encoding.decode_chunk_key(chunk_key)
58+
59+
60+
def test_default_encoding_rejects_key_using_the_other_separator() -> None:
61+
"""A key encoded with `.` is not valid for a `/`-separated encoding."""
62+
encoding = DefaultChunkKeyEncoding(separator="/")
63+
64+
with pytest.raises(ValueError, match="Invalid chunk key"):
65+
encoding.decode_chunk_key("c.0.1")

0 commit comments

Comments
 (0)