From 4a3ae43407f7a39a88fd6d7554b9dad6c248ac5c Mon Sep 17 00:00:00 2001
From: Jason Bedard <jason+github@jbedard.ca>
Date: Sat, 23 Jun 2018 20:50:15 -0700
Subject: [PATCH] fix($browser): normalize inputted URLs

Calls to `$browser.url` now normalize the inputted URL ensuring multiple
calls only differing in formatting do not force a browser `pushState`.

Normalization is done the same as the browser location URL and may
differ per browser and may be changed by browsers. Today no browsers
fully normalize URLs so this does not fix all instances of this issue.

See #16100
Closes #16606
---
 src/ng/browser.js       |  3 ++
 test/ng/browserSpecs.js | 75 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/src/ng/browser.js b/src/ng/browser.js
index 699602bc9..4bb96ee21 100644
--- a/src/ng/browser.js
+++ b/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
diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js
index 306cda81a..46a7325c9 100644
--- a/test/ng/browserSpecs.js
+++ b/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,73 @@ describe('browser', function() {
           expect(replaceState).not.toHaveBeenCalled();
           expect(locationReplace).not.toHaveBeenCalled();
         });
+
+        it('should not do pushState with a URL using relative protocol', function() {
+          browser.url('http://server/');
+
+          pushState.calls.reset();
+          replaceState.calls.reset();
+          locationReplace.calls.reset();
+
+          browser.url('//server');
+          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');
+        });
       };
     }
   });
@@ -1093,7 +1160,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);