Edgewall Software
Modify

Opened 5 years ago

Closed 5 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.

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@…>, 5 years ago

Description: modified (diff)

comment:2 by Quince <eimacdude@…>, 5 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@…>, 5 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, 5 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 5 years ago by Jun Omae (previous) (diff)

comment:5 by Jun Omae, 5 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, 5 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'.

Add Comment


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