Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit fe0d4f3

Browse files
committed
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
1 parent 0903c7c commit fe0d4f3

File tree

2 files changed

+74
-4
lines changed

2 files changed

+74
-4
lines changed

src/ng/browser.js

+3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ function Browser(window, document, $log, $sniffer, $$taskTrackerFactory) {
108108
if (url) {
109109
var sameState = lastHistoryState === state;
110110

111+
// Normalize the inputted URL
112+
url = urlResolve(url).href;
113+
111114
// Don't change anything if previous and current URLs and states match. This also prevents
112115
// IE<10 from getting into redirect loop when in LocationHashbangInHtml5Url mode.
113116
// See https://github.com/angular/angular.js/commit/ffb2701

test/ng/browserSpecs.js

+71-4
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ describe('browser', function() {
418418
browser.url('http://new.org');
419419

420420
expect(pushState).toHaveBeenCalledOnce();
421-
expect(pushState.calls.argsFor(0)[2]).toEqual('http://new.org');
421+
expect(pushState.calls.argsFor(0)[2]).toEqual('http://new.org/');
422422

423423
expect(replaceState).not.toHaveBeenCalled();
424424
expect(locationReplace).not.toHaveBeenCalled();
@@ -430,7 +430,7 @@ describe('browser', function() {
430430
browser.url('http://new.org', true);
431431

432432
expect(replaceState).toHaveBeenCalledOnce();
433-
expect(replaceState.calls.argsFor(0)[2]).toEqual('http://new.org');
433+
expect(replaceState.calls.argsFor(0)[2]).toEqual('http://new.org/');
434434

435435
expect(pushState).not.toHaveBeenCalled();
436436
expect(locationReplace).not.toHaveBeenCalled();
@@ -474,7 +474,7 @@ describe('browser', function() {
474474
sniffer.history = false;
475475
browser.url('http://new.org', true);
476476

477-
expect(locationReplace).toHaveBeenCalledWith('http://new.org');
477+
expect(locationReplace).toHaveBeenCalledWith('http://new.org/');
478478

479479
expect(pushState).not.toHaveBeenCalled();
480480
expect(replaceState).not.toHaveBeenCalled();
@@ -689,6 +689,73 @@ describe('browser', function() {
689689
expect(replaceState).not.toHaveBeenCalled();
690690
expect(locationReplace).not.toHaveBeenCalled();
691691
});
692+
693+
it('should not do pushState with a URL using relative protocol', function() {
694+
browser.url('http://server/');
695+
696+
pushState.calls.reset();
697+
replaceState.calls.reset();
698+
locationReplace.calls.reset();
699+
700+
browser.url('//server');
701+
expect(pushState).not.toHaveBeenCalled();
702+
expect(replaceState).not.toHaveBeenCalled();
703+
expect(locationReplace).not.toHaveBeenCalled();
704+
});
705+
706+
it('should not do pushState with a URL only adding a trailing slash after domain', function() {
707+
// A domain without a trailing /
708+
browser.url('http://server');
709+
710+
pushState.calls.reset();
711+
replaceState.calls.reset();
712+
locationReplace.calls.reset();
713+
714+
// A domain from something such as window.location.href with a trailing slash
715+
browser.url('http://server/');
716+
expect(pushState).not.toHaveBeenCalled();
717+
expect(replaceState).not.toHaveBeenCalled();
718+
expect(locationReplace).not.toHaveBeenCalled();
719+
});
720+
721+
it('should not do pushState with a URL only removing a trailing slash after domain', function() {
722+
// A domain from something such as window.location.href with a trailing slash
723+
browser.url('http://server/');
724+
725+
pushState.calls.reset();
726+
replaceState.calls.reset();
727+
locationReplace.calls.reset();
728+
729+
// A domain without a trailing /
730+
browser.url('http://server');
731+
expect(pushState).not.toHaveBeenCalled();
732+
expect(replaceState).not.toHaveBeenCalled();
733+
expect(locationReplace).not.toHaveBeenCalled();
734+
});
735+
736+
it('should do pushState with a URL only adding a trailing slash after the path', function() {
737+
browser.url('http://server/foo');
738+
739+
pushState.calls.reset();
740+
replaceState.calls.reset();
741+
locationReplace.calls.reset();
742+
743+
browser.url('http://server/foo/');
744+
expect(pushState).toHaveBeenCalledOnce();
745+
expect(fakeWindow.location.href).toEqual('http://server/foo/');
746+
});
747+
748+
it('should do pushState with a URL only removing a trailing slash after the path', function() {
749+
browser.url('http://server/foo/');
750+
751+
pushState.calls.reset();
752+
replaceState.calls.reset();
753+
locationReplace.calls.reset();
754+
755+
browser.url('http://server/foo');
756+
expect(pushState).toHaveBeenCalledOnce();
757+
expect(fakeWindow.location.href).toEqual('http://server/foo');
758+
});
692759
};
693760
}
694761
});
@@ -1093,7 +1160,7 @@ describe('browser', function() {
10931160
it('should not interfere with legacy browser url replace behavior', function() {
10941161
inject(function($rootScope) {
10951162
var current = fakeWindow.location.href;
1096-
var newUrl = 'notyet';
1163+
var newUrl = 'http://notyet/';
10971164
sniffer.history = false;
10981165
expect(historyEntriesLength).toBe(1);
10991166
browser.url(newUrl, true);

0 commit comments

Comments
 (0)