fix($browser): normalize inputted URLs

See angular#16100
jbedard · Oct 29, 2018 · e9e18f5 · e9e18f5
1 parent 33492fe
commit e9e18f5
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 4 deletions.
diff --git 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
@@ -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);