[Django] #28981: django.contrib.gis.geoip2.GeoIP2 issue when GEOIP_PATH exists but there is no MaxMind database
Django
no MaxMind database
-------------------------------------+-------------------------------------
Reporter: Hugo | Owner: nobody
Rodger-Brown |
Type: | Status: new
Uncategorized |
Component: GIS | Version: 1.11
Severity: Normal | Keywords: geoip2 maxmind
Triage Stage: | GEOIP_PATH
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
If a GEOIP_PATH setting is specified, but there is no database file at
that location, the initialisation of GeoIP2 runs without raising an error,
which is the expected behaviour. However, during this process, neither the
`_country` nor `_city` attributes are set (as there is no database), which
means in turn that the `_reader` property will always return `None`. The
problem occurs because both the `__repr__` and `info` methods expect the
`_reader` to exist, and will fail with an `AttributeError` if it is None.
The expected behaviour is that neither method will fail. The actual
behaviour can be replicated below, where GEOIP_PATH is a valid directory
that does not contain a MaxMind database file.
{{{#!python
>>> import os
>>> from django.contrib.gis.geoip2 import GeoIP2
>>> from django.contrib.gis.geoip2.base import GEOIP_SETTINGS
>>> assert GEOIP_SETTINGS['GEOIP_PATH']
>>> assert os.path.isdir(GEOIP_SETTINGS['GEOIP_PATH'])
>>> g = GeoIP2()
>>> assert g._city is None
>>> assert g._country is None
>>> assert g._reader is None
>>> 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'
}}}
Error occurs in
`https://github.com/django/django/blob/1.11.9/django/contrib/gis/geoip2/base.py`
Sugggested remedy: update `GeoIP2.__repr__` and `GeoIP2.info` to handle
`_reader is None`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28981>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
no MaxMind database
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: GIS | Version: 1.11
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage:
GEOIP_PATH | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
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.
{{{#!python
>>> 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:
{{{#!python
>>> 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
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:1>
Django
no MaxMind database
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage:
GEOIP_PATH | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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).
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Hugo Rodger-Brown | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: GIS | Version: 1.11
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage: Accepted
GEOIP_PATH |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Hugo Rodger-Brown | Owner: Alex
Type: | Status: assigned
Cleanup/optimization |
Component: GIS | Version: 1.11
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage: Accepted
GEOIP_PATH |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Alex
Comment:
I'll take, if no one mind this.
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Hugo Rodger-Brown | Owner: Alex
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage: Accepted
GEOIP_PATH |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Alex (added)
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Hugo Rodger-Brown | Owner: Alex
Type: | Stovbur
Cleanup/optimization | Status: assigned
Component: GIS | Version: 1.11
Severity: Normal | Resolution:
Keywords: geoip2 maxmind | Triage Stage: Accepted
GEOIP_PATH |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alex Stovbur):
PR https://github.com/django/django/pull/9749
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Hugo Rodger-Brown | Owner: Alex
Type: | Stovbur
Component: GIS | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: geoip2 maxmind | Triage Stage: Accepted
GEOIP_PATH |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d171843f5715b4c0162ab800539bd25ad60147c7" d171843f]:
{{{
#!CommitTicketReference repository=""
revision="d171843f5715b4c0162ab800539bd25ad60147c7"
Fixed #28981 -- Added an exception if GeoIP database can't be loaded from
the path.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28981#comment:7>