fix($route): correctly extract path params if path contains question mark or hash

src/ngMock/angular-mocks.js

@@ -1771,8 +1771,8 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
    * See {@link ngMock.$httpBackend#when `when`} for more info.
    */
   $httpBackend.whenRoute = function(method, url) {
-    var pathObj = routeToRegExp(url, {caseInsensitiveMatch: true, ignoreTrailingSlashes: true});
-    return $httpBackend.when(method, pathObj.regexp, undefined, undefined, pathObj.keys);
+    var parsed = parseRouteUrl(url);
+    return $httpBackend.when(method, parsed.regexp, undefined, undefined, parsed.keys);
   };
 
   /**
@@ -1955,8 +1955,8 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
    * See {@link ngMock.$httpBackend#expect `expect`} for more info.
    */
   $httpBackend.expectRoute = function(method, url) {
-    var pathObj = routeToRegExp(url, {caseInsensitiveMatch: true, ignoreTrailingSlashes: true});
-    return $httpBackend.expect(method, pathObj.regexp, undefined, undefined, pathObj.keys);
+    var parsed = parseRouteUrl(url);
+    return $httpBackend.expect(method, parsed.regexp, undefined, undefined, parsed.keys);
   };
 
 
@@ -2084,6 +2084,12 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
       };
     });
   }
+
+  function parseRouteUrl(url) {
+    var strippedUrl = stripQueryAndHash(url);
+    var parseOptions = {caseInsensitiveMatch: true, ignoreTrailingSlashes: true};
+    return routeToRegExp(strippedUrl, parseOptions);
+  }
 }
 
 function assertArgDefined(args, index, name) {
@@ -2092,110 +2098,124 @@ function assertArgDefined(args, index, name) {
   }
 }
 
