launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-715236 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #715236 in Launchpad itself: "Validate architecturetag"
  https://bugs.launchpad.net/launchpad/+bug/715236

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-715236/+merge/53576

DistroArchSeries.architecturetag is used in URLs and passed into external tools, for both of which some characters (eg. spaces) are dangerous. Yet it is an unconstrained line of text. This branch applies valid_name to it at the DB and form level.

valid_name probably isn't quite correct, but it fits all the valid archtags (try 'dpkg-architecture -L'), and Debian Policy does not specify a better constraint.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-715236/+merge/53576
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-715236 into lp:launchpad/db-devel.

=== added file 'database/schema/patch-2208-99-0.sql'
--- database/schema/patch-2208-99-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-0.sql	2011-03-16 08:06:07 +0000
@@ -0,0 +1,6 @@
+SET client_min_messages=ERROR;
+
+ALTER TABLE distroarchseries
+    ADD CONSTRAINT valid_architecturetag CHECK (valid_name(architecturetag));
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);

=== modified file 'lib/lp/soyuz/interfaces/distroarchseries.py'
--- lib/lp/soyuz/interfaces/distroarchseries.py	2010-10-06 11:46:51 +0000
+++ lib/lp/soyuz/interfaces/distroarchseries.py	2011-03-16 08:06:07 +0000
@@ -30,6 +30,7 @@
     )
 
 from canonical.launchpad import _
+from lp.app.validators.name import name_validator
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.role import IHasOwner
@@ -56,7 +57,8 @@
                 "identifies this architecture. All binary packages in the "
                 "archive will use this tag in their filename. Please get it "
                 "correct. It should really never be changed!"),
-            required=True),
+            required=True,
+            constraint=name_validator),
         exported_as="architecture_tag")
     official = exported(
         Bool(

=== modified file 'lib/lp/soyuz/stories/distroseries/add-architecture.txt'
--- lib/lp/soyuz/stories/distroseries/add-architecture.txt	2009-11-09 17:08:21 +0000
+++ lib/lp/soyuz/stories/distroseries/add-architecture.txt	2011-03-16 08:06:07 +0000
@@ -27,6 +27,16 @@
     >>> print admin_browser.title
     ia64 : Hoary (5.04) : Ubuntu
 
+Architecture tag is restricted to the usual Launchpad name format.
+
+    >>> admin_browser.open('http://launchpad.dev/ubuntu/hoary')
+    >>> admin_browser.getLink('Add architecture').click()
+    >>> admin_browser.getControl('Architecture Tag').value = 'foo bar'
+    >>> admin_browser.getControl('Continue').click()
+    >>> print "\n".join(get_feedback_messages(admin_browser.contents))
+    There is 1 error.
+    Invalid name 'foo bar'. ...
+
 Other users won't see the link nor the page where a new port can be
 registered.