launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/recipe-text-xss into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/recipe-text-xss/+merge/54638

See bug 740096. On the recipe index page, the Debian Version field could be rendered with non-escaped html. 
The cause was that the handing of updates was such that the raw field value from the Entry object returned by the web service patch call was being used instead of the escaped html value.

== Implementation ==

Fix the update_cache() implementation in client.js
Provide extra getHTML() methods so that a default value can be returned if required.

== Tests ==

Add new test to test_lp_client.js:
  test_update_cache_with_html
-- 
https://code.launchpad.net/~wallyworld/launchpad/recipe-text-xss/+merge/54638
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/recipe-text-xss into lp:launchpad.

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2011-02-25 01:54:00 +0000
+++ lib/lp/app/javascript/client.js	2011-03-24 02:16:19 +0000
@@ -183,7 +183,7 @@
   var fields_changed = new Array();
   for (var name in cache) {
     var old_value = cache[name];
-    var new_value = entry.get(name);
+    var new_value = entry.getHTMLOrDefault(name);
     if (name != 'lp_html') {
       if (old_value != new_value) {
         fields_changed.push(name);
@@ -448,7 +448,22 @@
     return this.get(key);
 };
 
+Entry.prototype.getHTMLOrDefault = function(key) {
+    /* Return the raw data if no HTML is found. */
+    var default_value = this.get(key);
+    if (default_value === undefined)
+        default_value = this.get(key + '_link');
+    if (default_value === undefined)
+        default_value = null;
+    return this.getHTML(key, default_value);
+};
+
 Entry.prototype.getHTML = function(key) {
+    return this.getHTML(key, null);
+};
+
+Entry.prototype.getHTML = function(key, default_value) {
+  /* Return the default_value if no HTML is found. */
   var lp_html = this.get('lp_html');
   if (lp_html) {
     // First look for the key.
@@ -462,7 +477,7 @@
       return value;
     }
   }
-  return null;
+  return default_value;
 };
 
 module.Entry = Entry;

=== modified file 'lib/lp/app/javascript/tests/test_lp_client.js'
--- lib/lp/app/javascript/tests/test_lp_client.js	2011-02-25 02:51:27 +0000
+++ lib/lp/app/javascript/tests/test_lp_client.js	2011-03-24 02:16:19 +0000
@@ -81,6 +81,25 @@
         Assert.areEqual("Unaltered", LP.cache.context.fourth);
       },
 
+    test_update_cache_with_html: function() {
+        // Make sure that the cached objects are in fact updated with the
+        // correct html representation if one exists.
+        var entry_repr = {
+          'first': "World",
+          'second': false,
+          'third': 24,
+          'fourth': "Unaltered",
+          'self_link': Y.lp.client.get_absolute_uri("a_self_link"),
+          'lp_html': {'first': "<p>World</p>"}
+        };
+        var entry = new Y.lp.client.Entry(null, entry_repr, "a_self_link");
+        Y.lp.client.update_cache(entry);
+        Assert.areEqual("<p>World</p>", LP.cache.context.first);
+        Assert.areEqual(false, LP.cache.context.second);
+        Assert.areEqual(24, LP.cache.context.third);
+        Assert.areEqual("Unaltered", LP.cache.context.fourth);
+      },
+
     test_update_cache_raises_events: function() {
         // Check that the object changed event is raised.
         var raised_event = null;