Opened 22 months ago

Closed 21 months ago

Last modified 21 months ago

#21496 closed Bug (fixed)

Django 1.6 django.contrib.gis GeometryField crashes on alternate widget

Reported by: rhettg@… Owned by: nobody
Component: GIS Version: 1.6
Severity: Normal Keywords:
Cc: 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 claudep)

If I use a TextInput widget to edit a PointField, form validation crashes because the widget doesn't have a map_srid.

The string representation of a Point using the default srid (4326) doesn't define the SRID, which is unfortunate.

  >> p.srid
  4326
  >> str(p)
  'POINT (-122.3996132001327055 37.7942941983347467)'

  >> p2 = GEOSGeometry('POINT (-122.3996132001327055 37.7942941983347467)')
  >> p2.srid
  None

I think the best solution would be to allow the field to transform the value into whatever the default is set as for the field, unless otherwise specified by the widget.

So something like the following PR would be required:
https://github.com/django/django/pull/1966

Change History (6)

comment:1 Changed 22 months ago by claudep

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 22 months ago by claudep

There are two parts in your proposed pull request. First of all, the crash concerning the missing map_srid is bad and should be fixed, clearly. But a simple hasattr(self.widget, 'map_srid') would be enough, at least at first sight.

Then there is the part about where to convert the value into the srid of the field. This is debatable, but I'm currently not entirely convinced by your patch. I would probably move in to_python the srid attribution to the field srid when the value has no srid, but not the transform conversion. Anyway, this is a different issue, which we could discuss in another ticket.

So I suggest to only fix something like this (which I would also backport to 1.6):

--- a/django/contrib/gis/forms/fields.py
+++ b/django/contrib/gis/forms/fields.py
@@ -44,7 +44,7 @@ class GeometryField(forms.Field):
         if not isinstance(value, GEOSGeometry):
             try:
                 value = GEOSGeometry(value)
-                if not value.srid:
+                if not value.srid and hasattr(self.widget, 'map_srid'):
                     value.srid = self.widget.map_srid
             except (GEOSException, ValueError, TypeError):
                 raise forms.ValidationError(self.error_messages['invalid_geom'], code='invalid_geom')

... with an appropriate test (I have the patch almost ready).

comment:3 Changed 22 months ago by rhettg

I didn't mention this, but the other reason for moving the transform out of clean was because of another crash.

in _has_changed, this line fails:

 data.transform(initial.srid)

Since to_python would then allow geom objects without srids... the srid from the field itself doesn't get applied unless you call clean(), which _has_changed does not.

My use of try...except was mostly habit from EAFP (http://docs.python.org/2/glossary.html)

comment:4 Changed 21 months ago by claudep

  • Has patch set

comment:5 Changed 21 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 34b8a385588a051814f77481c875047c8bd23b37:

Fixed #21496 -- Fixed crash when GeometryField uses TextInput

Thanks Rhett Garber for the report and initial patch.

comment:6 Changed 21 months ago by Claude Paroz <claude@…>

In 14ddc1b517266916d9e729bc20234edec1d7bd4c:

[1.6.x] Fixed #21496 -- Fixed crash when GeometryField uses TextInput

Thanks Rhett Garber for the report and initial patch.
Backport of 34b8a3855 from master.

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