Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ def open_geotiff(source: str | BinaryIO, *,
window : tuple or None
(row_start, col_start, row_stop, col_stop) for windowed reading.
overview_level : int or None
Overview level (0 = full resolution).
Overview level (0 = full resolution). Must be a non-negative int
or ``None``; passing ``bool`` or any other type raises
``TypeError``.
band : int or None
Band index (0-based). None returns all bands.
name : str or None
Expand Down Expand Up @@ -371,6 +373,14 @@ def open_geotiff(source: str | BinaryIO, *,

source = _coerce_path(source)

# Reject bool and non-int ``overview_level`` up front (issue #2074).
# Without this guard, ``overview_level=True`` is coerced to ``1`` and
# silently returns the first overview level, and non-int types leak
# raw ``TypeError`` messages from the internal numeric comparison or
# list indexing.
from ._validation import _validate_overview_level_arg
_validate_overview_level_arg(overview_level)

# ``on_gpu_failure`` is GPU-only. Reject it up front for CPU/dask paths
# rather than silently dropping it once dispatch is decided -- callers
# otherwise have no way to learn that the policy is being ignored.
Expand Down
7 changes: 7 additions & 0 deletions xrspatial/geotiff/_header.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,13 @@ def select_overview_ifd(ifds: list[IFD], overview_level: int | None) -> IFD:
if not ifds:
raise ValueError("No IFDs found in TIFF file")

# Defense in depth (issue #2074). ``open_geotiff`` already type-checks
# ``overview_level``, but this selector is also reachable from the dask,
# GPU, and accessor readers that forward the kwarg without going through
# the public entry point.
from ._validation import _validate_overview_level_arg
_validate_overview_level_arg(overview_level)

filtered = [ifd for ifd in ifds if _is_overview_or_full_res(ifd)]
if not filtered:
raise ValueError(
Expand Down
26 changes: 26 additions & 0 deletions xrspatial/geotiff/_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,32 @@ def _validate_tile_size_arg(tile_size):
_validate_tile_size(tile_size)


def _validate_overview_level_arg(overview_level) -> None:
"""Validate the ``overview_level`` kwarg type (issue #2074).

Accepts ``None`` (default, treated as full resolution) and any
``int`` / ``numpy.integer`` instance. ``bool`` is rejected
explicitly because Python treats it as a subclass of ``int``;
without the check, ``overview_level=True`` is coerced to ``1`` and
silently returns the first overview level. Non-int types like
``str`` and ``float`` would otherwise leak raw ``TypeError``
messages from the internal numeric comparison or list indexing.

Called by ``open_geotiff`` (public entry point) and by
``select_overview_ifd`` (defense in depth for the dask, GPU, and
accessor readers that reach the selector without going through
``open_geotiff``).
"""
if overview_level is None:
return
if (not isinstance(overview_level, (int, np.integer))
or isinstance(overview_level, bool)):
raise TypeError(
f"overview_level must be an int or None, got "
f"{type(overview_level).__name__}: {overview_level!r}"
)


def _validate_predictor_sample_format(predictor, sample_format) -> None:
"""Reject ``Predictor=3`` paired with a non-float ``SampleFormat`` (issue #1933).

Expand Down
125 changes: 125 additions & 0 deletions xrspatial/geotiff/tests/test_overview_level_type_validation_2074.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"""Type validation for ``open_geotiff(overview_level=...)``.

The selector in ``_header.select_overview_ifd`` compares ``overview_level``
numerically and indexes a list with it. Without an upfront type check,
``True`` is silently coerced to ``1`` (because ``bool`` is a subclass of
``int`` in Python), so a caller passing a bool by mistake gets back the
first overview level instead of an error. Non-int types like ``str`` and
``float`` leak raw ``TypeError`` messages from the internal comparison
or indexing. See issue #2074.

This module asserts that:

* ``overview_level=True`` / ``False`` raise ``TypeError`` naming ``bool``.
* ``overview_level="0"`` raises ``TypeError`` naming ``str``.
* ``overview_level=1.0`` raises ``TypeError`` naming ``float``.
* ``overview_level=0``, ``1``, and ``None`` continue to work.
"""
from __future__ import annotations

import numpy as np
import pytest


tifffile = pytest.importorskip("tifffile")


def _write_cog_with_one_overview(path: str) -> np.ndarray:
"""Write a 64x64 single-band TIFF with one half-resolution overview."""
rng = np.random.RandomState(0x2074)
arr = rng.randint(0, 256, size=(64, 64), dtype=np.uint8)
half = arr[::2, ::2]
with tifffile.TiffWriter(path) as tw:
tw.write(arr, tile=(32, 32), photometric="minisblack")
tw.write(half, tile=(32, 32), photometric="minisblack",
subfiletype=1)
return arr


@pytest.fixture
def cog_with_overview(tmp_path):
path = str(tmp_path / "cog_overview_level_type_2074.tif")
arr = _write_cog_with_one_overview(path)
return path, arr


@pytest.mark.parametrize("value", [True, False])
def test_overview_level_bool_raises_typeerror(cog_with_overview, value):
from xrspatial.geotiff import open_geotiff

path, _ = cog_with_overview
with pytest.raises(TypeError, match="bool"):
open_geotiff(path, overview_level=value)


def test_overview_level_str_raises_typeerror(cog_with_overview):
from xrspatial.geotiff import open_geotiff

path, _ = cog_with_overview
with pytest.raises(TypeError, match="str"):
open_geotiff(path, overview_level="0")


def test_overview_level_float_raises_typeerror(cog_with_overview):
from xrspatial.geotiff import open_geotiff

path, _ = cog_with_overview
with pytest.raises(TypeError, match="float"):
open_geotiff(path, overview_level=1.0)


def test_overview_level_zero_succeeds(cog_with_overview):
from xrspatial.geotiff import open_geotiff

path, arr = cog_with_overview
result = open_geotiff(path, overview_level=0)
assert result.shape == arr.shape


def test_overview_level_one_succeeds(cog_with_overview):
from xrspatial.geotiff import open_geotiff

path, arr = cog_with_overview
result = open_geotiff(path, overview_level=1)
# Half-resolution overview of a 64x64 source.
assert result.shape == (arr.shape[0] // 2, arr.shape[1] // 2)


def test_overview_level_none_succeeds(cog_with_overview):
from xrspatial.geotiff import open_geotiff

path, arr = cog_with_overview
result = open_geotiff(path, overview_level=None)
assert result.shape == arr.shape


@pytest.mark.parametrize("value", [np.int64(0), np.int32(0)])
def test_overview_level_numpy_int_zero_succeeds(cog_with_overview, value):
"""``np.int64`` / ``np.int32`` should be accepted like Python ints."""
from xrspatial.geotiff import open_geotiff

path, arr = cog_with_overview
result = open_geotiff(path, overview_level=value)
assert result.shape == arr.shape


@pytest.mark.parametrize("value", [np.int64(1), np.int32(1)])
def test_overview_level_numpy_int_one_succeeds(cog_with_overview, value):
"""``np.int64`` / ``np.int32`` reach the overview level just like int."""
from xrspatial.geotiff import open_geotiff

path, arr = cog_with_overview
result = open_geotiff(path, overview_level=value)
assert result.shape == (arr.shape[0] // 2, arr.shape[1] // 2)


def test_overview_level_typeerror_names_value(cog_with_overview):
"""Error message should name the offending value, not just the type."""
from xrspatial.geotiff import open_geotiff

path, _ = cog_with_overview
with pytest.raises(TypeError) as exc_info:
open_geotiff(path, overview_level="not-an-int")
msg = str(exc_info.value)
assert "str" in msg
assert "not-an-int" in msg
Loading