Opened 9 months ago

Closed 7 months ago

#28981 closed Cleanup/optimization (fixed)

GeoIP2 should error when GEOIP_PATH exists but there is no MaxMind database

Reported by: Hugo Rodger-Brown Owned by: Alex Stovbur
Component: GIS Version: 1.11
Severity: Normal Keywords: geoip2 maxmind GEOIP_PATH
Cc: Alex Stovbur Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Hugo Rodger-Brown)

When initialising a GeoIP2 object, if the GEOIP_PATH setting points to a directory that exists, but there is no MaxMind database in that location, the object is created in a state such that any call to obj.__repr__ or obj.info() will raise an AttributeError.

Expected behaviour:

If it is possible to create a new GeoIP2 object in the absence of a source database, then the object methods should either handle that situation gracefully, or raise an appropriate error (e.g. GeoIP2Exception).

Actual behaviour:

If you create object and then call either __repr__ (which can be done implicitly by calling print(obj)) or info methods, an AttributeError is raised.

>>> from django.contrib.gis.geoip2 import GeoIP2
>>> g = GeoIP2()  # the GEOIP_PATH exists, but there is no database
>>> print(g)
# traceback truncated
---> meta = self._reader.metadata()
AttributeError: 'NoneType' object has no attribute 'metadata'
>>> g.info()
# traceback truncated
---> meta = self._reader.metadata()
AttributeError: 'NoneType' object has no attribute 'metadata'

The problem is caused by both methods assuming that the _reader attribute will always be a valid geoip2.database.Reader object, which is not true in this case:

>>> from django.contrib.gis.geoip2 import GeoIP2
>>> g = GeoIP2()  # the GEOIP_PATH exists, but there is no database
>>> assert g._city is None
>>> assert g._country is None
>>> assert g._reader is None

Change History (7)

comment:1 Changed 9 months ago by Hugo Rodger-Brown

Description: modified (diff)

comment:2 Changed 9 months ago by Hugo Rodger-Brown

The cleanest solution to this is to raise a GeoIP2Exception in the initialisation if a path is specified but no valid database can be found. The init method already does this for all other variations (invalid path, invalid database format).

comment:3 Changed 9 months ago by Tim Graham

Summary: django.contrib.gis.geoip2.GeoIP2 issue when GEOIP_PATH exists but there is no MaxMind databaseGeoIP2 should error when GEOIP_PATH exists but there is no MaxMind database
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

comment:4 Changed 7 months ago by Alex Stovbur

Owner: changed from nobody to Alex Stovbur
Status: newassigned

I'll take, if no one mind this.

comment:5 Changed 7 months ago by Alex Stovbur

Cc: Alex Stovbur added
Has patch: set

comment:7 Changed 7 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In d171843f:

Fixed #28981 -- Added an exception if GeoIP database can't be loaded from the path.

Note: See TracTickets for help on using tickets.
Back to Top