Modify

Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#7496 closed defect (fixed)

SQL error on Trac 0.12

Reported by: Daniel Westermann-Clark Owned by: Ryan J Ollos
Priority: normal Component: TicketMoverPlugin
Severity: normal Keywords:
Cc: Jun Omae, Steffen Hoffmann Trac Release: 0.12

Description (last modified by Steffen Hoffmann)

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

     
    188188
    189189        # copy the changelog and attachment DBs
    190190        for table, _id in tables.items():
    191             for row in get_all_dict(self.env, "SELECT * FROM %s WHERE %s=%s" % (table, _id, ticket_id)):
     191            for row in get_all_dict(self.env, "SELECT * FROM %s WHERE %s = '%s'" % (table, _id, ticket_id)):
    192192                row[_id] = new_ticket.id
    193193                insert_row_from_dict(env, table, row)
    194194

Attachments (0)

Change History (7)

comment:1 Changed 5 years ago by Ryan J Ollos

#10720 closed as a duplicate.

comment:2 Changed 5 years ago by Daniel Westermann-Clark

comment:3 Changed 5 years ago by Ryan J Ollos

Cc: Jun Omae added; anonymous removed
Owner: changed from Jeff Hammel to Ryan J Ollos
Status: newassigned

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

comment:4 Changed 5 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added

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.

comment:5 Changed 5 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

(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.

comment:6 in reply to:  5 Changed 5 years ago by 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.

comment:7 in reply to:  2 Changed 5 years ago by Steffen Hoffmann

Description: modified (diff)

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.