Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

Last modified 3 years ago

#11571 closed defect (fixed)

Dynamic variable assignment in ticket reports fails in some cases

Reported by: Quince <eimacdude@…> Owned by: Jun Omae
Priority: normal Milestone: 0.12.6
Component: report system Version: 1.0.1
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fix report crashing if percent signs used in quoted-identifiers and dynamic variables used.

API Changes:

trac.db.api: sql_escape_percent escapes literals and quoted strings, now.

Internal Changes:

Description (last modified by Quince <eimacdude@…>)

The following ticket report query worked just fine with MySQL, but fails after migration to PostgreSQL:

SELECT p.value AS __color__, status AS __group__,
    id AS ticket, summary, t.type AS type, priority, milestone, component, version, s.value AS "Start", e.value AS "End", c.value AS "% Done", r.value as "Hours Left"
  FROM ticket t
  LEFT OUTER JOIN ticket_custom s ON (t.id = s.ticket AND s.name = 'due_assign')
  LEFT OUTER JOIN ticket_custom e ON (t.id = e.ticket AND e.name = 'due_close')
  LEFT OUTER JOIN ticket_custom c ON (t.id = c.ticket AND c.name = 'complete')
  LEFT OUTER JOIN ticket_custom r ON (t.id = r.ticket AND r.name = 'estimatedhours')
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  WHERE t.status <> 'closed' AND owner = $USER
  ORDER BY status = 'in_work' DESC, status = 'in_QA' DESC, status = 'assigned' DESC, status, CAST(p.value AS bigint)

Replacing $USER with the actual username works. However, this isn't the problem all by itself, because something like SELECT * FROM ticket as t WHERE owner = $USER does work fine. The error itself shows that in the above example, $USER is replaced with %s for whatever reason:

Report execution failed:

IndexError: list index out of range

SELECT COUNT(*) FROM (
SELECT p.value AS __color__, status AS __group__,
    id AS ticket, summary, t.type AS type, priority, milestone, component, version, s.value AS "Start", e.value AS "End", c.value AS "% Done", r.value as "Hours Left"
  FROM ticket t
  LEFT OUTER JOIN ticket_custom s ON (t.id = s.ticket AND s.name = 'due_assign')
  LEFT OUTER JOIN ticket_custom e ON (t.id = e.ticket AND e.name = 'due_close')
  LEFT OUTER JOIN ticket_custom c ON (t.id = c.ticket AND c.name = 'complete')
  LEFT OUTER JOIN ticket_custom r ON (t.id = r.ticket AND r.name = 'estimatedhours')
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  WHERE t.status <> 'closed' AND owner = %s
  ORDER BY status = 'in_work' DESC, status = 'in_QA' DESC, status = 'assigned' DESC, status, CAST(p.value AS bigint)
) AS tab

Why? How do I get around this error while waiting for a fix?

Here's another failure case where $USER gets replaced with %s instead of the username when running trac with PostgreSQL but wasn't an issue with MySQL:

SELECT p.value AS __color__,
    CONCAT('Milestone ', milestone, CASE WHEN m.due > 0 THEN CONCAT(' (due ', TO_TIMESTAMP(m.due/1000000)::TIMESTAMP, ')') ELSE '' END) AS __group__,
    (CASE status 
      WHEN 'closed' THEN 'color: #777; background: #ddd; border-color: #ccc;'
      ELSE (CASE owner WHEN $USER THEN 'font-weight: bold' END)
    END) AS __style__,
    id AS ticket, summary, owner, t.type AS type, status, component, version, s.value AS "Start", e.value AS "End", c.value AS "% Done", r.value as "Hours Left"
  FROM ticket t
  LEFT JOIN milestone m ON m.name = t.milestone
  LEFT OUTER JOIN ticket_custom s ON (t.id = s.ticket AND s.name = 'due_assign')
  LEFT OUTER JOIN ticket_custom e ON (t.id = e.ticket AND e.name = 'due_close')
  LEFT OUTER JOIN ticket_custom c ON (t.id = c.ticket AND c.name = 'complete')
  LEFT OUTER JOIN ticket_custom r ON (t.id = r.ticket AND r.name = 'estimatedhours')
  LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
  ORDER BY (m.due > 0) DESC, m.due, (milestone IS NULL), milestone, status = 'closed', (CASE status WHEN 'closed' THEN changetime ELSE (-1) * CAST(p.value AS bigint) END) DESC

