Opened 13 years ago

Closed 13 years ago

Last modified 11 years ago

#16778 closed Bug (fixed)

Bad geometry escape in postgis

Reported by: piro Owned by: nobody
Component: GIS Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Geometries for postgis are adapted using an escaping style only working with postgresql setting for standard_conforming_string = off. On is the default for PostgreSQL 9.1

The attached patch fixes the problem and works for any PG version and any SCS setting.

Also see http://psycopg.lighthouseapp.com/projects/62710/tickets/69

Attachments (2)

postgis-adapter.patch (1.4 KB ) - added by piro 13 years ago.
postgis-adapter-2.patch (1.2 KB ) - added by piro 13 years ago.
Better patch, slightly more efficient

Download all attachments as: .zip

Change History (19)

by piro, 13 years ago

Attachment: postgis-adapter.patch added

comment:1 by John Paulett, 13 years ago

Triage Stage: UnreviewedReady for checkin

Verified that the patch works.

1) Set standard_conforming_string = on in /etc/postgresql/8.4/main/postgresql.conf
2) Ran postgis test suite, which had multiple failures
3) Applied patch, re-ran test suite. All tests pass


by piro, 13 years ago

Attachment: postgis-adapter-2.patch added

Better patch, slightly more efficient

comment:2 by piro, 13 years ago

The original patch does some useless work.

Submitted new patch with a better written adapter, with straightforward prepare/getquoted methods. Please use it instead of the original patch.

Last edited 13 years ago by piro (previous) (diff)

comment:3 by jbronn, 13 years ago

Resolution: fixed
Status: newclosed

In [16826]:

Fixed #16778 -- Improved escaping of geometries on PostgreSQL, allowing GeoDjango to work on 9.1. Thanks, piro for ticket and patch.

comment:4 by remco@…, 12 years ago

Will this fix be included in Django 1.3.2?

comment:5 by Pi Delport, 12 years ago

It would be great if Django 1.3.2 includes this fix.

We're working around the bug at the moment by running PostgreSQL 9.1 with standard_conforming_string = off.

comment:6 by Stephane Raimbault, 12 years ago

Just for the sake of clarity, the PostgreSQL option is:

standard_conforming_strings = off

See the trailing s at the end.

Now that major Linux distributions have switched to PostgreSQL 9.1 (eg. Fedora 16, Ubuntu 11.10), this bug impacts many more people.
So as said by pjdelport, it would great to include this fix in Django 1.3.2.

in reply to:  6 ; comment:7 by anonymous, 12 years ago

Replying to stephaner:

Just for the sake of clarity, the PostgreSQL option is:

standard_conforming_strings = off

See the trailing s at the end.

Now that major Linux distributions have switched to PostgreSQL 9.1 (eg. Fedora 16, Ubuntu 11.10), this bug impacts many more people.
So as said by pjdelport, it would great to include this fix in Django 1.3.2.

The current policy for backporting fixes to maintenance branches (like 1.3.x) is that they only get fixes for data loss or security problems. So unfortunately that means this fix won't be backported to such branch.

The good news is that you can apply this four-line patch by yourself to your copy of Django. By doing this:

  • You know there is no other change to 1.3.1 apart than this fix (some shops have policies about only using officially released versions of software, this would mean breaking that but OTOH this item means you can keep risks low when you have heavily invested in and tested 1.3.1)
  • You don't have to wait for Django 1.3.2
  • You don't have to wait for Django 1.4

In practice, a line needs to be drawn regarding compatibility with features of other parts of the stack. Django 1.3 was releases in March 2011 and Postgres 9.1 in September 2011. If every release (or its corresponding maintenance branch) of Django had to be updated for every change of behavior of every new version of every piece of the software stack that appears in every new OS release, then the task of adapting to them would be a never ending one and

  1. Would increase risks of regressions/unintended changes of behavior (we have been bitten by this in the past)
  2. Would divert resources from development in trunk.
Last edited 12 years ago by Ramiro Morales (previous) (diff)

in reply to:  7 comment:8 by Stephane Raimbault, 12 years ago

Replying to anonymous:

Your position makes sense, I understand the maintenance burden it implies.

comment:9 by Ramiro Morales, 12 years ago

Sorry, anonymous from comment:7 was me.

comment:10 by anonymous, 12 years ago

I applied the patch, but I get the same error. I've tried restarting apache, do I need to run setup.py on Django again?

in reply to:  10 comment:11 by anonymous, 12 years ago

Replying to anonymous:

I applied the patch, but I get the same error. I've tried restarting apache, do I need to run setup.py on Django again?

nevermind... im stupid

comment:12 by dpifke, 12 years ago

Ubuntu 12.04LTS ships Django 1.3.1 and PostgreSQL 9.1, but does not contain this patch. I have requested this patch be included in the version of Django they are distributing:

https://bugs.launchpad.net/ubuntu/+source/python-django/+bug/1004204

in reply to:  6 comment:13 by smcoll, 12 years ago

It appears that this fix didn't make the 1.3.2 release, unfortunately.

Replying to stephaner:

Now that major Linux distributions have switched to PostgreSQL 9.1 (eg. Fedora 16, Ubuntu 11.10), this bug impacts many more people.
So as said by pjdelport, it would great to include this fix in Django 1.3.2.

comment:14 by visgean@…, 11 years ago

Hi I am using django '1.4.2' and I still get this error, when I tried to apply the patch I got following:

visgean@Rew:/usr/local/lib/python2.7/dist-packages$ sudo patch /home/visgean/postgis-adapter-2.patch django/contrib/gis/db/backends/postgis/adapter.py
patch: **** Only garbage was found in the patch input.

Am I doing something wrong?

comment:15 by visgean@…, 11 years ago

Oh I see now that I already have the patch included, though the problem is still there..

comment:16 by Claude Paroz, 11 years ago

This issue should be fixed in 1.4. If you really think you've hit a bug, please open a new ticket (optionally referencing this one) with a full traceback.

comment:17 by visgean@…, 11 years ago

I just managed to get it work with postgreqsql settings...

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