Add query param whitelisting for navigation (bug 810308)

The back button will now take you back to the search that you were on
and properly close navigation loops with pages that have similar but
not identical sets of query parameters.

media/js/impala/search.js

@@ -9,7 +9,7 @@ function autofillPlatform(context) {
         }
 
         // Populate search form with browser version and OS.
-        var gv = z.getVars(location.search),
+        var gv = z.getVars(),
             appver = '',
             platform = '',
             appver_defined = typeof gv.appver !== 'undefined',
@@ -73,7 +73,7 @@ $(function() {
 
 function rebuildLink(url, urlparams, qs) {
     var params = JSON.parseNonNull(urlparams),
-        newVars = $.extend(z.getVars(qs), params);
+        newVars = $.extend(z.getVars(qs, true), params);
     return url.split('?')[0] + '?' + $.param(newVars);
 }
 

media/js/impala/serializers.js

@@ -1,19 +1,18 @@
-z.getVars = function(qs) {
+z.getVars = function(qs, excl_undefined) {
     if (typeof qs === 'undefined') {
         qs = location.search;
     }
-    var vars = {};
-    if (qs.length > 1) {
-        var items = qs.substr(1).split('&'),
-            item;
-        for (var i = 0; i < items.length; i++) {
-            item = items[i].split('=');
-            if (item[0] !== '' && typeof item[1] !== 'undefined') {
-                vars[escape_(unescape(item[0]))] = escape_(unescape(item[1]));
-            }
-        }
+    if (qs[0] == '?') {
+        qs = qs.substr(1);  // Filter off the leading ? if it's there.
     }
-    return vars;
+
+    var pairs = _.chain(qs.split('&'))  // ['a=b', 'c=d']
+                 .map(function(c) {return _.map(c.split('='), escape_);}); //  [['a', 'b'], ['c', 'd']]
+    if (excl_undefined) {
+        // [['a', 'b'], ['c', undefined]] -> [['a', 'b']]
+        pairs = pairs.filter(function(p) {return !_.isUndefined(p[1]);})
+    }
+    return pairs.object().value();  // {'a': 'b', 'c': 'd'}
 };
 
 

media/js/mkt/login.js

@@ -60,7 +60,7 @@ function gotVerifiedEmail(assertion) {
 }
 
 function finishLogin() {
-    var to = z.getVars(window.location.search).to;
+    var to = z.getVars().to;
     $.Deferred().resolve();
     if (to && to[0] == '/') {
         // Browsers may helpfully add "http:" to URIs that begin with double

media/js/mkt/navigation.js

@@ -6,11 +6,44 @@ var nav = (function() {
             type: 'root'
         }
     ];
-    // Ask potch.
-    var path_a = document.createElement('a');
+    var param_whitelist = ['q', 'sort', 'cat'];
+
+    function extract_nav_url(url) {
+        // This function returns the URL that we should use for navigation.
+        // It filters and orders the parameters to make sure that they pass
+        // equality tests down the road.
+
+        // If there's no URL params, return the original URL.
+        if (url.indexOf('?') < 0) {
+            return url;
+        }
+
+        var url_parts = url.split('?');
+        // If there's nothing after the `?`, return the original URL.
+        if (!url_parts[1]) {
+            return url;
+        }
+
+        var used_params = _.pick(z.getVars(url_parts[1]), param_whitelist);
+        var param_pairs = _.sortBy(_.pairs(used_params), function(x) {return x[0];});
+        return url_parts[0] + '?' + _.map(
+            param_pairs,
+            function(pair) {
+                if (typeof pair[1] === 'undefined')
+                    return encodeURIComponent(pair[1]);
+                else
+                    return encodeURIComponent(pair[0]) + '=' +
+                           encodeURIComponent(pair[1]);
+            }
+        ).join('&');
+    }
 
     z.page.on('fragmentloaded', function(event, href, popped, state) {
 
+        // Clean the path's parameters.
+        // /foo/bar?foo=bar&q=blah -> /foo/bar?q=blah
+        state.path = extract_nav_url(state.path);
+
         // Truncate any closed navigational loops.
         for (var i=0; i<stack.length; i++) {
             if (stack[i].path === state.path) {
@@ -18,10 +51,6 @@ var nav = (function() {
                 break;
             }
         }
-        // <ask potch>
-        path_a.href = state.path;
-        state.path = path_a.pathname;
-        // </ask>
 
         // Are we home? clear any history.
         if (state.type == 'root') {

media/js/zamboni/tests/serializers_tests.js

@@ -2,22 +2,24 @@ module('Serializers');
 
 
 test('getVars', function() {
-    function check(s, expected) {
-        tests.equalObjects(z.getVars(s), expected);
+    function check(s, excl, expected) {
+        tests.equalObjects(z.getVars(s, excl), expected);
     }
     check('', {});
     check('?', {});
-    check('?a', {});
-    check('?==a', {});
-    check('?a=apple&a=apricot', {'a': 'apricot'});
-    check('?a=apple&b=banana&c=carrot',
+    check('?a', true, {});
+    check('?a', false, {'a': undefined});
+    check('?==a', true, {});
+    check('?==a', false, {});
+    check('?a=apple&a=apricot', false, {'a': 'apricot'});
+    check('?a=apple&b=banana&c=carrot', false,
           {'a': 'apple', 'b': 'banana', 'c': 'carrot'});
-    check('?a?a=apple', {'a?a': 'apple'});
-    check('?a=apple&b?c=banana', {'a': 'apple', 'b?c': 'banana'});
-    check('?a=b=c&d=e', {'a': 'b', 'd': 'e'});
-    check('?<script>alert("xss")</script>="a"',
+    check('?a?a=apple', false, {'a?a': 'apple'});
+    check('?a=apple&b?c=banana', false, {'a': 'apple', 'b?c': 'banana'});
+    check('?a=b=c&d=e', false, {'a': 'b', 'd': 'e'});
+    check('?<script>alert("xss")</script>="a"', false,
           {'&lt;script&gt;alert(&#34;xss&#34;)&lt;/script&gt;': '&#34;a&#34;'});
-    check('?"a"=<script>alert("xss")</script>',
+    check('?"a"=<script>alert("xss")</script>', false,
           {'&#34;a&#34;': '&lt;script&gt;alert(&#34;xss&#34;)&lt;/script&gt;'});
 });