From a33c447d069cd5b98918c00ffbe2c8c9c581053f Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 5 Jun 2026 12:07:44 -0700 Subject: [PATCH] geotiff: reject non-string compression at the to_geotiff boundary (#2975) A non-string compression skipped the wrapper's name-validation block and later hit compression.lower() during compression_level validation, leaking a raw AttributeError. Reject the bad type up front with the same TypeError shape the low-level writer in _writer.py already uses. --- xrspatial/geotiff/_writers/eager.py | 10 +++++++ xrspatial/geotiff/tests/write/test_basic.py | 30 +++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/xrspatial/geotiff/_writers/eager.py b/xrspatial/geotiff/_writers/eager.py index 939f2f5e..1aa1b05f 100644 --- a/xrspatial/geotiff/_writers/eager.py +++ b/xrspatial/geotiff/_writers/eager.py @@ -496,6 +496,16 @@ def to_geotiff(data: xr.DataArray | np.ndarray, # Up-front validation: catch bad compression names before they reach # any of the deeper write paths (streaming, GPU, VRT, COG) where the # error surfaces from _compression_tag with a less obvious traceback. + if not isinstance(compression, str) and compression is not None: + # The string block below validates bad NAMES, but a non-string + # ``compression`` skips it entirely and later lands in + # ``compression.lower()`` during compression_level validation, + # surfacing as ``AttributeError`` instead of a typed error. + # Reject the bad TYPE here with the same shape as the low-level + # writer's guard in ``_writer.py``. + raise TypeError( + f"compression must be a str (in to_geotiff); " + f"got {type(compression).__name__}.") if isinstance(compression, str): if compression.lower() not in _VALID_COMPRESSIONS: raise ValueError( diff --git a/xrspatial/geotiff/tests/write/test_basic.py b/xrspatial/geotiff/tests/write/test_basic.py index 05f336fb..0110dca3 100644 --- a/xrspatial/geotiff/tests/write/test_basic.py +++ b/xrspatial/geotiff/tests/write/test_basic.py @@ -158,6 +158,36 @@ def test_unsupported_compression(self, tmp_path): with pytest.raises(ValueError, match="(Unsupported|Unknown) compression"): write(arr, str(tmp_path / 'bad.tif'), compression='bzip2') + @pytest.mark.parametrize('bad', [5, 5.0, ['deflate'], object()]) + def test_to_geotiff_non_string_compression_typeerror(self, tmp_path, bad): + # Regression for #2975: a non-string ``compression`` skipped the + # public wrapper's name-validation block and later landed in + # ``compression.lower()`` during compression_level validation, + # surfacing as a raw ``AttributeError``. The wrapper now rejects + # the bad type with the same ``TypeError`` shape as the low-level + # writer's guard. + arr = np.zeros((4, 4), dtype=np.float32) + path = str(tmp_path / 'tmp_2975_badtype.tif') + with pytest.raises(TypeError, match="compression must be a str"): + to_geotiff(arr, path, compression=bad, compression_level=1) + + def test_to_geotiff_non_string_compression_no_level(self, tmp_path): + # The same rejection must fire even without ``compression_level``, + # so a non-string never reaches any ``.lower()`` call (#2975). + arr = np.zeros((4, 4), dtype=np.float32) + path = str(tmp_path / 'tmp_2975_badtype_nolevel.tif') + with pytest.raises(TypeError, match="compression must be a str"): + to_geotiff(arr, path, compression=7) + + def test_to_geotiff_valid_compression_still_works(self, tmp_path): + # Valid string compressions, the no-compression 'none' string, and + # a string + compression_level combo must still write (#2975). + arr = np.arange(16, dtype=np.float32).reshape(4, 4) + to_geotiff(arr, str(tmp_path / 'tmp_2975_default.tif')) + to_geotiff(arr, str(tmp_path / 'tmp_2975_none.tif'), compression='none') + to_geotiff(arr, str(tmp_path / 'tmp_2975_deflate.tif'), + compression='deflate', compression_level=6) + # ------------------------------------------------------------------------- # Section: writer dtype x compression matrix