fix: validate Geo filter arguments before they reach the query string

[limjoobin]

Fixes #723

Summary

GeoSpec validated only its unit. longitude and latitude were stored as given despite their float annotations, and every one of the four arguments is interpolated into a RediSearch query by Geo.__str__.

A str coordinate carrying a ] closed the geo clause and had its remainder parsed as syntax. Similar to #721, an injected | binds looser than the implicit space-AND, so it lifts to the root of the parse tree and any filter sharing the query stops constraining it:

crafted = "-122.4194 37.7749 10 km] | @secret:{leaked}"
str(Geo("location") == GeoRadius(crafted, 37.7749, 1))
# before: @location:[-122.4194 37.7749 10 km] | @secret:{leaked} 37.7749 1 km]
# after:  TypeError: GeoRadius longitude must be an int, a float, or another
#         numbers.Real; got str

The unit had the same shape by a different route, and this one is not in the issue as filed. unit.lower() not in self.GEO_UNITS is an equality test the caller controls, while self._unit = unit.lower() stored the caller's object. A str subclass is neutralised by str.lower() returning a builtin, but an object whose lower() returns self and whose __eq__ matches "km" passed the check and then rendered its own __str__:

class Kilometres:
    def lower(self): return self
    def __eq__(self, other): return other == "km"
    def __hash__(self): return hash("km")
    def __str__(self): return "km] | @secret:{leaked}"

str(Geo("geo_field") == GeoRadius(1.0, 2.0, 3, Kilometres()))
# before: @geo_field:[1.0 2.0 3 km] | @secret:{leaked}]
# after:  @geo_field:[1.0 2.0 3 km]

Coordinates were also unbounded, so GeoRadius(9999, -9999, 1) and a NaN coordinate both rendered queries the server rejects, failing far from the line that caused them.

Changes

1. Coordinates are coerced and range-checked at the constructor

GeoSpec.__init__ routes both coordinates through _coerce_to_number_within, which coerces to a builtin and requires a finite value inside an inclusive range — LONGITUDE_RANGE = (-180.0, 180.0) and LATITUDE_RANGE = (-90.0, 90.0), class attributes beside the existing GEO_UNITS so the numeric and unit domains share a namespace and a subclass can widen either.

Constructor-time rather than render-time is deliberate. The error names the offending argument at the caller's line instead of surfacing from inside __str__, and str() is on the query path so it should not re-validate on every call. The bounds are inclusive because the antimeridian and the poles are real places.

isfinite is checked separately rather than left to the range comparison, so an infinite bound could not admit an infinite value. That much is unreachable through this class's own finite ranges and is covered by a subclass test. The ordering is load-bearing for a second and entirely reachable reason, though: isfinite raises OverflowError on an int too large to convert to a float, so the range is compared first and rejects 10**400 as the documented ValueError.

