#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:2 by , 6 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
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)
If you want the raw SQL
"%x"
formatting specifier to reach the sqliteprintf
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.