SQL error on Trac 0.12

[Daniel Westermann-Clark]

On Trac 0.12, an error results when moving a ticket:

ProgrammingError: operator does not exist: text = integer LINE 1: SELECT * FROM attachment WHERE id=17 ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.

This occurs on PostgreSQL databases, at least, due to its use of strict type checking.

A simple fix is to quote the value in the SQL statement. This works on PostgreSQL and SQLite databases.

0.11/ticketmoverplugin/ticketmover.py

@@ -188 +188 @@
 
         # copy the changelog and attachment DBs
         for table, _id in tables.items():
-            for row in get_all_dict(self.env, "SELECT * FROM %s WHERE %s=%s" % (table, _id, ticket_id)):
+            for row in get_all_dict(self.env, "SELECT * FROM %s WHERE %s = '%s'" % (table, _id, ticket_id)):
                 row[_id] = new_ticket.id
                 insert_row_from_dict(env, table, row)
 

[Ryan J Ollos]

#10720 closed as a duplicate.

[Daniel Westermann-Clark]

In case it's helpful, here is a more permanent link to the referenced patch:

​https://github.com/dwc/dwc-overlay/blob/abb8505f4cc4aa9a30c176f0bdd655a0d4c75ae6/dev-python/trac-ticket-mover/files/trac-ticket-mover-sql-quoting.patch

[Ryan J Ollos]

Thanks, I'll take care of this if no one beats me to it.

[Ryan J Ollos]

I have a slightly different patch that seems to fix the reported issue, and might also fix another issue.

• I think that we should not be using string formatting for ticket_id since the value comes from the request, and therefore doing so opens up the possibility for SQL injection. Replace,

  get_all_dict(self.env, "SELECT * FROM %s WHERE %s=%s" % (table, _id, ticket_id))
  

  with,

  get_all_dict(self.env, "SELECT * FROM %s WHERE %s=%%s" % (table, _id), str(ticket_id))
  

• In the aforementioned query, (table, _id) takes on the values ('attachment', 'id'), which is a TEXT field, and ('ticket_change', 'ticket'), which is an INTEGER field (​t:TracDev/DatabaseSchema#DatabaseSchema0.11). Given that the change shown above fixes this issue but results in a cast of ticket_id to a string for both fields, I have to assume that PostgreSQL allows an implicit cast of a string to an integer, but not an integer to a string.

[Ryan J Ollos]

(In [12956])

Fixes #7496: Perform an explict cast of ticket_id to a string to avoid a programming error on PostgreSQL, where strict type checking is done. Prevent possibility of SQL injection since ticket_id comes from the request.

[Steffen Hoffmann]

Replying to rjollos:

Prevent possibility of SQL injection since ticket_id comes from the request.

+1, better indeed. Definitely the way to go.

[Steffen Hoffmann]

Replying to dwc:

In case it's helpful, here is a more permanent link to the referenced patch:

Thanks for taking care. But the most "permanent" way to provide the patch, would have been to attach a patch file to this ticket, or just inline the patch into the description, so it is really self-contained and not relying on external resources. I've done so for you now.