+function stripQueryAndHash(url) {
+  return url.replace(/[?#].*$/, '');
+}
 
-function MockHttpExpectation(method, url, data, headers, keys) {
-
-  function getUrlParams(u) {
-    var params = u.slice(u.indexOf('?') + 1).split('&');
-    return params.sort();
-  }
-
-  function compareUrl(u) {
-    return (url.slice(0, url.indexOf('?')) === u.slice(0, u.indexOf('?')) &&
-      getUrlParams(url).join() === getUrlParams(u).join());
-  }
+function MockHttpExpectation(expectedMethod, expectedUrl, expectedData, expectedHeaders,
+                             expectedKeys) {
 
-  this.data = data;
-  this.headers = headers;
+  this.data = expectedData;
+  this.headers = expectedHeaders;
 
-  this.match = function(m, u, d, h) {
-    if (method !== m) return false;
-    if (!this.matchUrl(u)) return false;
-    if (angular.isDefined(d) && !this.matchData(d)) return false;
-    if (angular.isDefined(h) && !this.matchHeaders(h)) return false;
+  this.match = function(method, url, data, headers) {
+    if (expectedMethod !== method) return false;
+    if (!this.matchUrl(url)) return false;
+    if (angular.isDefined(data) && !this.matchData(data)) return false;
+    if (angular.isDefined(headers) && !this.matchHeaders(headers)) return false;
     return true;
   };
 
-  this.matchUrl = function(u) {
-    if (!url) return true;
-    if (angular.isFunction(url.test)) return url.test(u);
-    if (angular.isFunction(url)) return url(u);
-    return (url === u || compareUrl(u));
+  this.matchUrl = function(url) {
+    if (!expectedUrl) return true;
+    if (angular.isFunction(expectedUrl.test)) return expectedUrl.test(url);
+    if (angular.isFunction(expectedUrl)) return expectedUrl(url);
+    return (expectedUrl === url || compareUrlWithQuery(url));
   };
 
-  this.matchHeaders = function(h) {
-    if (angular.isUndefined(headers)) return true;
-    if (angular.isFunction(headers)) return headers(h);
-    return angular.equals(headers, h);
+  this.matchHeaders = function(headers) {
+    if (angular.isUndefined(expectedHeaders)) return true;
+    if (angular.isFunction(expectedHeaders)) return expectedHeaders(headers);
+    return angular.equals(expectedHeaders, headers);
   };
 
-  this.matchData = function(d) {
-    if (angular.isUndefined(data)) return true;
-    if (data && angular.isFunction(data.test)) return data.test(d);
-    if (data && angular.isFunction(data)) return data(d);
-    if (data && !angular.isString(data)) {
-      return angular.equals(angular.fromJson(angular.toJson(data)), angular.fromJson(d));
+  this.matchData = function(data) {
+    if (angular.isUndefined(expectedData)) return true;
+    if (expectedData && angular.isFunction(expectedData.test)) return expectedData.test(data);
+    if (expectedData && angular.isFunction(expectedData)) return expectedData(data);
+    if (expectedData && !angular.isString(expectedData)) {
+      return angular.equals(angular.fromJson(angular.toJson(expectedData)), angular.fromJson(data));
     }
     // eslint-disable-next-line eqeqeq
-    return data == d;
+    return expectedData == data;
   };
 
   this.toString = function() {
-    return method + ' ' + url;
+    return expectedMethod + ' ' + expectedUrl;
   };
 
-  this.params = function(u) {
-    return angular.extend(parseQuery(), pathParams());
+  this.params = function(url) {
+    var queryStr = url.indexOf('?') === -1 ? '' : url.substring(url.indexOf('?') + 1);
+    var strippedUrl = stripQueryAndHash(url);
 
-    function pathParams() {
-      var keyObj = {};
-      if (!url || !angular.isFunction(url.test) || !keys || keys.length === 0) return keyObj;
+    return angular.extend(extractParamsFromQuery(queryStr), extractParamsFromPath(strippedUrl));
+  };
 
-      var m = url.exec(u);
-      if (!m) return keyObj;
-      for (var i = 1, len = m.length; i < len; ++i) {
-        var key = keys[i - 1];
-        var val = m[i];
-        if (key && val) {
-          keyObj[key.name || key] = val;
-        }
-      }
+  function compareUrlWithQuery(url) {
+    var urlWithQueryRe = /^([^?]*)\?(.*)$/;
+
+    var expectedMatch = urlWithQueryRe.exec(expectedUrl);
+    var actualMatch = urlWithQueryRe.exec(url);
+
+    return !!(expectedMatch && actualMatch) &&
+      (expectedMatch[1] === actualMatch[1]) &&
+      (normalizeQuery(expectedMatch[2]) === normalizeQuery(actualMatch[2]));
+  }
+
+  function normalizeQuery(queryStr) {
+    return queryStr.split('&').sort().join('&');
+  }
+
+  function extractParamsFromPath(strippedUrl) {
+    var keyObj = {};
+
+    if (!expectedUrl || !angular.isFunction(expectedUrl.test) ||
+        !expectedKeys || !expectedKeys.length) return keyObj;
 
-      return keyObj;
+    var match = expectedUrl.exec(strippedUrl);
+    if (!match) return keyObj;
+
+    for (var i = 1, len = match.length; i < len; ++i) {
+      var key = expectedKeys[i - 1];
+      var val = match[i];
+      if (key && val) {
+        keyObj[key.name || key] = val;
+      }
     }
 
-    function parseQuery() {
-      var obj = {}, key_value, key,
-          queryStr = u.indexOf('?') > -1
-          ? u.substring(u.indexOf('?') + 1)
-          : '';
-
-      angular.forEach(queryStr.split('&'), function(keyValue) {
-        if (keyValue) {
-          key_value = keyValue.replace(/\+/g,'%20').split('=');
-          key = tryDecodeURIComponent(key_value[0]);
-          if (angular.isDefined(key)) {
-            var val = angular.isDefined(key_value[1]) ? tryDecodeURIComponent(key_value[1]) : true;
-            if (!hasOwnProperty.call(obj, key)) {
-              obj[key] = val;
-            } else if (angular.isArray(obj[key])) {
-              obj[key].push(val);
-            } else {
-              obj[key] = [obj[key],val];
-            }
-          }
+    return keyObj;
+  }
+
+  function extractParamsFromQuery(queryStr) {
+    var obj = {},
+        keyValuePairs = queryStr.split('&').
+            filter(angular.identity).  // Ignore empty segments.
+            map(function(keyValue) { return keyValue.replace(/\+/g, '%20').split('='); });
+
+    angular.forEach(keyValuePairs, function(pair) {
+      var key = tryDecodeURIComponent(pair[0]);
+      if (angular.isDefined(key)) {
+        var val = angular.isDefined(pair[1]) ? tryDecodeURIComponent(pair[1]) : true;
+        if (!hasOwnProperty.call(obj, key)) {
+          obj[key] = val;
+        } else if (angular.isArray(obj[key])) {
+          obj[key].push(val);
+        } else {
+          obj[key] = [obj[key], val];
         }
-      });
-      return obj;
-    }
-    function tryDecodeURIComponent(value) {
-      try {
-        return decodeURIComponent(value);
-      } catch (e) {
-        // Ignore any invalid uri component
       }
+    });
+
+    return obj;
+  }
+
+  function tryDecodeURIComponent(value) {
+    try {
+      return decodeURIComponent(value);
+    } catch (e) {
+      // Ignore any invalid uri component
     }
-  };
+  }
 }
 
 function createMockXhr() {

src/ngRoute/route.js

@@ -225,6 +225,7 @@ function $RouteProvider() {
     }
     routes[path] = angular.extend(
       routeCopy,
+      {originalPath: path},
       path && routeToRegExp(path, routeCopy)
     );
 
@@ -235,7 +236,7 @@ function $RouteProvider() {
             : path + '/';
 
       routes[redirectPath] = angular.extend(
-        {redirectTo: path},
+        {originalPath: path, redirectTo: path},
         routeToRegExp(redirectPath, routeCopy)
       );
     }

src/routeToRegExp.js

@@ -3,15 +3,16 @@
 /* global routeToRegExp: true */
 
 /**
- * @param path {string} path
- * @param opts {Object} options
- * @return {?Object}
+ * @param {string} path - The path to parse. (It is assumed to have query and hash stripped off.)
+ * @param {Object} opts - Options.
+ * @return {Object} - An object containing an array of path parameter names (`keys`) and a regular
+ *     expression (`regexp`) that can be used to identify a matching URL and extract the path
+ *     parameter values.
  *
  * @description
- * Normalizes the given path, returning a regular expression
- * and the original path.
+ * Parses the given path, extracting path parameter names and a regular expression to match URLs.
  *
- * Inspired by pathRexp in visionmedia/express/lib/utils.js.
+ * Originally inspired by `pathRexp` in `visionmedia/express/lib/utils.js`.
  */
 function routeToRegExp(path, opts) {
   var keys = [];
@@ -21,11 +22,11 @@ function routeToRegExp(path, opts) {
     .replace(/(\/)?:(\w+)(\*\?|[?*])?/g, function(_, slash, key, option) {
       var optional = option === '?' || option === '*?';
       var star = option === '*' || option === '*?';
-      keys.push({ name: key, optional: optional });
+      keys.push({name: key, optional: optional});
       slash = slash || '';
       return (
         (optional ? '(?:' + slash : slash + '(?:') +
-        (star ? '([^?#]+?)' : '([^/?#]+)') +
+        (star ? '(.+?)' : '([^/]+)') +
         (optional ? '?)?' : ')')
       );
     })
@@ -36,7 +37,6 @@ function routeToRegExp(path, opts) {
   }
 
   return {
-    originalPath: path,
     keys: keys,
     regexp: new RegExp(
       '^' + pattern + '(?:[?#]|$)',

test/ngMock/angular-mocksSpec.js

@@ -2251,7 +2251,7 @@ describe('ngMock', function() {
           }
         );
         they('should ignore query params when matching in ' + routeShortcut + ' $prop method', methods,
-          function() {
+          function(method) {
             angular.forEach([
               {route: '/route1/:id', url: '/route1/Alpha', expectedParams: {id: 'Alpha'}},
               {route: '/route2/:id', url: '/route2/Bravo/?', expectedParams: {id: 'Bravo'}},
@@ -2268,14 +2268,14 @@ describe('ngMock', function() {
             ], function(testDataEntry) {
               callback.calls.reset();
               var paramsSpy = jasmine.createSpy('params');
-              hb[routeShortcut](this, testDataEntry.route).respond(
+              hb[routeShortcut](method, testDataEntry.route).respond(
                 function(method, url, data, headers, params) {
                   paramsSpy(params);
                   // status, response, headers, statusText, xhrStatus
                   return [200, 'path', { 'x-header': 'foo' }, 'OK', 'complete'];
                 }
               );
-              hb(this, testDataEntry.url, undefined, callback);
+              hb(method, testDataEntry.url, undefined, callback);
               hb.flush();
               expect(callback).toHaveBeenCalledOnceWith(200, 'path', 'x-header: foo', 'OK', 'complete');
               expect(paramsSpy).toHaveBeenCalledOnceWith(testDataEntry.expectedParams);

test/ngRoute/routeParamsSpec.js

@@ -77,5 +77,45 @@ describe('$routeParams', function() {
     });
   });
 
+  it('should correctly extract path params containing hashes and/or question marks', function() {
+    module(function($routeProvider) {
+      $routeProvider.when('/foo/:bar', {});
+      $routeProvider.when('/zoo/:bar/:baz/:qux', {});
+    });
+
+    inject(function($location, $rootScope, $routeParams) {
+      $location.path('/foo/bar?baz');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar?baz'});
+
+      $location.path('/foo/bar?baz=val');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar?baz=val'});
+
+      $location.path('/foo/bar#baz');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar#baz'});
+
+      $location.path('/foo/bar?baz#qux');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar?baz#qux'});
+
+      $location.path('/foo/bar?baz=val#qux');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar?baz=val#qux'});
+
+      $location.path('/foo/bar#baz?qux');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({bar: 'bar#baz?qux'});
+
+      $location.path('/zoo/bar?p1=v1#h1/baz?p2=v2#h2/qux?p3=v3#h3');
+      $rootScope.$digest();
+      expect($routeParams).toEqual({
+        bar: 'bar?p1=v1#h1',
+        baz: 'baz?p2=v2#h2',
+        qux: 'qux?p3=v3#h3'
+      });
+    });
+  });
 
 });