From 7a992d5224e020dbad885be816f0c5c3ae2cdd0b Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 13 May 2026 11:35:55 -0700 Subject: [PATCH] geotiff: update #1796 and #1823 VRT tests for new minimal-read semantics PR #1821 (#1704) made the VRT resample path read only the minimal SrcRect sub-rect that maps to the clipped destination sub-window. Test_vrt_dstrect_resample_cap_1737.py was updated in that PR but two sibling tests asserting the old "full SrcRect rejected by max_pixels" behaviour were missed and now fail in CI on main. test_vrt_source_max_pixels_1796.py: the tiny-VRT-cannot-force-huge- source-decode invariant is now structural (the source read is bounded by the dst sub-window), so the test asserts the positive behaviour (read succeeds, returns 1 pixel) plus a companion test that the per-source cap still fires when the sub-window itself exceeds the budget. test_vrt_source_tile_check_1823.py: TestOutputWindowCheckStillEnforced was checking the OUTPUT WINDOW CHECK with a 1x1 raster and a 4x4 SrcRect; under #1821 the source read shrinks to one pixel and the check no longer fires. Reframed the test to assert that an over-budget OUTPUT EXTENT still rejects via _check_dimensions at the top of read_vrt. --- .../tests/test_vrt_source_max_pixels_1796.py | 54 +++++++++++++++++-- .../tests/test_vrt_source_tile_check_1823.py | 48 ++++++++++------- 2 files changed, 78 insertions(+), 24 deletions(-) diff --git a/xrspatial/geotiff/tests/test_vrt_source_max_pixels_1796.py b/xrspatial/geotiff/tests/test_vrt_source_max_pixels_1796.py index 7ce72288..f93e336b 100644 --- a/xrspatial/geotiff/tests/test_vrt_source_max_pixels_1796.py +++ b/xrspatial/geotiff/tests/test_vrt_source_max_pixels_1796.py @@ -1,4 +1,17 @@ -"""VRT source reads must honor the caller's max_pixels budget (#1796).""" +"""VRT source reads must honor the caller's max_pixels budget (#1796). + +Originally the source loop in ``read_vrt`` read the full SrcRect of a +SimpleSource with size mismatch, so a tiny VRT output could force a huge +source decode. #1803 forwarded ``max_pixels`` to ``read_to_array`` to +catch that pattern. + +After #1704 / PR #1821 the resample path inverse-maps the clipped +destination sub-window to the minimal SrcRect sub-rect and reads only +that. A tiny VRT output is now bounded structurally: the source read +cannot exceed the dst sub-window size. The per-source ``max_pixels`` +guard still applies (defence in depth) and still bites when the +sub-window itself exceeds the budget. +""" from __future__ import annotations import os @@ -9,8 +22,10 @@ from xrspatial.geotiff import to_geotiff, read_vrt -def test_vrt_source_read_forwards_max_pixels(tmp_path): - """A tiny VRT output cannot force an oversized source-window decode.""" +def test_tiny_vrt_with_huge_srcrect_now_reads_minimally(tmp_path): + """A 1x1 VRT pointing at a 4x4 SrcRect now reads only the one source + pixel that maps to the single output pixel, so ``max_pixels=1`` is + no longer exceeded. Locks in the structural improvement from #1704.""" src = tmp_path / "tmp_1796_source.tif" data = np.arange(16, dtype=np.uint8).reshape(4, 4) to_geotiff(data, str(src), compression='none') @@ -30,6 +45,35 @@ def test_vrt_source_read_forwards_max_pixels(tmp_path): '\n' ) - with pytest.raises(ValueError, match="exceed"): - read_vrt(str(vrt), max_pixels=1) + arr = read_vrt(str(vrt), max_pixels=1) + assert arr.shape == (1, 1) + + +def test_source_cap_still_fires_when_sub_window_exceeds_budget(tmp_path): + """The per-source pixel-budget guard still rejects a sub-window that + exceeds ``max_pixels``. With the sub-window-bounded read, the cap is + measured against the clipped destination region rather than the raw + SrcRect; the protection from #1796 carries over to that new + measurement. + """ + src = tmp_path / "tmp_1796_big_source.tif" + data = np.arange(64, dtype=np.uint8).reshape(8, 8) + to_geotiff(data, str(src), compression='none', tiled=False) + + vrt = tmp_path / "tmp_1796_big_cap.vrt" + vrt.write_text( + '\n' + ' \n' + ' \n' + f' {os.path.basename(src)}' + '\n' + ' 1\n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n' + ) + with pytest.raises(ValueError, match="exceed|safety limit"): + read_vrt(str(vrt), max_pixels=4) diff --git a/xrspatial/geotiff/tests/test_vrt_source_tile_check_1823.py b/xrspatial/geotiff/tests/test_vrt_source_tile_check_1823.py index b02dd2fb..14df1c00 100644 --- a/xrspatial/geotiff/tests/test_vrt_source_tile_check_1823.py +++ b/xrspatial/geotiff/tests/test_vrt_source_tile_check_1823.py @@ -2,14 +2,17 @@ PR #1803 forwarded the caller's ``max_pixels`` to ``read_to_array`` inside the VRT source loop so that a tiny VRT output could not force a huge -source decode (#1796). The output-window check enforces that. A separate -per-tile dimension check was incorrectly using the same ``max_pixels`` -value, so a caller setting ``max_pixels`` as an output budget (e.g. -10,000) would also fail the per-tile sanity check on every normal source -whose default tile size is 256x256 (= 65,536 pixels). - -The #1796 protection remains: the output-window check still catches a -tiny VRT output that asks for a large source window. +source decode (#1796). A separate per-tile dimension check was +incorrectly using the same ``max_pixels`` value, so a caller setting +``max_pixels`` as an output budget (e.g. 10,000) would also fail the +per-tile sanity check on every normal source whose default tile size is +256x256 (= 65,536 pixels). + +After PR #1821 (#1704) the resample path reads only the minimal source +sub-rect that feeds the clipped destination sub-window, so a tiny VRT +output cannot force a huge source decode by construction. The +output-extent check at the top of ``read_vrt`` still rejects requests +whose output itself exceeds ``max_pixels``. """ from __future__ import annotations @@ -78,20 +81,27 @@ def test_normal_tile_source_with_tiny_max_pixels(self): class TestOutputWindowCheckStillEnforced: - """The output-window check at the source read still rejects an - over-budget read; the #1796 protection is preserved.""" - - def test_output_window_exceeds_max_pixels_still_rejected(self): + """The output-window check still rejects a read whose VRT extent + exceeds ``max_pixels``. After #1704 the source read is bounded by + the clipped destination sub-window, so the per-source guard now + rarely fires; the top-level ``_check_dimensions`` call against the + output extent catches over-budget requests up front. The #1796 + protection (tiny VRT cannot force huge source decode) is preserved + structurally. + """ + + def test_output_extent_exceeds_max_pixels_still_rejected(self): with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as td: src = os.path.join(td, 'src.tif') - to_geotiff(np.arange(16, dtype=np.uint8).reshape(4, 4), - src, compression='none') - vrt = _write_vrt(td, dst_x_size=1, dst_y_size=1, - raster_x=1, raster_y=1, + to_geotiff(np.arange(64, dtype=np.uint8).reshape(8, 8), + src, compression='none', tiled=False) + # VRT output extent is 8x8 = 64 pixels; max_pixels=4 trips + # the dimensions check before any source read runs. + vrt = _write_vrt(td, dst_x_size=8, dst_y_size=8, + raster_x=8, raster_y=8, src_x_size=4, src_y_size=4) - # SrcRect 4x4 = 16 pixels > max_pixels=1 → output check fires. - with pytest.raises(ValueError, match="exceed"): - read_vrt(vrt, max_pixels=1) + with pytest.raises(ValueError, match="exceed|safety limit"): + read_vrt(vrt, max_pixels=4) class TestPerTileCheckStillRejectsCraftedHeader: