Merge ~cjwatson/launchpad:sqlvalues-text into launchpad:master

[Colin Watson]

Avoid quote() and sqlvalues() on arbitrary text

The `quote` and `sqlvalues` functions purported to do safe SQL quoting, but in fact they don't get it quite right: when a constructed query is executed, `psycopg2` expects to be able to do %-substitution of parameters, and so '%' characters in parameters must also be escaped. This can cause problems when substituting arbitrary user-supplied text into queries using these functions; I don't believe SQL injection is possible, but it can cause `IndexError` exceptions due to the parameter count being wrong. `ProjectGroupSet.search` handled its own escaping, but this wasn't done systematically.

A better approach is to avoid these functions where arbitrary text is involved, and instead use the style of passing parameters separately (either directly or via the Storm query compiler) so that `psycopg2` can deal with all the substitutions.

Ultimately we should remove all these old quoting functions entirely, but that's quite a bit more work.

[Thiago F. Pappacena (pappacena)]

LGTM

review: Approve

diff --git a/lib/lp/bugs/model/structuralsubscription.py b/lib/lp/bugs/model/structuralsubscription.py
index 59955d3..9d69921 100644
--- a/lib/lp/bugs/model/structuralsubscription.py
+++ b/lib/lp/bugs/model/structuralsubscription.py
@@ -19,6 +19,7 @@ import six
 from storm.base import Storm
 from storm.expr import (
     And,
+    Cast,
     Count,
     In,
     Join,
@@ -82,8 +83,8 @@ from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import quote
 from lp.services.database.stormexpr import (
+    Array,
     ArrayAgg,
     ArrayContains,
     ArrayIntersects,
@@ -927,8 +928,7 @@ def _calculate_tag_query(conditions, tags):
         # space as, effectively, NULL.  This is safe because a
         # single space is not an acceptable tag.  Again, the
         # clearest alternative is defining a custom Postgres aggregator.
-        tags_array = "ARRAY[%s,' ']::TEXT[]" % ",".join(
-            quote(tag) for tag in tags)
+        tags_array = Cast(Array(tuple(tags) + (u" ",)), "text[]")
         # Now let's build the select itself.
         second_select = Select(
             BugSubscriptionFilter.id,
@@ -944,7 +944,7 @@ def _calculate_tag_query(conditions, tags):
                 # The list of tags should be a superset of the filter tags to
                 # be included.
                 ArrayContains(
-                    SQL(tags_array),
+                    tags_array,
                     # This next line gives us an array of the tags that the
                     # filter wants to include.  Notice that it includes the
                     # empty string when the condition does not match, per the
@@ -957,7 +957,7 @@ def _calculate_tag_query(conditions, tags):
                 # tags that the filter wants to exclude.
                 Not(
                     ArrayIntersects(
-                        SQL(tags_array),
+                        tags_array,
                         # This next line gives us an array of the tags
                         # that the filter wants to exclude.  We do not bother
                         # with the empty string, and therefore allow NULLs
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 2879b37..34e19b4 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -600,10 +600,14 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
         for recipe in oci_proj1_recipes + oci_proj2_recipes:
             self.assertFalse(recipe.official)
 
+        # Cache permissions.
+        oci_project1.setOfficialRecipeStatus
+
         # Set official for project1 and make sure nothing else got changed.
         with StormStatementRecorder() as recorder:
             oci_project1.setOfficialRecipeStatus(oci_proj1_recipes[0], True)
-            self.assertEqual(1, recorder.count)
+            Store.of(oci_project1).flush()
+        self.assertEqual(1, recorder.count)
 
         self.assertTrue(oci_project2.getOfficialRecipes().is_empty())
         self.assertEqual(
@@ -615,7 +619,8 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
         # Set back no recipe as official.
         with StormStatementRecorder() as recorder:
             oci_project1.setOfficialRecipeStatus(oci_proj1_recipes[0], False)
-            self.assertEqual(0, recorder.count)
+            Store.of(oci_project1).flush()
+        self.assertEqual(1, recorder.count)
 
         for recipe in oci_proj1_recipes + oci_proj2_recipes:
             self.assertFalse(recipe.official)
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index 1000bea..26271b5 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -874,10 +874,8 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
 
     def getMilestone(self, name):
         """See `IDistribution`."""
-        return Milestone.selectOne("""
-            distribution = %s AND
-            name = %s
-            """ % sqlvalues(self.id, name))
+        return Store.of(self).find(
+            Milestone, distribution=self, name=name).one()
 
     def getOCIProject(self, name):
         oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
diff --git a/lib/lp/registry/model/distroseries.py b/lib/lp/registry/model/distroseries.py
index 3242c33..0ceff83 100644
--- a/lib/lp/registry/model/distroseries.py
+++ b/lib/lp/registry/model/distroseries.py
@@ -1180,7 +1180,7 @@ class DistroSeries(SQLBase, BugTargetBase, HasSpecificationsMixin,
         find_spec = (
             DistroSeriesPackageCache,
             BinaryPackageName,
-            SQL('ts_rank(fti, ftq(%s)) AS rank' % sqlvalues(text)))
+            SQL('ts_rank(fti, ftq(?)) AS rank', params=(text,)))
         origin = [
             DistroSeriesPackageCache,
             Join(
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 69ea961..6785698 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -3243,8 +3243,8 @@ class PersonSet:
             user_id = user.id
         cur = cursor()
         cur.execute(
-            "SELECT is_blacklisted_name(%(name)s, %(user_id)s)" % sqlvalues(
-                name=name, user_id=user_id))
+            "SELECT is_blacklisted_name(%(name)s, %(user_id)s)",
+            {"name": name, "user_id": user_id})
         return bool(cur.fetchone()[0])
 
     def getTopContributors(self, limit=50):
diff --git a/lib/lp/registry/model/pillar.py b/lib/lp/registry/model/pillar.py
index e138952..c591925 100644
--- a/lib/lp/registry/model/pillar.py
+++ b/lib/lp/registry/model/pillar.py
@@ -17,10 +17,14 @@ from sqlobject import (
     ForeignKey,
     StringCol,
     )
+from storm.databases.postgres import Case
 from storm.expr import (
     And,
+    Coalesce,
+    Desc,
     LeftJoin,
-    SQL,
+    Lower,
+    Or,
     )
 from storm.info import ClassAlias
 from storm.store import Store
@@ -51,9 +55,10 @@ from lp.services.config import config
 from lp.services.database.bulk import load_related
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
+from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormexpr import (
+    fti_search,
+    rank_by_fti,
     )
 from lp.services.librarian.model import LibraryFileAlias
 
@@ -172,20 +177,16 @@ class PillarNameSet:
             LeftJoin(
                 Distribution, PillarName.distribution == Distribution.id),
             ]
-        conditions = SQL('''
-            PillarName.active = TRUE
-            AND (PillarName.name = lower(%(text)s) OR
-
-                 Product.fti @@ ftq(%(text)s) OR
-                 lower(Product.title) = lower(%(text)s) OR
-
-                 Project.fti @@ ftq(%(text)s) OR
-                 lower(Project.title) = lower(%(text)s) OR
-
-                 Distribution.fti @@ ftq(%(text)s) OR
-                 lower(Distribution.title) = lower(%(text)s)
-                )
-            ''' % sqlvalues(text=text))
+        conditions = And(
+            PillarName.active,
+            Or(
+                PillarName.name == Lower(text),
+                fti_search(Product, text),
+                Lower(Product._title) == Lower(text),
+                fti_search(ProjectGroup, text),
+                Lower(ProjectGroup._title) == Lower(text),
+                fti_search(Distribution, text),
+                Lower(Distribution._title) == Lower(text)))
         columns = [
             PillarName, OtherPillarName, Product, ProjectGroup, Distribution]
         return IStore(PillarName).using(*origin).find(
@@ -193,14 +194,21 @@ class PillarNameSet:
             And(conditions, ProductSet.getProductPrivacyFilter(user)))
 
     def count_search_matches(self, user, text):
+        text = six.ensure_text(text)
         result = self.build_search_query(user, text)
         return result.count()
 
     def search(self, user, text, limit):
         """See `IPillarSet`."""
-        # Avoid circular import.
-        from lp.registry.model.product import get_precached_products
+        # Avoid circular imports.
+        from lp.registry.model.distribution import Distribution
+        from lp.registry.model.product import (
+            get_precached_products,
+            Product,
+            )
+        from lp.registry.model.projectgroup import ProjectGroup
 
+        text = six.ensure_text(text)
         if limit is None:
             limit = config.launchpad.default_batch_size
 
@@ -216,17 +224,20 @@ class PillarNameSet:
         # of either the Product, Project, or Distribution tables,
         # so the coalesce() is necessary to find the ts_rank() which
         # is not null.
-        result.order_by(SQL('''
-            (CASE WHEN PillarName.name = lower(%(text)s)
-                      OR lower(Product.title) = lower(%(text)s)
-                      OR lower(Project.title) = lower(%(text)s)
-                      OR lower(Distribution.title) = lower(%(text)s)
-                THEN 9999999
-                ELSE coalesce(ts_rank(Product.fti, ftq(%(text)s)),
-                              ts_rank(Project.fti, ftq(%(text)s)),
-                              ts_rank(Distribution.fti, ftq(%(text)s)))
-            END) DESC, PillarName.name
-            ''' % sqlvalues(text=text)))
+        result.order_by(
+            Desc(Case(
+                cases=(
+                    (Or(
+                        PillarName.name == Lower(text),
+                        Lower(Product._title) == Lower(text),
+                        Lower(ProjectGroup._title) == Lower(text),
+                        Lower(Distribution._title) == Lower(text)),
+                     9999999),),
+                default=Coalesce(
+                    rank_by_fti(Product, text, desc=False),
+                    rank_by_fti(ProjectGroup, text, desc=False),
+                    rank_by_fti(Distribution, text, desc=False)))),
+            PillarName.name)
         # People shouldn't be calling this method with too big limits
         longest_expected = 2 * config.launchpad.default_batch_size
         if limit > longest_expected:
diff --git a/lib/lp/registry/model/product.py b/lib/lp/registry/model/product.py
index 5ab5d1b..441e95f 100644
--- a/lib/lp/registry/model/product.py
+++ b/lib/lp/registry/model/product.py
@@ -33,8 +33,10 @@ from storm.expr import (
     Coalesce,
     Desc,
     Exists,
+    Func,
     Join,
     LeftJoin,
+    Lower,
     Not,
     Or,
     Select,
@@ -1171,8 +1173,7 @@ class Product(SQLBase, BugTargetBase, MakesAnnouncements,
 
     def getMilestone(self, name):
         """See `IProduct`."""
-        return Milestone.selectOne("""
-            product = %s AND name = %s""" % sqlvalues(self.id, name))
+        return IStore(Milestone).find(Milestone, product=self, name=name).one()
 
     def getBugSummaryContextWhereClause(self):
         """See BugTargetBase."""
@@ -1859,12 +1860,12 @@ class ProductSet:
             conditions.append(Product.active == active)
 
         if search_text is not None and search_text.strip() != '':
-            conditions.append(SQL('''
-                Product.fti @@ ftq(%(text)s) OR
-                Product.name = %(text)s OR
-                strpos(lower(Product.license_info), %(text)s) > 0 OR
-                strpos(lower(Product.reviewer_whiteboard), %(text)s) > 0
-                ''' % sqlvalues(text=search_text.lower())))
+            text = search_text.lower()
+            conditions.append(Or(
+                fti_search(Product, text),
+                Product.name == text,
+                Func("strpos", Lower(Product.license_info), text) > 0,
+                Func("strpos", Lower(Product.reviewer_whiteboard), text) > 0))
 
         def dateToDatetime(date):
             """Convert a datetime.date to a datetime.datetime
diff --git a/lib/lp/registry/model/productseries.py b/lib/lp/registry/model/productseries.py
index 3459214..82904cf 100644
--- a/lib/lp/registry/model/productseries.py
+++ b/lib/lp/registry/model/productseries.py
@@ -72,10 +72,8 @@ from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
-from lp.services.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
-    )
+from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import SQLBase
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.sorting import sorted_dotted_numbers
@@ -274,8 +272,8 @@ class ProductSeries(SQLBase, BugTargetBase, HasMilestonesMixin,
 
     def getPOTemplate(self, name):
         """See IProductSeries."""
-        return POTemplate.selectOne(
-            "productseries = %s AND name = %s" % sqlvalues(self.id, name))
+        return IStore(POTemplate).find(
+            POTemplate, productseries=self, name=name).one()
 
     @property
     def title(self):
diff --git a/lib/lp/registry/model/projectgroup.py b/lib/lp/registry/model/projectgroup.py
index cfeda26..b56499a 100644
--- a/lib/lp/registry/model/projectgroup.py
+++ b/lib/lp/registry/model/projectgroup.py
@@ -10,6 +10,7 @@ __all__ = [
     'ProjectGroupSet',
     ]
 
+import six
 from sqlobject import (
     AND,
     BoolCol,
@@ -95,10 +96,12 @@ from lp.registry.model.productseries import ProductSeries
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
 from lp.services.database.enumcol import EnumCol
+from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     SQLBase,
     sqlvalues,
     )
+from lp.services.database.stormexpr import fti_search
 from lp.services.helpers import shortlist
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp.authorization import check_permission
@@ -592,32 +595,27 @@ class ProjectGroupSet:
         should be limited to projects that are active in those Launchpad
         applications.
         """
-        if text:
-            text = text.replace("%", "%%")
-        clauseTables = set()
-        clauseTables.add('Project')
-        queries = []
+        joining_product = False
+        clauses = []
 
         if text:
+            text = six.ensure_text(text)
             if search_products:
-                clauseTables.add('Product')
-                product_query = "Product.fti @@ ftq(%s)" % sqlvalues(text)
-                queries.append(product_query)
+                joining_product = True
+                clauses.extend([
+                    Product.projectgroup == ProjectGroup.id,
+                    fti_search(Product, text),
+                    ])
             else:
-                projectgroup_query = "Project.fti @@ ftq(%s)" % sqlvalues(text)
-                queries.append(projectgroup_query)
-
-        if 'Product' in clauseTables:
-            queries.append('Product.project=Project.id')
+                clauses.append(fti_search(ProjectGroup, text))
 
         if not show_inactive:
-            queries.append('Project.active IS TRUE')
-            if 'Product' in clauseTables:
-                queries.append('Product.active IS TRUE')
+            clauses.append(ProjectGroup.active)
+            if joining_product:
+                clauses.append(Product.active)
 
-        query = " AND ".join(queries)
-        return ProjectGroup.select(
-            query, distinct=True, clauseTables=clauseTables)
+        return IStore(ProjectGroup).find(
+            ProjectGroup, *clauses).config(distinct=True)
 
 
 @implementer(IProjectGroupSeries)
diff --git a/lib/lp/registry/tests/test_pillar.py b/lib/lp/registry/tests/test_pillar.py
index 10a259e..0ff1db2 100644
--- a/lib/lp/registry/tests/test_pillar.py
+++ b/lib/lp/registry/tests/test_pillar.py
@@ -42,6 +42,18 @@ class TestPillarNameSet(TestCaseWithFactory):
                 getUtility(IPersonSet).getByName('mark'), 'lz', limit=5)]
         self.assertEqual(result_names, [u'launchzap', u'lz-bar', u'lz-foo'])
 
+    def test_search_percent(self):
+        """Searches involving '%' characters work correctly."""
+        login('[e-mail]')
+        self.factory.makeProduct(name='percent', title='contains % character')
+        self.factory.makeProduct()
+        pillar_set = getUtility(IPillarNameSet)
+        mark = getUtility(IPersonSet).getByName('mark')
+        result_names = [
+            pillar.name
+            for pillar in pillar_set.search(mark, '% character', limit=5)]
+        self.assertEqual([u'percent'], result_names)
+
 
 class TestPillarPerson(TestCaseWithFactory):
 
diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
index a38082b..02e3158 100644
--- a/lib/lp/registry/vocabularies.py
+++ b/lib/lp/registry/vocabularies.py
@@ -1517,7 +1517,7 @@ class DistributionVocabulary(NamedSQLObjectVocabulary):
 
     def getTermByToken(self, token):
         """See `IVocabularyTokenized`."""
-        obj = Distribution.selectOne("name=%s" % sqlvalues(token))
+        obj = IStore(Distribution).find(Distribution, name=token).one()
         if obj is None:
             raise LookupError(token)
         else:
@@ -1565,12 +1565,11 @@ class DistroSeriesVocabulary(NamedSQLObjectVocabulary):
         except ValueError:
             raise LookupError(token)
 
-        obj = DistroSeries.selectOne('''
-                    Distribution.id = DistroSeries.distribution AND
-                    Distribution.name = %s AND
-                    DistroSeries.name = %s
-                    ''' % sqlvalues(distroname, distroseriesname),
-                    clauseTables=['Distribution'])
+        obj = IStore(DistroSeries).find(
+            DistroSeries,
+            DistroSeries.distribution == Distribution.id,
+            Distribution.name == distroname,
+            DistroSeries.name == distroseriesname).one()
         if obj is None:
             raise LookupError(token)
         else:
diff --git a/lib/lp/services/database/sqlbase.py b/lib/lp/services/database/sqlbase.py
index 0476df1..9d53175 100644
--- a/lib/lp/services/database/sqlbase.py
+++ b/lib/lp/services/database/sqlbase.py
@@ -368,8 +368,8 @@ def quote(x):
 def sqlvalues(*values, **kwvalues):
     """Return a tuple of converted sql values for each value in some_tuple.
 
-    This safely quotes strings, or gives representations of dbschema items,
-    for example.
+    This safely quotes strings (except for '%'!), or gives representations
+    of dbschema items, for example.
 
     Use it when constructing a string for use in a SELECT.  Always use
     %s as the replacement marker.
@@ -377,6 +377,11 @@ def sqlvalues(*values, **kwvalues):
       ('SELECT foo from Foo where bar = %s and baz = %s'
        % sqlvalues(BugTaskSeverity.CRITICAL, 'foo'))
 
+    This is DEPRECATED in favour of passing parameters to SQL statements
+    using the second parameter to `cursor.execute` (normally via the Storm
+    query compiler), because it does not deal with escaping '%' characters
+    in strings.
+
     >>> sqlvalues()
     Traceback (most recent call last):
     ...
diff --git a/lib/lp/services/database/stormexpr.py b/lib/lp/services/database/stormexpr.py
index 37772f1..648bec2 100644
--- a/lib/lp/services/database/stormexpr.py
+++ b/lib/lp/services/database/stormexpr.py
@@ -329,8 +329,9 @@ def fti_search(table, text, ftq=True):
         tables=(table,))
 
 
-def rank_by_fti(table, text, ftq=True):
+def rank_by_fti(table, text, ftq=True, desc=True):
     table, query_fragment = determine_table_and_fragment(table, ftq)
     return SQL(
-        '-ts_rank(%s.fti, %s)' % (table.name, query_fragment), params=(text,),
-        tables=(table,))
+        '%sts_rank(%s.fti, %s)' % (
+            '-' if desc else '', table.name, query_fragment),
+        params=(text,), tables=(table,))
diff --git a/lib/lp/services/identity/model/emailaddress.py b/lib/lp/services/identity/model/emailaddress.py
index a866a0a..c9c9a4e 100644
--- a/lib/lp/services/identity/model/emailaddress.py
+++ b/lib/lp/services/identity/model/emailaddress.py
@@ -13,17 +13,21 @@ __all__ = [
 import hashlib
 import operator
 
+import six
 from sqlobject import (
     ForeignKey,
     StringCol,
     )
+from storm.expr import Lower
 from zope.interface import implementer
 
 from lp.app.validators.email import valid_email
 from lp.services.database.enumcol import EnumCol
-from lp.services.database.interfaces import IMasterStore
+from lp.services.database.interfaces import (
+    IMasterStore,
+    IStore,
+    )
 from lp.services.database.sqlbase import (
-    quote,
     SQLBase,
     sqlvalues,
     )
@@ -111,8 +115,10 @@ class EmailAddressSet:
 
     def getByEmail(self, email):
         """See `IEmailAddressSet`."""
-        return EmailAddress.selectOne(
-            "lower(email) = %s" % quote(email.strip().lower()))
+        return IStore(EmailAddress).find(
+            EmailAddress,
+            Lower(EmailAddress.email) ==
+                six.ensure_text(email).strip().lower()).one()
 
     def new(self, email, person=None, status=EmailAddressStatus.NEW):
         """See IEmailAddressSet."""
diff --git a/lib/lp/services/statistics/model/statistics.py b/lib/lp/services/statistics/model/statistics.py
index 75487be..e7bce2d 100644
--- a/lib/lp/services/statistics/model/statistics.py
+++ b/lib/lp/services/statistics/model/statistics.py
@@ -33,7 +33,6 @@ from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     cursor,
     SQLBase,
-    sqlvalues,
     )
 from lp.services.statistics.interfaces.statistic import (
     ILaunchpadStatistic,
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index df275c1..9696b8f 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -2593,16 +2593,16 @@ class ArchiveSet:
     def getPPAByDistributionAndOwnerName(self, distribution, person_name,
                                          ppa_name):
         """See `IArchiveSet`"""
-        query = """
-            Archive.purpose = %s AND
-            Archive.distribution = %s AND
-            Person.id = Archive.owner AND
-            Archive.name = %s AND
-            Person.name = %s
-        """ % sqlvalues(
-                ArchivePurpose.PPA, distribution, ppa_name, person_name)
+        # Circular import.
+        from lp.registry.model.person import Person
 
-        return Archive.selectOne(query, clauseTables=['Person'])
+        return IStore(Archive).find(
+            Archive,
+            Archive.purpose == ArchivePurpose.PPA,
+            Archive.distribution == distribution,
+            Archive.owner == Person.id,
+            Archive.name == ppa_name,
+            Person.name == person_name).one()
 
     def _getDefaultArchiveNameByPurpose(self, purpose):
         """Return the default for a archive in a given purpose.
@@ -2641,11 +2641,11 @@ class ArchiveSet:
 
     def getByDistroAndName(self, distribution, name):
         """See `IArchiveSet`."""
-        return Archive.selectOne("""
-            Archive.distribution = %s AND
-            Archive.name = %s AND
-            Archive.purpose != %s
-            """ % sqlvalues(distribution, name, ArchivePurpose.PPA))
+        return IStore(Archive).find(
+            Archive,
+            Archive.distribution == distribution,
+            Archive.name == name,
+            Archive.purpose != ArchivePurpose.PPA).one()
 
     def _getDefaultDisplayname(self, name, owner, distribution, purpose):
         """See `IArchive`."""
@@ -2697,9 +2697,8 @@ class ArchiveSet:
         # For non-PPA archives we enforce unique names within the context of a
         # distribution.
         if purpose != ArchivePurpose.PPA:
-            archive = Archive.selectOne(
-                "Archive.distribution = %s AND Archive.name = %s" %
-                sqlvalues(distribution, name))
+            archive = IStore(Archive).find(
+                Archive, distribution=distribution, name=name).one()
             if archive is not None:
                 raise AssertionError(
                     "archive '%s' exists already in '%s'." %
diff --git a/lib/lp/soyuz/scripts/gina/handlers.py b/lib/lp/soyuz/scripts/gina/handlers.py
index 94aac78..432efcf 100644
--- a/lib/lp/soyuz/scripts/gina/handlers.py
+++ b/lib/lp/soyuz/scripts/gina/handlers.py
@@ -29,6 +29,11 @@ from sqlobject import (
     SQLObjectMoreThanOneResultError,
     SQLObjectNotFound,
     )
+from storm.exceptions import NotOneError
+from storm.expr import (
+    Cast,
+    Desc,
+    )
 from zope.component import getUtility
 
 from lp.archivepublisher.diskpool import poolify
@@ -46,10 +51,7 @@ from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import (
-    quote,
-    sqlvalues,
-    )
+from lp.services.database.sqlbase import quote
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
 from lp.services.scripts import log
 from lp.soyuz.enums import (
@@ -561,26 +563,19 @@ class SourcePackageHandler:
 
         # Check here to see if this release has ever been published in
         # the distribution, no matter what status.
-        query = """
-                SourcePackageRelease.sourcepackagename = %s AND
-                SourcePackageRelease.version::text = %s AND
-                SourcePackagePublishingHistory.sourcepackagerelease =
-                    SourcePackageRelease.id AND
-                SourcePackagePublishingHistory.distroseries =
-                    DistroSeries.id AND
-                SourcePackagePublishingHistory.archive = %s AND
-                SourcePackagePublishingHistory.sourcepackagename = %s AND
-                DistroSeries.distribution = %s
-                """ % sqlvalues(sourcepackagename, version,
-                                distroseries.main_archive,
-                                sourcepackagename,
-                                distroseries.distribution)
-        ret = SourcePackageRelease.select(query,
-            clauseTables=['SourcePackagePublishingHistory', 'DistroSeries'],
-            orderBy=["-SourcePackagePublishingHistory.datecreated"])
-        if not ret:
-            return None
-        return ret[0]
+        SPR = SourcePackageRelease
+        SPPH = SourcePackagePublishingHistory
+        rows = IStore(SPR).find(
+            SPR,
+            SPR.sourcepackagename == sourcepackagename,
+            Cast(SPR.version, "text") == version,
+            SPPH.sourcepackagerelease == SPR.id,
+            SPPH.distroseries == DistroSeries.id,
+            SPPH.archive == distroseries.main_archive,
+            SPPH.sourcepackagename == sourcepackagename,
+            DistroSeries.distribution == distroseries.distribution)
+        return rows.order_by(
+            Desc(SourcePackagePublishingHistory.datecreated)).first()
 
     def createSourcePackageRelease(self, src, distroseries):
         """Create a SourcePackagerelease and db dependencies if needed.
@@ -749,37 +744,33 @@ class BinaryPackageHandler:
         version = binarypackagedata.version
         architecture = binarypackagedata.architecture
 
-        clauseTables = ["BinaryPackageRelease", "DistroSeries",
-                        "DistroArchSeries", "BinaryPackageBuild",
-                        "BinaryPackagePublishingHistory"]
         distroseries = distroarchseries.distroseries
 
         # When looking for binaries, we need to remember that they are
         # shared between distribution releases, so match on the
         # distribution and the architecture tag of the distroarchseries
         # they were built for
-        query = (
-            "BinaryPackagePublishingHistory.archive = %s AND "
-            "BinaryPackagePublishingHistory.binarypackagename = %s AND "
-            "BinaryPackageRelease.id ="
-            " BinaryPackagePublishingHistory.binarypackagerelease AND "
-            "BinaryPackageRelease.binarypackagename=%s AND "
-            "BinaryPackageRelease.version::text = %s AND "
-            "BinaryPackageRelease.build = BinaryPackageBuild.id AND "
-            "BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
-            "DistroArchSeries.distroseries = DistroSeries.id AND "
-            "DistroSeries.distribution = %s" %
-            sqlvalues(distroseries.main_archive, binaryname, binaryname,
-                      version, distroseries.distribution))
+        BPR = BinaryPackageRelease
+        BPPH = BinaryPackagePublishingHistory
+        BPB = BinaryPackageBuild
+        clauses = [
+            BPPH.archive == distroseries.main_archive,
+            BPPH.binarypackagename == binaryname,
+            BPPH.binarypackagerelease == BPR.id,
+            BPR.binarypackagename == binaryname,
+            Cast(BPR.version, "text") == version,
+            BPR.build == BPB.id,
+            BPB.distro_arch_series == DistroArchSeries.id,
+            DistroArchSeries.distroseries == DistroSeries.id,
+            DistroSeries.distribution == distroseries.distribution,
+            ]
 
         if architecture != "all":
-            query += ("AND DistroArchSeries.architecturetag = %s" %
-                      quote(architecture))
+            clauses.append(DistroArchSeries.architecturetag == architecture)
 
         try:
-            bpr = BinaryPackageRelease.selectOne(
-                query, clauseTables=clauseTables, distinct=True)
-        except SQLObjectMoreThanOneResultError:
+            bpr = IStore(BPR).find(BPR, *clauses).config(distinct=True).one()
+        except NotOneError:
             # XXX kiko 2005-10-27: Untested
             raise MultiplePackageReleaseError("Found more than one "
                     "entry for %s (%s) for %s in %s" %