From a7863c78b7f3619a8d3d27e67c7284a349058c08 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 6 Aug 2016 11:14:41 +0200 Subject: [PATCH] Fixed #27024 -- Prevented logging error with empty string as geometry widget value Thanks Gavin Wahl for the report, and Tim Graham for the review. --- django/contrib/gis/admin/widgets.py | 2 +- django/contrib/gis/forms/widgets.py | 2 +- tests/gis_tests/geoadmin/tests.py | 30 ++++++++++++++++++++ tests/gis_tests/test_geoforms.py | 43 +++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/django/contrib/gis/admin/widgets.py b/django/contrib/gis/admin/widgets.py index abf38ddac4..bf6340d239 100644 --- a/django/contrib/gis/admin/widgets.py +++ b/django/contrib/gis/admin/widgets.py @@ -31,7 +31,7 @@ class OpenLayersWidget(Textarea): # If a string reaches here (via a validation error on another # field) then just reconstruct the Geometry. - if isinstance(value, six.string_types): + if value and isinstance(value, six.string_types): try: value = GEOSGeometry(value) except (GEOSException, ValueError) as err: diff --git a/django/contrib/gis/forms/widgets.py b/django/contrib/gis/forms/widgets.py index efa76a0b2d..c27cd90d3e 100644 --- a/django/contrib/gis/forms/widgets.py +++ b/django/contrib/gis/forms/widgets.py @@ -46,7 +46,7 @@ class BaseGeometryWidget(Widget): def render(self, name, value, attrs=None): # If a string reaches here (via a validation error on another # field) then just reconstruct the Geometry. - if isinstance(value, six.string_types): + if value and isinstance(value, six.string_types): value = self.deserialize(value) if value: diff --git a/tests/gis_tests/geoadmin/tests.py b/tests/gis_tests/geoadmin/tests.py index fc6c031dc0..2658ef32e8 100644 --- a/tests/gis_tests/geoadmin/tests.py +++ b/tests/gis_tests/geoadmin/tests.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals from django.contrib.gis import admin from django.contrib.gis.geos import Point from django.test import TestCase, override_settings, skipUnlessDBFeature +from django.test.utils import patch_logger from .admin import UnmodifiableAdmin from .models import City, site @@ -71,3 +72,32 @@ class GeoAdminTest(TestCase): self.assertFalse(has_changed(initial, data_same)) self.assertFalse(has_changed(initial, data_almost_same)) self.assertTrue(has_changed(initial, data_changed)) + + def test_olwidget_empty_string(self): + geoadmin = site._registry[City] + form = geoadmin.get_changelist_form(None)({'point': ''}) + with patch_logger('django.contrib.gis', 'error') as logger_calls: + output = str(form['point']) + self.assertInHTML( + '', + output + ) + self.assertEqual(logger_calls, []) + + def test_olwidget_invalid_string(self): + geoadmin = site._registry[City] + form = geoadmin.get_changelist_form(None)({'point': 'INVALID()'}) + with patch_logger('django.contrib.gis', 'error') as logger_calls: + output = str(form['point']) + self.assertInHTML( + '', + output + ) + self.assertEqual(len(logger_calls), 1) + self.assertEqual( + logger_calls[0], + "Error creating geometry from value 'INVALID()' (String or unicode input " + "unrecognized as WKT EWKT, and HEXEWKB.)" + ) diff --git a/tests/gis_tests/test_geoforms.py b/tests/gis_tests/test_geoforms.py index 5425443a82..f4b6e299b5 100644 --- a/tests/gis_tests/test_geoforms.py +++ b/tests/gis_tests/test_geoforms.py @@ -2,6 +2,7 @@ from django.contrib.gis import forms from django.contrib.gis.geos import GEOSGeometry from django.forms import ValidationError from django.test import SimpleTestCase, override_settings, skipUnlessDBFeature +from django.test.utils import patch_logger from django.utils.html import escape @@ -84,6 +85,48 @@ class GeometryFieldTest(SimpleTestCase): form = PointForm(data={'pt': 'POINT(5 23)'}, initial={'pt': point}) self.assertFalse(form.has_changed()) + def test_field_string_value(self): + """ + Initialization of a geometry field with a valid/empty/invalid string. + Only the invalid string should trigger an error log entry. + """ + class PointForm(forms.Form): + pt1 = forms.PointField(srid=4326) + pt2 = forms.PointField(srid=4326) + pt3 = forms.PointField(srid=4326) + + form = PointForm({ + 'pt1': 'SRID=4326;POINT(7.3 44)', # valid + 'pt2': '', # empty + 'pt3': 'PNT(0)', # invalid + }) + + with patch_logger('django.contrib.gis', 'error') as logger_calls: + output = str(form) + + self.assertInHTML( + '', + output + ) + self.assertInHTML( + '', + output + ) + self.assertInHTML( + '', + output + ) + # Only the invalid PNT(0) should trigger an error log entry + self.assertEqual(len(logger_calls), 1) + self.assertEqual( + logger_calls[0], + "Error creating geometry from value 'PNT(0)' (String or unicode input " + "unrecognized as WKT EWKT, and HEXEWKB.)" + ) + @skipUnlessDBFeature("gis_enabled") class SpecializedFieldTest(SimpleTestCase):