ogrinfo() does not validate num_features, so None and negative values give surprising results

[Caleb Jeffery Teye]

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.

[David Smith]

What do you think about documenting it as part of #25927.

num_features is used as a list slice to limit the number of features returned. Typically, this is expected to be a non-negative integer.

[Caleb Jeffery Teye]

Replying to David Smith:

What do you think about documenting it as part of #25927.

num_features is used as a list slice to limit the number of features returned. Typically, this is expected to be a non-negative integer.

Sounds good to me. Let's document it as part of #25927. I've added a note to the num_features description:

num_features sets how many features are listed per layer, and defaults to 10. It is used as a list slice, so it should be a non-negative integer.

Once that lands, this ticket can be closed.

[maheen8q]

I'd like to work on this if it's still available. My plan is to add validation at the top of ogrinfo() that raises a ValueError for negative integers, and a TypeError for non-integer values. I'll leave the None behavior open for reviewer guidance given the backwards compatibility concerns mentioned in the description, and will include tests for the new validation.

[David Smith]

Hi Caleb -- I'll close this ticket to try and avoid folk working on something you've already well progressed. I'll leave a note on the other ticket back to this as well.