Fix crop=True dropping boundary pixels when all_touched=True (#1197)#1200
Merged
Conversation
Two security fixes for the geotiff subpackage: 1. Add a configurable max_pixels guard to read_to_array() and all internal read functions (_read_strips, _read_tiles, _read_cog_http). A crafted TIFF with fabricated header dimensions could previously trigger multi-TB allocations. The default limit is 1 billion pixels (~4 GB for float32 single-band), overridable via max_pixels kwarg. Fixes #1184. 2. Canonicalize VRT source filenames with os.path.realpath() after resolving relative paths. Previously, a VRT file with "../" in SourceFilename could read arbitrary files outside the VRT directory. Fixes #1185.
os.path.realpath() converts Unix-style paths to Windows paths on Windows (e.g. /data/tile.tif becomes D:\data\tile.tif). Use os.path.realpath() in the assertion so it matches the production code's canonicalization on all platforms.
_crop_to_bbox compared pixel center coordinates against the geometry bounding box without accounting for pixel cell extent. When all_touched=True, pixels whose centers fell just outside the bbox were excluded even though their cells overlapped the polygon. Now _crop_to_bbox receives the all_touched flag and expands the bbox comparison by half a pixel on each side when set, so rasterize gets to see every pixel whose cell intersects the geometry. Also removed dead ascending/descending branches that computed the same mask regardless of coordinate order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_crop_to_bboxwas comparing pixel center coordinates against the geometry bounding box without accounting for pixel cell extent. Whenall_touched=True, pixels whose centers fell just outside the bbox were silently excluded even though their cells overlapped the polygon._crop_to_bboxreceives theall_touchedflag and pads the bbox comparison by half a pixel on each side when set, so rasterize sees every pixel whose cell intersects the geometry.Closes #1197
Test plan
test_all_touched_crop_matches_nocropin numpy backend confirmscrop=True, all_touched=Trueproduces identical pixel counts and values ascrop=False, all_touched=Truefor a polygon whose edges land between pixel centers