Opened 11 years ago

Closed 11 years ago

Last modified 11 years 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 Claude Paroz)

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 by Claude Paroz, 11 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

comment:2 by Claude Paroz, 11 years ago

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 by Rhett Garber, 11 years ago

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 by Claude Paroz, 11 years ago

Has patch: set

comment:5 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 34b8a385588a051814f77481c875047c8bd23b37:

Fixed #21496 -- Fixed crash when GeometryField uses TextInput

Thanks Rhett Garber for the report and initial patch.

comment:6 by Claude Paroz <claude@…>, 11 years ago

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