Attachments (0)

Change History (6)

comment:1 by Quince <eimacdude@…>, 11 years ago

Description: modified (diff)

comment:2 by Quince <eimacdude@…>, 11 years ago

Well, on a whim I replaced "% Done" with "%% Done", which makes it work, but I don't know if this is really a correct solution, so I'll leave the ticket open until someone takes a look.

comment:3 by Quince <eimacdude@…>, 11 years ago

Yeah, %% isn't the right solution, since it's inconsistent. Using %% in tickets reports that don't make use of a dynamic variable like $USER makes the % appear duplicated as %% in the header resulting report, whereas using it in the above makes it appear as a single %. I'm sure this classifies as a bug.

comment:4 by Jun Omae, 11 years ago

Component: query systemreport system
Milestone: next-minor-0.12.x

Good catch! Thanks for the reporting.

Reproduced with the following cases.

  • PostgreSQL: SELECT 1 AS "%s", $USER
  • SQLite and MySQL: SELECT 1 AS `%s`, $USER

The percent sign in quoted identifier and dynamic variables in query lead the issue. Unfortunately, Trac doesn't log the error while executing report.

After patching to log, I got the following.

  • SQLite:
    2014-04-04 13:08:48,947 Trac[report] WARNING: Exception caught while executing report for count: SELECT COUNT(*) FROM (
    SELECT 1 AS `%s`, %s
    ) AS tab, args [u'jun66j5']
    Traceback (most recent call last):
      File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/report.py", line 683, in execute_paginated_report
        cursor.execute(count_sql, args)
      File "/home/jun66j5/src/trac/edgewall/git/trac/db/util.py", line 65, in execute
        return self.cursor.execute(sql_escape_percent(sql), args)
      File "/home/jun66j5/src/trac/edgewall/git/trac/db/sqlite_backend.py", line 78, in execute
        result = PyFormatCursor.execute(self, *args)
      File "/home/jun66j5/src/trac/edgewall/git/trac/db/sqlite_backend.py", line 54, in execute
        sql = sql % (('?',) * len(args))
    TypeError: not enough arguments for format string
    
  • PostgreSQL:
    2014-04-04 13:10:02,400 Trac[report] WARNING: Exception caught while executing report for count: SELECT COUNT(*) FROM (
    SELECT 1 AS "%s", %s
    ) AS tab, args [u'jun66j5']
    Traceback (most recent call last):
      File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/report.py", line 683, in execute_paginated_report
        cursor.execute(count_sql, args)
      File "/home/jun66j5/src/trac/edgewall/git/trac/db/util.py", line 65, in execute
        return self.cursor.execute(sql_escape_percent(sql), args)
    IndexError: list index out of range
    
  • MySQL:
    2014-04-04 13:23:29,053 Trac[report] WARNING: Exception caught while executing report for count: SELECT COUNT(*) FROM (
    SELECT 1 AS `%s`, %s
    ) AS tab, args [u'jun66j5']
    Traceback (most recent call last):
      File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/report.py", line 683, in execute_paginated_report
        cursor.execute(count_sql, args)
      File "/home/jun66j5/src/trac/edgewall/git/trac/db/util.py", line 65, in execute
        return self.cursor.execute(sql_escape_percent(sql), args)
      File "/usr/lib/python2.5/site-packages/MySQLdb/cursors.py", line 159, in execute
        query = query % db.literal(args)
    TypeError: not enough arguments for format string
    
Last edited 11 years ago by Jun Omae (previous) (diff)

comment:5 by Jun Omae, 11 years ago

Milestone: next-minor-0.12.x0.12.6
Owner: set to Jun Omae
Status: newassigned

Currently, we escape percent characters in literals. I think we should also escape the characters in quoted-identifiers.

Proposed changes in jomae.git@t11571_0.12 and jomae.git@t11571_1.0. I've verified with the following environments.

  • SQLite 3.3.6 and 3.7.9
  • PostgreSQL 8.1.23 and 9.1.12
  • MySQL 5.0.95 and 5.5.35

comment:6 by Jun Omae, 11 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to 0.12-stable in [12668] and merged to 1.0-stable in [12669] and trunk in [12670].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.