Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29610 closed Uncategorized (invalid)

Crash in sqlite3 last_executed_query when using printf() formatting

Reported by: Matthias Kestenholz Owned by: nobody
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here's a minimal testcase which shows the problem:

diff --git a/tests/backends/sqlite/tests.py b/tests/backends/sqlite/tests.py                                                                                                                                                           
index c82ed1667d..297b658810 100644                                                                                                                                                                                                    
--- a/tests/backends/sqlite/tests.py                                                                                                                                                                                                   
+++ b/tests/backends/sqlite/tests.py                                                                                                                                                                                                   
@@ -128,6 +128,12 @@ class LastExecutedQueryTest(TestCase):
             # This should not raise an exception.
             cursor.db.ops.last_executed_query(cursor.cursor, sql, params)
 
+    def test_printf(self):
+        with connection.cursor() as cursor:
+            sql = "SELECT printf(\"%x\", 10), %s"
+            params = ["hello"]
+            cursor.db.ops.last_executed_query(cursor.cursor, sql, params)
+
 
 @unittest.skipUnless(connection.vendor == 'sqlite', 'SQLite tests')
 class EscapingChecks(TestCase):

Here's the exception I got:

ERROR: test_printf (backends.sqlite.tests.LastExecutedQueryTest)                                                                                                                                                                       
----------------------------------------------------------------------                                                                                                                                                                 
Traceback (most recent call last):                                                                                                                                                                                                     
  File "/home/matthias/Projects/django/tests/backends/sqlite/tests.py", line 135, in test_printf                                                                                                                                       
    cursor.db.ops.last_executed_query(cursor.cursor, sql, params)                                                                                                                                                                      
  File "/home/matthias/Projects/django/django/db/backends/sqlite3/operations.py", line 147, in last_executed_query                                                                                                                     
    return sql % params                                                                                                                                                                                                                
TypeError: %x format: an integer is required, not str    

A real world example of using printf() can be found here: https://github.com/matthiask/django-tree-queries/blob/7eb6996203ad128dcef8fc88b29afa87dc2a4164/tree_queries/compiler.py#L69

Change History (2)

comment:1 by Ramiro Morales, 6 years ago

Type: BugUncategorized

If you want the raw SQL "%x" formatting specifier to reach the sqlite printf function you'll need to double the % so it gets passed "un-interpreted" by the query building stage.

We have documentation about this requirement in other areas all of them related to raw SQL handling (circumventing the ORM):

It could argued a similar note should be added to the connection.cursor.db.ops.last_executed_query() docs but currently it isn't documented, which IMHO isn't strange as it isn't a public API at all.

Do we want it to be a public API? The answer to this question would decide if this ticket should be converted to a documentation issue or closed as wontfix.

Or we could accept a patch for the last_executed_query() docstring.

Version 0, edited 6 years ago by Ramiro Morales (next)

comment:2 by Matthias Kestenholz, 6 years ago

Resolution: invalid
Status: newclosed

Oh wow, thanks! I'll retire this bug report. Someone else may reopen if they think that something should be done about this.

I definitely don't think making last_executed_query public API by documenting it is the right way to go. I'm using str.format in this project and it simply didn't occur to me that the printf argument should be escaped too (since everything worked fine as is)

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