Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#13648 closed Bug (fixed)

'%s' escaping support for sqlite3

Reported by: master Owned by: Luke Plant
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords: sqlite escape
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Somewhere between 1.2beta1 and svn r13307, the string processing of convert_query() changed.

Now the regex to escape '%%s' content for sqlite3 SQL is wrong and produces '%?'.
It doesn't throw any exception but it means nothing to the DB and you may have 'None' column values returned.

Note : for a complete working sqlite solution, you still have to apply the patch in #12268.

Attachments (3)

sqlite3_base_%s.diff​ (498 bytes ) - added by master 14 years ago.
all_patches.patch​ (2.4 KB ) - added by tzulberti 14 years ago.
The patch of the #12268 and #13648, and a test
sqlite_base_with_test.diff​ (1.4 KB ) - added by Guilherme Salgado 14 years ago.
a fix for just this issue; based on the fix from #15155

Download all attachments as: .zip

Change History (14)

by master, 14 years ago

Attachment: sqlite3_base_%s.diff​ added

comment:1 by tzulberti, 14 years ago

Owner: changed from nobody to tzulberti

by tzulberti, 14 years ago

Attachment: all_patches.patch​ added

The patch of the #12268 and #13648, and a test

comment:2 by tzulberti, 14 years ago

Has patch: unset
Patch needs improvement: set
Triage Stage: Unreviewed → Accepted

Added a patch with a test. The change patch uploaded by *master* makes the query works.
BUT, there is a problem that I wasn't able to found, that the returned value isn't the value returned by sqllite.
To test the value of sqllite, y made a simple case:

sqlite3 path.sql
sqlite> select strftime("%s", '2010-11-13', 'utc');
1289617200

comment:3 by tzulberti, 14 years ago

Owner: tzulberti removed

comment:4 by Ramiro Morales, 14 years ago

#15155 was a duplicate and contains a similar fix for this issue alone plus tests (latest patch for this ticket also has tests but it mixes a fix for another issue).

by Guilherme Salgado, 14 years ago

Attachment: sqlite_base_with_test.diff​ added

a fix for just this issue; based on the fix from #15155

comment:5 by Guilherme Salgado, 14 years ago

I proposed a patch just for the issue reported here (based on the one from #15155), but I think it makes more sense to join this ticket with #12268 as there doesn't seem to be much point in fixing just one of them.

comment:6 by Julien Phalip, 14 years ago

Severity: → Normal
Type: → Bug

comment:7 by Aymeric Augustin, 14 years ago

Easy pickings: unset

#15927 was a duplicate.

comment:8 by Aymeric Augustin, 14 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

The "patch needs improvement" flag applied to the first proposals; the latest patch by salgado looks fine.

I don't think it is useful to "join" this ticket with #12268, which would mean marking one as a duplicate of the other. Actually, it's rather necessary to fix this one before tackling #12268.

comment:9 by Luke Plant, 14 years ago

Severity: Normal → Release blocker

This is a regression and needs to be backported. Since it really ought to have been fixed before 1.3.X, it ought to have been backported to 1.2.X as well, and there is still a case for doing that.

comment:10 by Luke Plant, 14 years ago

Owner: set to Luke Plant
Resolution: → fixed
Status: new → closed

In [16209]:

Fixed #13648 - '%s' escaping support for sqlite3 regression.

Thanks to master for the report and initial patch, and salgado and others
for work on the patch.

comment:11 by Luke Plant, 14 years ago

In [16210]:

[1.3.X] Fixed #13648 - '%s' escaping support for sqlite3 regression.

Thanks to master for the report and initial patch, and salgado and others
for work on the patch.

Backport of [16209] from trunk.

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