launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-739915 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

LP.cache entries need to be escaped, or HTML in strings will be parsed by the browser. That's not optimal.

If IE did not exist then we could use XHTML, where <script> is PCDATA and the escaped JS would have entities expanded. But HTML's <script> is CDATA, so we have to live with some over-escaped values in the cache. Despite how bad this sounds, it won't affect URLs, and it worked fine until this vulnerability was introduced a month ago.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-739915/+merge/54296
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-739915 into lp:launchpad.

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2011-02-28 01:09:21 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2011-03-22 05:39:31 +0000
@@ -170,13 +170,13 @@
                    '${links/?key/fmt:api_url}';">
     </script>
     <script tal:repeat="key objects"
-      tal:content="structure string:LP.cache['${key}'] =
+      tal:content="string:LP.cache['${key}'] =
                    ${objects/?key/webservice:json};">
     </script>
   </tal:cache>
 
   <script tal:condition="context/webservice:is_entry"
-    tal:content="structure string:LP.cache['context'] =
+    tal:content="string:LP.cache['context'] =
                  ${context/webservice:json};">
   </script>
 </metal:lp-client-cache>