diff --git a/packages/zarr-indexing/changes/4347.bugfix.md b/packages/zarr-indexing/changes/4347.bugfix.md new file mode 100644 index 0000000000..b2eea6c071 --- /dev/null +++ b/packages/zarr-indexing/changes/4347.bugfix.md @@ -0,0 +1 @@ +Validate inclusive `index_array_bounds` against all raw index values when loading transforms and output maps from JSON. Accept valid finite and one-sided constraints, and reject out-of-bounds values eagerly before offset, stride, or map simplification. Validated immutable maps need not retain the constraints; message normalization preserves the original bounds. diff --git a/packages/zarr-indexing/docs/ndsel.md b/packages/zarr-indexing/docs/ndsel.md index 090dce64c4..3ba13176c1 100644 --- a/packages/zarr-indexing/docs/ndsel.md +++ b/packages/zarr-indexing/docs/ndsel.md @@ -99,8 +99,22 @@ assert t.to_json() == canonical map kind has a `to_json`; `output_index_map_from_json` dispatches the wire's structurally discriminated union back to the right kind. Exact JSON equality in this example is not a general round-trip guarantee: implicit flags are -removed, finite `index_array_bounds` are not retained or enforced by the -engine, and degenerate array maps are collapsed. +removed and degenerate array maps are collapsed. + +`index_array_bounds` constrains raw index-array values before the map's offset +and stride are applied. Both `IndexTransform.from_json` and +`output_index_map_from_json` validate every supplied value against the inclusive +bounds when loading. Finite and one-sided bounds are supported; omitted bounds +and `["-inf", "+inf"]` impose no additional constraint. Values outside the +bounds raise `NdselError("invalid_json", ...)`, including in singleton arrays +and zero-stride maps. Empty arrays satisfy any well-formed, ordered bounds. + +Validation is eager: an invalid entry rejects the entire map even if a later +selection would avoid that entry. After validation the engine owns immutable +index coordinates, so it need not retain the bounds; serialization emits +unbounded constraints for non-degenerate maps. Message normalization preserves +the original bounds without checking array contents. This implementation does +not defer bounds errors until individual positions are accessed. A canonical body carrying a `"-inf"` or `"+inf"` bound cannot be lowered — an `IndexDomain` addresses a diff --git a/packages/zarr-indexing/src/zarr_indexing/_wire.py b/packages/zarr-indexing/src/zarr_indexing/_wire.py index e2fc28d73c..75f6db8e4a 100644 --- a/packages/zarr-indexing/src/zarr_indexing/_wire.py +++ b/packages/zarr-indexing/src/zarr_indexing/_wire.py @@ -2,9 +2,9 @@ Package-private: the types that serialize themselves (`IndexDomain`, `IndexTransform`, the output map kinds) all need these, so they cannot live in -any one of them, and they are not API. The three engine constraints named in -[`zarr_indexing.json`][zarr_indexing.json] — finite bounds, implicit bounds -lowering by value, integer `index_array` content — are enforced here. +any one of them, and they are not API. Domain bounds, index-array bounds, +implicit bounds lowering by value, and integer `index_array` content are +handled here, as described in [`zarr_indexing.json`][zarr_indexing.json]. """ from __future__ import annotations @@ -13,13 +13,37 @@ import numpy as np -from zarr_indexing.messages import NdselError +from zarr_indexing.messages import NdselError, validate_index_array_bounds if TYPE_CHECKING: from zarr_indexing.domain import IndexDomain from zarr_indexing.json import BoundJSON +def check_index_array_bounds(array: np.ndarray[Any, Any], bounds: Any, where: str) -> None: + """Validate every raw index value against an inclusive interval. + + Validate interval syntax even for empty arrays. Once checked, immutable + index coordinates need no retained constraint. Use Python integer extrema + to avoid overflow or floating-point rounding at integer limits. + """ + lo, hi = validate_index_array_bounds(bounds, where) + if array.size == 0 or (lo == "-inf" and hi == "+inf"): + return + minimum, maximum = int(array.min()), int(array.max()) + if ( + lo == "+inf" + or hi == "-inf" + or (isinstance(lo, int) and minimum < lo) + or (isinstance(hi, int) and maximum > hi) + ): + raise NdselError( + "invalid_json", + f"{where}.index_array values [{minimum}, {maximum}] are outside " + f"index_array_bounds {bounds!r}", + ) + + def lower_bound(bound: BoundJSON, where: str) -> int: """Lower a canonical bound to a finite integer, rejecting infinities. diff --git a/packages/zarr-indexing/src/zarr_indexing/json.py b/packages/zarr-indexing/src/zarr_indexing/json.py index 70d29e1585..59d45d1678 100644 --- a/packages/zarr-indexing/src/zarr_indexing/json.py +++ b/packages/zarr-indexing/src/zarr_indexing/json.py @@ -14,6 +14,9 @@ The engine lowering rules include: +- **Validated index arrays.** Raw index values must satisfy the inclusive + `index_array_bounds` before offset and stride are applied. Lowering checks + all supplied values eagerly; validated immutable maps need not retain bounds. - **Finite bounds.** An `IndexDomain` addresses a finite array, so a canonical body carrying a `"-inf"`/`"+inf"` bound cannot be lowered; `from_json` raises. - **Implicit bounds lower by value.** The `[n]`-bracket implicit/explicit flag @@ -132,8 +135,9 @@ class OutputIndexMapJSON(TypedDict, total=False): index_array_bounds: list[IndexValueJSON] """Wire bounds on index-array values; `["-inf", "+inf"]` if unconstrained. - The engine currently discards this field on load and does not enforce - finite bounds against the array values. Serialization emits unconstrained bounds. + The message layer preserves these inclusive constraints on raw index values. + Engine lowering validates all values eagerly before offset and stride. + Serialization emits unconstrained bounds for validated non-degenerate maps. """ diff --git a/packages/zarr-indexing/src/zarr_indexing/messages.py b/packages/zarr-indexing/src/zarr_indexing/messages.py index 6cd5447cc1..ef3b051e22 100644 --- a/packages/zarr-indexing/src/zarr_indexing/messages.py +++ b/packages/zarr-indexing/src/zarr_indexing/messages.py @@ -556,7 +556,7 @@ def _normalize_output_map(raw: Any, where: str) -> dict[str, Any]: if has_index_array: stride = _check_int(raw["stride"], f"{where}.stride") if "stride" in raw else 1 bounds = ( - _check_index_array_bounds(raw["index_array_bounds"], where) + validate_index_array_bounds(raw["index_array_bounds"], where) if "index_array_bounds" in raw else ["-inf", "+inf"] ) @@ -585,7 +585,8 @@ def _normalize_output_map(raw: Any, where: str) -> dict[str, Any]: return {"offset": offset} -def _check_index_array_bounds(value: Any, where: str) -> list[int | str]: +def validate_index_array_bounds(value: Any, where: str) -> list[int | str]: + """Validate the syntax and ordering of an inclusive index-array interval.""" if not isinstance(value, list) or len(value) != 2: raise NdselError( "invalid_json", diff --git a/packages/zarr-indexing/src/zarr_indexing/output_map.py b/packages/zarr-indexing/src/zarr_indexing/output_map.py index 75d25d1d1d..b3c9eeb281 100644 --- a/packages/zarr-indexing/src/zarr_indexing/output_map.py +++ b/packages/zarr-indexing/src/zarr_indexing/output_map.py @@ -357,6 +357,10 @@ def output_index_map_from_json(data: OutputIndexMapJSON) -> OutputIndexMap: selects an array map, `input_dimension` selects a dimension map, and neither selects a constant map. + Validate every raw index value against the inclusive `index_array_bounds` + before applying offset and stride. Out-of-bounds values raise `NdselError` + at load time. Validated immutable maps do not retain the bounds. + Examples -------- >>> output_index_map_from_json({"offset": 5}) @@ -364,11 +368,13 @@ def output_index_map_from_json(data: OutputIndexMapJSON) -> OutputIndexMap: >>> output_index_map_from_json({"offset": 0, "stride": 2, "input_dimension": 1}) DimensionMap(input_dimension=1, offset=0, stride=2) """ - from zarr_indexing._wire import lower_index_array + from zarr_indexing._wire import check_index_array_bounds, lower_index_array if "index_array" in data: + array = lower_index_array(data["index_array"], "index_array") + check_index_array_bounds(array, data.get("index_array_bounds", ["-inf", "+inf"]), "output") return ArrayMap( - index_array=lower_index_array(data["index_array"], "index_array"), + index_array=array, offset=data.get("offset", 0), stride=data.get("stride", 1), ) diff --git a/packages/zarr-indexing/src/zarr_indexing/transform.py b/packages/zarr-indexing/src/zarr_indexing/transform.py index 5c01c9e85b..39976edc90 100644 --- a/packages/zarr-indexing/src/zarr_indexing/transform.py +++ b/packages/zarr-indexing/src/zarr_indexing/transform.py @@ -793,7 +793,10 @@ def from_json(cls, data: IndexTransformJSON) -> IndexTransform: that omitted fields — identity `output`, default bounds and labels — are filled and validated, then lowered to the engine representation. Lower-rank `index_array`s are widened to the full input rank on the way - in. + in. Every supplied raw index value is checked against the inclusive + `index_array_bounds` before offset, stride, or map simplification. + Out-of-bounds values raise `NdselError` immediately, even if a later + selection would avoid them. Validated immutable maps do not retain bounds. Examples -------- @@ -805,6 +808,7 @@ def from_json(cls, data: IndexTransformJSON) -> IndexTransform: True """ from zarr_indexing._wire import ( + check_index_array_bounds, full_rank_index_array, lower_bound, lower_index_array, @@ -841,6 +845,7 @@ def from_json(cls, data: IndexTransformJSON) -> IndexTransform: if "index_array" in om: where = f"output[{i}]" arr = lower_index_array(om["index_array"], f"{where}.index_array") + check_index_array_bounds(arr, om["index_array_bounds"], where) # ndsel leaves index-array rank unvalidated, so an external # producer may send an array of lower rank that broadcasts # against the domain. Widen it here, on the way in, so every diff --git a/packages/zarr-indexing/tests/test_wire_bounds.py b/packages/zarr-indexing/tests/test_wire_bounds.py new file mode 100644 index 0000000000..6ae5581f04 --- /dev/null +++ b/packages/zarr-indexing/tests/test_wire_bounds.py @@ -0,0 +1,130 @@ +"""Index-array bounds are validated against raw coordinates during loading.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import numpy as np +import pytest + +from zarr_indexing import ConstantMap, IndexTransform, normalize_ndsel +from zarr_indexing.messages import NdselError +from zarr_indexing.output_map import output_index_map_from_json + +if TYPE_CHECKING: + from zarr_indexing.json import IndexTransformJSON, OutputIndexMapJSON + + +def load(values: Any, bounds: Any, entry: str, offset: int, stride: int) -> Any: + output: OutputIndexMapJSON = {"index_array": values, "offset": offset, "stride": stride} + if bounds is not None: + output["index_array_bounds"] = bounds + if entry == "output_map": + return output_index_map_from_json(output) + body: IndexTransformJSON = {"input_shape": list(np.asarray(values).shape), "output": [output]} + return IndexTransform.from_json(body) + + +@pytest.mark.parametrize("entry", ["transform", "output_map"]) +@pytest.mark.parametrize(("offset", "stride"), [(0, 1), (10, -2), (7, 0)]) +@pytest.mark.parametrize( + ("values", "bounds"), + [ + ([-3, 0, 4], [-3, 4]), + ([-3, 0, 4], ["-inf", 4]), + ([-3, 0, 4], [-3, "+inf"]), + ([-3, 0, 4], ["-inf", "+inf"]), + ([-3, 0, 4], None), + ([2], [2, 2]), + ([], [2, 2]), + ([], ["+inf", "+inf"]), + ([], ["-inf", "-inf"]), + ([[0, 2], [2, 0]], [0, 2]), + ], +) +def test_valid_bounds_preserve_values_and_roundtrip( + values: Any, bounds: Any, entry: str, offset: int, stride: int +) -> None: + model = np.asarray(values, dtype=np.intp) + loaded = load(values, bounds, entry, offset, stride) + expected = offset + stride * model + if entry == "transform": + points = np.array(list(np.ndindex(model.shape)), dtype=np.intp).reshape(-1, model.ndim) + restored = IndexTransform.from_json(loaded.to_json()) + np.testing.assert_array_equal(loaded.apply_many(points).reshape(model.shape), expected) + np.testing.assert_array_equal(restored.apply_many(points).reshape(model.shape), expected) + # Further selection and affine adjustment operate on validated coordinates. + if model.ndim == 1 and model.size > 1: + selected = loaded.oindex[[model.size - 1, 0]].translate((3,)) + np.testing.assert_array_equal( + selected.apply_many([[0], [1]]).ravel(), expected[[-1, 0]] + 3 + ) + else: + restored = output_index_map_from_json(loaded.to_json()) + if isinstance(restored, ConstantMap): + np.testing.assert_array_equal(np.full(model.shape, restored.offset), expected) + else: + assert restored == loaded + np.testing.assert_array_equal(loaded.offset + loaded.stride * loaded.index_array, expected) + if bounds is not None: + canonical = normalize_ndsel( + { + "kind": "transform", + "input_shape": list(model.shape), + "output": [{"index_array": values, "index_array_bounds": bounds}], + } + ) + assert canonical["output"][0]["index_array_bounds"] == bounds + + +@pytest.mark.parametrize("entry", ["transform", "output_map"]) +@pytest.mark.parametrize(("offset", "stride"), [(0, 1), (10, -2), (7, 0)]) +@pytest.mark.parametrize( + ("values", "bounds"), + [ + ([-1, 0], [0, 2]), + ([0, 3], [0, 2]), + ([3], [2, 2]), + ([-1], [0, "+inf"]), + ([3], ["-inf", 2]), + ([0], ["+inf", "+inf"]), + ([0], ["-inf", "-inf"]), + ([[0, 1], [1, 3]], [0, 2]), + ], +) +def test_out_of_bounds_raw_values_are_rejected( + values: Any, bounds: Any, entry: str, offset: int, stride: int +) -> None: + with pytest.raises(NdselError, match="index_array_bounds") as exc: + load(values, bounds, entry, offset, stride) + assert exc.value.reason == "invalid_json" + + +@pytest.mark.parametrize("entry", ["transform", "output_map"]) +@pytest.mark.parametrize("values", [[], [1]]) +@pytest.mark.parametrize("bounds", [[], [0], [0, 1, 2], [False, 2], [0.5, 2], ["bad", 2]]) +def test_malformed_bounds_are_rejected(values: Any, bounds: Any, entry: str) -> None: + with pytest.raises(NdselError) as exc: + load(values, bounds, entry, 0, 1) + assert exc.value.reason == "invalid_json" + + +@pytest.mark.parametrize("entry", ["transform", "output_map"]) +@pytest.mark.parametrize("values", [[], [1]]) +def test_reversed_bounds_are_rejected(values: Any, entry: str) -> None: + with pytest.raises(NdselError) as exc: + load(values, [2, 0], entry, 0, 1) + assert exc.value.reason == "bounds_out_of_order" + + +@pytest.mark.parametrize("entry", ["transform", "output_map"]) +@pytest.mark.parametrize("value", [int(np.iinfo(np.intp).min), int(np.iinfo(np.intp).max)]) +def test_integer_extreme_bounds_are_exact(entry: str, value: int) -> None: + result = load([value], [value, value], entry, 0, 1) + if entry == "transform": + assert result.apply((0,)) == (value,) + else: + assert int(result.index_array[0]) == value + bounds = [value + 1, "+inf"] if value < 0 else ["-inf", value - 1] + with pytest.raises(NdselError, match="index_array_bounds"): + load([value], bounds, entry, 0, 1)