geotiff: validate overview_level type#2079
Merged
Merged
Conversation
overview_level was compared numerically with no type check, so True was coerced to 1 and silently returned the first overview level, and non-int types leaked raw TypeErrors from internal comparison or list indexing. Validate at the open_geotiff entry point and at the internal select_overview_ifd selector (defense in depth for the dask/GPU/accessor readers that forward the kwarg) and raise a TypeError that names the offending type and value.
) Review feedback: the previous isinstance(int) check rejected numpy.integer values like np.int64(1), which is inconsistent with _validate_tile_size and friends in _validation.py. Move the check into a shared _validate_overview_level_arg helper that accepts (int, np.integer) and rejects bool, then call it from both open_geotiff and select_overview_ifd so the two copies cannot drift. Add tests for np.int64 / np.int32 at levels 0 and 1.
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.
Closes #2074
Summary
overview_levelat theopen_geotiffentry point: must beintorNone, withboolrejected explicitly (Python treatsboolas a subclass ofint, so without this guardoverview_level=Truesilently selected overview level 1).select_overview_ifdfor defense in depth: dask, GPU, and accessor readers forwardoverview_levelto this selector directly, so a uniformTypeErroris raised regardless of entry point.Test plan
test_overview_level_type_validation_2074.pycoversbool(True/False),str,float, plus the still-workingint(0),int(1), andNonecases.overview-related tests in the geotiff suite still pass.xrspatial/geotiff/suite: 3849 pass; 2 pre-existing failures unrelated to this change (test_predictor2_big_endian_gpu_1517,test_size_param_validation_gpu_vrt_1776) reproduce onmainas well.