launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/mustache-escaping into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #882081 in Launchpad itself: "Bug listing mustache template is not properly escaped"
  https://bugs.launchpad.net/launchpad/+bug/882081

For more details, see:
https://code.launchpad.net/~abentley/launchpad/mustache-escaping/+merge/80506

= Summary =
Fix bug #882081: Bug listing mustache template is not properly escaped.

== Proposed fix ==
Use JSONEncoderForHTML to render the template.

== Pre-implementation notes ==
Discussed with wgrant

== Implementation details ==
None

== Tests ==
bin/test -t TestBugListingBatchNavigator bugtask

== Demo and Q/A ==
With dynamic listings enabled, go to a listings page and view source.  The LP.mustache_listings assignment should not contain unescaped < or > signs.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/browser/bugtask.py
-- 
https://code.launchpad.net/~abentley/launchpad/mustache-escaping/+merge/80506
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/mustache-escaping into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-10-20 19:14:32 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-10-26 20:53:24 +0000
@@ -78,6 +78,7 @@
 import pystache
 from pytz import utc
 from simplejson import dumps
+from simplejson.encoder import JSONEncoderForHTML
 from z3c.pt.pagetemplate import ViewPageTemplateFile
 from zope import (
     component,
@@ -2208,7 +2209,8 @@
 
     @property
     def mustache_listings(self):
-        return 'LP.mustache_listings = %s;' % dumps(self.mustache_template)
+        return 'LP.mustache_listings = %s;' % dumps(
+            self.mustache_template, cls=JSONEncoderForHTML)
 
     @property
     def mustache(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-10-20 17:46:49 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-10-26 20:53:24 +0000
@@ -45,6 +45,7 @@
 from lp.bugs.browser.bugtask import (
     BugActivityItem,
     BugTaskEditView,
+    BugListingBatchNavigator,
     BugTaskListingItem,
     BugTasksAndNominationsView,
     BugTaskSearchListingView,
@@ -1307,6 +1308,18 @@
             browser, self.client_listing, self.server_listing, bug_number)
 
 
+class TestBugListingBatchNavigator(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_mustache_listings_escaped(self):
+        """Mustache template is encoded such that it has no unescaped tags."""
+        navigator = BugListingBatchNavigator(
+            [], LaunchpadTestRequest(), [], 0)
+        self.assertNotIn('<', navigator.mustache_listings)
+        self.assertNotIn('>', navigator.mustache_listings)
+
+
 class TestBugTaskListingItem(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer