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

Commit

Permalink
fix($browser): normalize inputted URLs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jbedard authored and Narretz committed Nov 26, 2018
1 parent 8a67ac5 commit 2f72a69
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 4 deletions.
3 changes: 3 additions & 0 deletions src/ng/browser.js
Expand Up @@ -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
Expand Down
75 changes: 71 additions & 4 deletions test/ng/browserSpecs.js
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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');
});
};
}
});
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 2f72a69

Please sign in to comment.