Context Navigation

← Previous Ticket
Next Ticket →

#28105 closed Bug (fixed)

BaseGeometryWidget.get_context() crashes if attrs contains the name of an existing key

Reported by:	Dylan Verheul	Owned by:	Dylan Verheul
Component:	GIS	Version:	1.11
Severity:	Release blocker	Keywords:	gis, forms, widgets
Cc:		Triage Stage:	Accepted
Has patch:	no	Needs documentation:	no
Needs tests:	no	Patch needs improvement:	no
Easy pickings:	yes	UI/UX:	no

Description (last modified by Tim Graham)

Incontrib.gis.forms.widgets there is this part (line 67 in current master):

context = self.build_attrs(self.attrs, dict(
    name=name,
    module='geodjango_%s' % name.replace('-', '_'),  # JS-safe
    serialized=self.serialize(value),
    geom_type=gdal.OGRGeomType(self.attrs['geom_type']),
    STATIC_URL=settings.STATIC_URL,
    LANGUAGE_BIDI=translation.get_language_bidi(),
    **attrs
))

If attrs also contains a key 'geom_type' this leads to an inevitable crash.

This should probaly be something like:

context_kwargs = attrs.copy()
context_kwargs.update(dict(
    name=name,
    module='geodjango_%s' % name.replace('-', '_'),  # JS-safe
    serialized=self.serialize(value),
    geom_type=gdal.OGRGeomType(self.attrs['geom_type']),
    STATIC_URL=settings.STATIC_URL,
    LANGUAGE_BIDI=translation.get_language_bidi(),
))

Currently this causes django-bootstrap3 to fail for Django 1.11.

Change History (12)

comment:1 by Tim Graham, 8 years ago

Component:	Forms → GIS
Description:	modified (diff)
Severity:	Normal → Release blocker
Summary:	Missing check in django.contrib.forms.widgets → BaseGeometryWidget.get_context() crashes if attrs contains the name of an existing key
Triage Stage:	Unreviewed → Accepted

Could you explain more about the use case that causes the crash? If you could write a test for tests/gis_tests/test_geoforms.py, that would be ideal.

comment:2 by Dylan Verheul, 8 years ago

Owner:	changed from nobody to Dylan Verheul
Status:	new → assigned

I'll see what I can do to fix it myself.

comment:3 by Dylan Verheul, 8 years ago

Easy pickings:	set
Owner:	Dylan Verheul removed
Status:	assigned → new

The test suite keeps getting stuck on my Mac. Seems like this is easy pickings so I'll hope someone else picks it up.

comment:4 by Tim Graham, 8 years ago

You likely wouldn't need to run the entire test suite (see Running only some of the tests). Running gis_tests should be enough -- the pull request tester runs the entire suite.

On the mailing list you wrote, "My patch and tests are about done". If you don't continue, can you provide what you did so far?

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:5 by Dylan Verheul, 8 years ago

I'm no stranger to pyenv, virtualenv, git and unit tests. I have followed https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/working-with-git/ and https://docs.djangoproject.com/en/dev/internals/contributing/new-contributors/ and https://docs.djangoproject.com/en/dev/intro/contributing/.

Unfortunately the instructions for running the test suite end up in either a stuck test (have to abort it after several hours), essential tests skipped (because of sqlite instead of postgis I presume). I've sent quite a few hours because I really want to contribute, but it seems that it just does not work on my MacBook. A co-worker did get the test suite to run on his ubuntu machine. So, it's probably me.

I'm giving it a final try now. It's bothering me because the error is blatantly obvious.

comment:6 by Tim Graham, 8 years ago

If you copy tests/test_sqlite.py to tests/test_spatialite.py and change the 'ENGINE' in that file to 'django.contrib.gis.db.backends.spatialite', you should be able to run ./runtests.py --settings=test_spatialite gis_tests.test_geoforms. Ping me (timograham) in the #django-dev IRC channel if I can help further.