fix($browser): normalize inputted URLs

Fixes #16100

Author: jbedard

src/ng/browser.js

@@ -108,6 +108,9 @@ function Browser(window, document, $log, $sniffer, $$taskTrackerFactory) {
     if (url) {
       var sameState = lastHistoryState === state;
 
+      // Normalize the inputted URL
+      url = urlResolve(url).href;
+
       // Don't change anything if previous and current URLs and states match. This also prevents
       // IE<10 from getting into redirect loop when in LocationHashbangInHtml5Url mode.
       // See https://github.com/angular/angular.js/commit/ffb2701

test/ng/browserSpecs.js

@@ -418,7 +418,7 @@ describe('browser', function() {
       browser.url('http://new.org');
 
       expect(pushState).toHaveBeenCalledOnce();
-      expect(pushState.calls.argsFor(0)[2]).toEqual('http://new.org');
+      expect(pushState.calls.argsFor(0)[2]).toEqual('http://new.org/');
 
       expect(replaceState).not.toHaveBeenCalled();
       expect(locationReplace).not.toHaveBeenCalled();
@@ -430,7 +430,7 @@ describe('browser', function() {
       browser.url('http://new.org', true);
 
       expect(replaceState).toHaveBeenCalledOnce();
-      expect(replaceState.calls.argsFor(0)[2]).toEqual('http://new.org');
+      expect(replaceState.calls.argsFor(0)[2]).toEqual('http://new.org/');
 
       expect(pushState).not.toHaveBeenCalled();
       expect(locationReplace).not.toHaveBeenCalled();
@@ -474,7 +474,7 @@ describe('browser', function() {
       sniffer.history = false;
       browser.url('http://new.org', true);
 
-      expect(locationReplace).toHaveBeenCalledWith('http://new.org');
+      expect(locationReplace).toHaveBeenCalledWith('http://new.org/');
 
       expect(pushState).not.toHaveBeenCalled();
       expect(replaceState).not.toHaveBeenCalled();
@@ -689,6 +689,103 @@ describe('browser', function() {
           expect(replaceState).not.toHaveBeenCalled();
           expect(locationReplace).not.toHaveBeenCalled();
         });
+
+        it('should not detect changes on $$checkUrlChange() due to input vs actual encoding', function() {
+          var callback = jasmine.createSpy('onUrlChange');
+          browser.onUrlChange(callback);
+
+          browser.url('http://server/abc?q=\'');
+          browser.$$checkUrlChange();
+          expect(callback).not.toHaveBeenCalled();
+
+          browser.url('http://server/abc?q=%27');
+          browser.$$checkUrlChange();
+          expect(callback).not.toHaveBeenCalled();
+        });
+
+        it('should not do pushState with a URL only different in encoding (less)', function() {
+          // A URL from something such as window.location.href
+          browser.url('http://server/abc?q=%27');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          // A prettier URL from something such as $location
+          browser.url('http://server/abc?q=\'');
+          expect(pushState).not.toHaveBeenCalled();
+          expect(replaceState).not.toHaveBeenCalled();
+          expect(locationReplace).not.toHaveBeenCalled();
+        });
+
+        it('should not do pushState with a URL only different in encoding (more)', function() {
+          // A prettier URL from something such as $location
+          browser.url('http://server/abc?q=\'');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          // A URL from something such as window.location.href
+          browser.url('http://server/abc?q=%27');
+          expect(pushState).not.toHaveBeenCalled();
+          expect(replaceState).not.toHaveBeenCalled();
+          expect(locationReplace).not.toHaveBeenCalled();
+        });
+
+        it('should not do pushState with a URL only adding a trailing slash after domain', function() {
+          // A domain without a trailing /
+          browser.url('http://server');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          // A domain from something such as window.location.href with a trailing slash
+          browser.url('http://server/');
+          expect(pushState).not.toHaveBeenCalled();
+          expect(replaceState).not.toHaveBeenCalled();
+          expect(locationReplace).not.toHaveBeenCalled();
+        });
+
+        it('should not do pushState with a URL only removing a trailing slash after domain', function() {
+          // A domain from something such as window.location.href with a trailing slash
+          browser.url('http://server/');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          // A domain without a trailing /
+          browser.url('http://server');
+          expect(pushState).not.toHaveBeenCalled();
+          expect(replaceState).not.toHaveBeenCalled();
+          expect(locationReplace).not.toHaveBeenCalled();
+        });
+
+        it('should do pushState with a URL only adding a trailing slash after the path', function() {
+          browser.url('http://server/foo');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          browser.url('http://server/foo/');
+          expect(pushState).toHaveBeenCalledOnce();
+          expect(fakeWindow.location.href).toEqual('http://server/foo/');
+        });
+
+        it('should do pushState with a URL only removing a trailing slash after the path', function() {
+          browser.url('http://server/foo/');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          browser.url('http://server/foo');
+          expect(pushState).toHaveBeenCalledOnce();
+          expect(fakeWindow.location.href).toEqual('http://server/foo');
+        });
       };
     }
   });
@@ -813,7 +910,7 @@ describe('browser', function() {
     it('should not fire urlChange if changed by browser.url method', function() {
       sniffer.history = false;
       browser.onUrlChange(callback);
-      browser.url('http://new.com/');
+      browser.url('http://new.com');
 
       fakeWindow.fire('hashchange');
       expect(callback).not.toHaveBeenCalled();
@@ -1093,7 +1190,7 @@ describe('browser', function() {
     it('should not interfere with legacy browser url replace behavior', function() {
       inject(function($rootScope) {
         var current = fakeWindow.location.href;
-        var newUrl = 'notyet';
+        var newUrl = 'http://notyet/';
         sniffer.history = false;
         expect(historyEntriesLength).toBe(1);
         browser.url(newUrl, true);