Merge lp:~stevenk/launchpad/ppa-packages-deletion-grammar into lp:launchpad

[Steve Kowalik]

[r=wgrant][bug=923604] Use better grammar and kill two XSSes in the notification when deleting packages from a PPA.

At least *try* for better grammar in the deletion notification when deleting packages from a PPA.

As an added bonus, fix two XSSes since we weren't escaping untrusted data.

[William Grant (wgrant)]

33 + notification = '\n'.join([msg.escapedtext for msg in messages])
34 + self.request.response.addNotification(structured(notification))

I'd prefer to see the structured() in the previous line, as that's where the .escapedtext which makes calling structured() safe is. Calling structured() with a variable as the first arg is going to lead to mistakes.

review: Approve (code)

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2012-01-06 14:22:17 +0000
+++ lib/lp/soyuz/browser/archive.py	2012-01-30 12:14:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for archive."""
@@ -1211,20 +1211,15 @@
         publishing_set.requestDeletion(selected_sources, self.user, comment)
 
         # Present a page notification describing the action.
-        messages = []
-        messages.append(
-            '<p>Source and binaries deleted by %s request:'
-            % self.user.displayname)
+        messages = [structured(
+            '<p>Source and binaries deleted by %s:', self.user.displayname)]
         for source in selected_sources:
-            messages.append('<br/>%s' % source.displayname)
-        messages.append('</p>')
-        # Replace the 'comment' content added by the user via structured(),
-        # so it will be quoted appropriately.
-        messages.append("<p>Deletion comment: %(comment)s</p>")
-
-        notification = "\n".join(messages)
-        self.request.response.addNotification(
-            structured(notification, comment=comment))
+            messages.append(structured('<br/>%s', source.displayname))
+        messages.append(structured(
+            '</p>\n<p>Deletion comment: %s</p>', comment))
+        notification = structured(
+            '\n'.join([msg.escapedtext for msg in messages]))
+        self.request.response.addNotification(notification)
 
         self.setNextURL()
 

=== modified file 'lib/lp/soyuz/stories/ppa/xx-delete-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-delete-packages.txt	2012-01-15 11:06:57 +0000
+++ lib/lp/soyuz/stories/ppa/xx-delete-packages.txt	2012-01-30 12:14:24 +0000
@@ -1,4 +1,5 @@
-= PPA package deletion =
+PPA package deletion
+====================
 
 The PPA 'Delete packages' view allows users to delete packages form
 their PPAs via the web UI. Only the owner of the PPA and Launchpad
@@ -111,7 +112,7 @@
     >>> messages = get_feedback_messages(admin_browser.contents)
     >>> for msg in messages:
     ...     print msg
-    Source and binaries deleted by Foo Bar request:
+    Source and binaries deleted by Foo Bar:
     iceweasel 1.0 in breezy-autotest
     Deletion comment: None
 
@@ -155,7 +156,7 @@
     >>> messages = get_feedback_messages(admin_browser.contents)
     >>> for msg in messages:
     ...     print msg
-    Source and binaries deleted by Foo Bar request:
+    Source and binaries deleted by Foo Bar:
     cdrkit 1.0 in breezy-autotest
     Deletion comment: DO &lt;where is my XSS ?&gt; IT
 
@@ -242,7 +243,7 @@
     >>> messages = get_feedback_messages(admin_browser.contents)
     >>> for msg in messages:
     ...     print msg
-    Source and binaries deleted by Foo Bar request:
+    Source and binaries deleted by Foo Bar:
     iceweasel 1.0 in warty
     pmount 0.1-1 in warty
     Deletion comment: DO IT AGAIN !
@@ -307,7 +308,8 @@
     ...
     This PPA does not contain any source packages published.
 
-== Removing source partially deleted ==
+Removing source partially deleted
+---------------------------------
 
 The 'delete-packages' interface should allow users to enforce removal
 of packages partially removed or superseded. It happens, for instance,
@@ -459,7 +461,7 @@
     >>> messages = get_feedback_messages(user_browser.contents)
     >>> for msg in messages:
     ...     print msg
-    Source and binaries deleted by No Privileges Person request:
+    Source and binaries deleted by No Privileges Person:
     foo 1.0 in hoary
     Deletion comment: Deletion of a number of base pairs that is ...
 
@@ -470,7 +472,7 @@
 downloaded from librarian.
 
 Please note also how the deletion comment is diplayed in its entirety as
-opposed to being truncated after the fisrt 20 characters.
+opposed to being truncated after the first 20 characters.
 
     >>> user_browser.open(
     ...     'http://launchpad.dev/~no-priv/+archive/ppa/+packages')
@@ -488,7 +490,8 @@
     >>> print extract_text(anon_browser.contents)
     Publishing details
     Deleted ... ago by No Privileges Person
-    Deletion of a number of base pairs that is not evenly divisible by three will lead to a frameshift mutation.
+    Deletion of a number of base pairs that is not evenly divisible by three
+    will lead to a frameshift mutation.
     Changelog
     Builds
       i386

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-person-packages.txt'
--- lib/lp/soyuz/stories/soyuz/xx-person-packages.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-person-packages.txt	2012-01-30 12:14:24 +0000
@@ -375,7 +375,7 @@
     >>> messages = get_feedback_messages(admin_browser.contents)
     >>> for msg in messages:
     ...     print msg
-    Source and binaries deleted by Foo Bar request:
+    Source and binaries deleted by Foo Bar:
     source2 666 in breezy-autotest
     Deletion comment: Bug 184490