Code review comment for lp:~sinzui/launchpad/vocab-storm-bug-413287

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is my branch to prevent oopses search or validating persons with '%'
in their names.

    lp:~sinzui/launchpad/vocab-storm-bug-413287
    Diff size: 96
    Launchpad bug: https://bugs.launchpad.net/bugs/413287
    Test command: ./bin/test -vvt "registry.*doc/vocab"
    Pre-implementation: no one
    Target release: 3.1.11

= Prevent oopses search or validating persons with '%' in their names =

The '%' is just not escaped, so psycopg's variable substitution blows up.
The ValidPersonOrTeamVocabulary uses SQL() to add hand-coded sql to a storm
query. When it does this, it uses python string substitutions. Instead, we
should use the optional second argument to SQL() to pass in the variables,
which will escape them properly.
For example:
   SQL('SELECT name FROM person WHERE id = ? AND displayname = ?', (33, 'foo'))

This should also eliminate the need to use quote() and quote_like() on the
parameters.

== Rules ==

    * Revise the query to be a Storm expression.

== QA ==

    * Try to reassign an project or team to another user.
    * Place a percent in the user's name.
    * Verify that the screen explains that there are no matching users.

    * Using the person picker
    * Search for a name with a percent in it
    * Verify you do not get an error.

== Lint ==

Linting changed files:
  lib/lp/registry/vocabularies.py
  lib/lp/registry/doc/vocabularies.txt

== Test ==

    * lib/lp/registry/doc/vocabularies.txt
      * Added a test to verify that searching for % and ? work.

== Implementation ==

    * lib/lp/registry/vocabularies.py
      * Revised the two parts of the SQL to use a storm expression

« Back to merge proposal