Code

Opened 3 years ago

Closed 3 years ago

Last modified 15 months 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 3 years ago.
postgis-adapter-2.patch (1.2 KB) - added by piro 3 years ago.
Better patch, slightly more efficient

Download all attachments as: .zip

Change History (19)

Changed 3 years ago by piro

comment:1 Changed 3 years ago by jpaulett

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready 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


Changed 3 years ago by piro

Better patch, slightly more efficient

comment:2 Changed 3 years ago by piro

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 3 years ago by piro (previous) (diff)

comment:3 Changed 3 years ago by jbronn

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

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 Changed 2 years ago by remco@…

Will this fix be included in Django 1.3.2?

comment:5 Changed 2 years ago by pjdelport

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 follow-ups: Changed 2 years ago by 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.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 2 years ago by anonymous

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 2 years ago by ramiro (previous) (diff)

comment:8 in reply to: ↑ 7 Changed 2 years ago by stephaner

Replying to anonymous:

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

comment:9 Changed 2 years ago by ramiro

Sorry, anonymous from comment:7 was me.

comment:10 follow-up: Changed 2 years ago by 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?

comment:11 in reply to: ↑ 10 Changed 2 years ago by anonymous

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 Changed 2 years ago by dpifke

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

comment:13 in reply to: ↑ 6 Changed 21 months ago by smcoll

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 Changed 15 months ago by visgean@…

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 Changed 15 months ago by visgean@…

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

comment:16 Changed 15 months ago by claudep

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 Changed 15 months ago by visgean@…

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.