Opened 3 weeks ago
Last modified 3 weeks ago
#37193 closed Cleanup/optimization
ogrinfo() does not validate num_features, so None and negative values give surprising results — at Version 1
| Reported by: | Caleb Jeffery Teye | Owned by: | |
|---|---|---|---|
| Component: | GIS | Version: | dev |
| Severity: | Normal | Keywords: | ogrinfo |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
django.contrib.gis.utils.ogrinfo(data_source, num_features=10) is documented (and named) as taking the number of features to display per layer. Internally, it passes the argument straight into a list slice:
# django/contrib/gis/utils/ogrinfo.py
for j, feature in enumerate(layer[:num_features]):
...
Because the value isn't validated, Python's slicing rules leak through and produce results that don't match what the parameter name implies:
>>> from django.contrib.gis.utils import ogrinfo
>>> ogrinfo("cities.shp", num_features=None) # prints *every* feature, not none
>>> ogrinfo("cities.shp", num_features=-2) # prints all features except the last two
A caller reasonably expecting num_features to mean "how many features to show" gets neither an error nor the requested count.
ogrinfo is exported in all, so this is public API. The fix should decide on the intended contract — most likely validating that num_features is a non-negative integer and raising TypeError/ValueError otherwise — and add tests. Any change in behaviour for None/negative input should be weighed for backwards compatibility (possibly a deprecation path).
Spotted while documenting ogrinfo() in #25927 — see the review discussion on PR #21443.