2. The unit stores `GEO_UNITS' own spelling

_canonical_unit returns the matched element of GEO_UNITS rather than the value that matched it. The caller's __eq__ still decides whether a unit matches; it no longer supplies what renders. There is deliberately no isinstance(unit, str) check, which would reject a legitimate str subclass — canonicalisation makes the identity of lower()'s return value irrelevant, since it is only ever compared, never rendered. GEO_UNITS stays the single source of truth for both the lookup and the error message.

Unit validation still runs before the coordinates, so a caller passing both a bad unit and a bad coordinate sees the error they saw before.

3. The radius is guarded explicitly, and stops being truncated

%i was doing two jobs badly. As a type guard it worked by accident, raising TypeError on a str, which left a format specifier load-bearing where nothing documented it. As a formatter it was simply wrong: it truncates toward zero, so a fractional radius silently queried a smaller circle than the caller asked for, and a sub-unit radius rendered 0, which the server rejects outright.

Measured on 8.4.5 against a document 1.5 km from the centre:

Caller writes	Rendered before	Hits	Rendered now	Hits
radius=1.9, unit="km"	1 km	1	1.9 km	2
radius=0.5, unit="km"	0 km	Invalid GeoFilter radius	0.5 km	1

So the radius now renders through %s, and GeoRadius coerces it — that coercion is the type guard, explicitly, with a test that fails without it. An integral float renders as an int, because repr switches to exponent form at 1e16 and 1e+16 is a syntax error at DIALECT 1, the Redis 8 server default.

The radius is also bounded now: zero, negative and infinite are refused at the constructor. Measured, the server answers Invalid GeoFilter radius to zero and to a negative, and an infinite radius previously escaped as an OverflowError from inside Geo.__str__. The bound is a chained comparison rather than math.isfinite, which raises OverflowError on an int too large to convert — a radius has no upper bound to reject such a value first, unlike a coordinate.

4. The numeric coercion moves to module scope

Num._coerce_numeric from #721 becomes the module-level _coerce_to_number, because GeoSpec is not a FilterField and could not inherit it. Num._coerce_numeric stays as a one-line delegator binding cls.__name__, so error text is unchanged and Timestamp still reports its own name through cls.

Tests

30 geo cases in tests/unit/test_filter.py, sharing a _geo_radius(**overrides) builder so each row names only the argument it changes. _StrOverridingInt/_StrOverridingFloat are reused from #721 rather than duplicated per filter type. Every guard was mutation-checked by reverting it alone, including both OPERATOR_MAP templates separately, since __ne__ is otherwise asserted nowhere.

The geo filter in tests/integration/test_query.py now uses a fractional radius, so a truncation regression fails against a real server rather than only against a string comparison. Verified: reverting the template alone turns that test red with Invalid GeoFilter radius.

Release Notes

Geo filter arguments are now validated where they are supplied, rather than reaching the query string unchecked. A str coordinate could previously close the geo clause and have its remainder parsed as RediSearch syntax, so a filter built from untrusted input could be widened past the scope it was meant to enforce. Upgrade if any geo filter argument in your application originates from user input.

One change moves results silently and deserves attention before you upgrade. A fractional radius used to be truncated toward zero, so GeoRadius(lon, lat, 1.9, "km") queried a 1 km circle; it now queries 1.9 km and returns the rows it always should have. Nobody gets an error from this, only different rows.

Three changes are backwards-incompatible, all of them turning a value that used to build a broken query into an error at the line that supplied it. Decimal coordinates now raise TypeError, having rendered successfully before, so a coordinate arriving from a NUMERIC database column or from json.loads(payload, parse_float=Decimal) has to be passed through float() first. A zero, negative or infinite radius now raises ValueError, where zero and negative previously built a query the server rejected with Invalid GeoFilter radius and infinite raised OverflowError from inside the formatter. And a coordinate outside its range now raises ValueError instead of building a query the server refuses.

[vishal-bala]

The injection is closed, and the reasoning in the description holds up. I reproduced the original hole against Redis 8.4.5 (search module 80410): a crafted latitude gives a UNION root under DIALECT 2 and leaks 3 documents where 1 is correct, and this branch refuses it at the constructor. A separate pass tried nineteen bypasses of the new guard, including numbers.Real subclasses overriding __str__/__repr__/__format__, a nondeterministic __float__ aimed at the range check, __index__-only integrals, numpy scalars, and str subclasses overriding lower(). Every one either rendered a plain numeric literal or raised. Validating in GeoSpec.__init__ is also the right layer: the spec holds all three arguments before any field is in scope, so the constructor is the only place that can own the check.

Two things should change before this merges, and the first is the argument the description is most confident about.

The radius is coerced but never bounded, and %i truncates it. That combination turns a documented value class into silently wrong results. Point 3 of the description is right that the guard should not rest on a format specifier, but the fix makes the constructor accept floats that %i cannot render. Measured against a document planted roughly 1.5 km north of the centre:

Caller writes	Renders as	Server returns
radius=1.9, unit="km"	1 km	1 hit, losing the 1.5 km document
the query that was asked for	1.9 km	2 hits
radius=0.5, unit="km"	0 km	Invalid GeoFilter radius
radius=-5	-5 km	Invalid GeoFilter radius

The first row is the one that matters: no exception, no warning, just a narrower result set. RediSearch accepts a fractional radius, so this is the template discarding precision rather than a server limit.

Second, rejecting Decimal is an unflagged behaviour break. Geo("l") == GeoRadius(Decimal("-122.4194"), Decimal("37.7749"), 1) renders @l:[-122.4194 37.7749 1 km] on main and raises TypeError here. Num already rejected Decimal, so this is Geo-only, and #721 treated the same rejection as a breaking change and said so under ## Release Notes. This one needs either the same note or a widened guard.

Everything else is optional. Longitude range, the unit list, case-insensitive unit handling, the inclusive endpoints and the DIALECT 2 premise all check out against the documentation and the server, and the LONGITUDE_RANGE/LATITUDE_RANGE endpoints mirror the query parser exactly (@loc:[180 0 3 km] parses, 180.00001 does not).

[vishal-bala]

Taking this over with Joo Bin's permission so it can ship today. Rebased onto main and pushed the fixes in 54271a0; his authorship is preserved on 0869a1f and the commit is co-authored.

The rebase needed --onto, because the branch carried the pre-squash version of #721 underneath the geo commit. A plain rebase would have replayed stale hunks over the merged fix.

Both must-fix items are closed.

The radius no longer truncates. %i became %s, which is safe because the coercion this PR added is the real type guard. Measured on 8.4.5 against a document 1.5 km out, 1.9 km now returns 2 hits where it returned 1. An integral float renders as an int, because repr switches to exponent form at 1e16 and 1e+16 is a syntax error at DIALECT 1, the server default — without that, the flip would have regressed large radii, since %i rendered them digit by digit.

The radius is bounded: zero, negative and infinite are refused at the constructor. GeoSpec's docstring no longer claims the radius is checked there.

Decimal is still rejected, matching #721, and the break is now documented under ## Release Notes in the body along with the silent result-set change, which is the one nobody can catch with a try/except.

The optional items are in too, plus four things the second review round turned up:

• GEO_UNITS is an annotated tuple. The bare tuple broke subclass widening under mypy, which the body advertises.
• A non-string unit leaves as the documented ValueError, found by lookup rather than by catching, so an AttributeError from inside a caller's own lower() still propagates.
• _coerce_to_number_within compares the range before math.isfinite, and float() is guarded for a Real that is finite but unrepresentable, e.g. Fraction(10**400, 1).
• The pole limit moved to the Geo class docstring. On latitude it contradicted the "-90 to 90" bullet it sat in, and it applies to every geo query rather than one argument.

Two corrections to my own review, both found by mutation testing rather than reading.

I was wrong to call latitude_below_range redundant. Widening LATITUDE_RANGE to (-180.0, 90.0) passed the whole suite without it, so it was the sole killer of that bound. Restored.

And the __ne__ template's radius specifier was unpinned — reverting only that one stayed green, because test_geo_filter[ne] used a whole radius. It now uses 0.5, so both templates are pinned and the dedicated render row is gone.

All eleven guards are mutation-checked by reverting each alone. The integration geo filter uses a fractional radius, so a truncation regression fails against a real server — verified: reverting the template turns it red with Invalid GeoFilter radius. 157 unit tests pass, mypy clean.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 54271a0. Configure here.}

[applied-ai-release-bot]

🚀 PR was released in v0.27.1 🚀