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

fix($browser): normalize inputted URLs #16606

Merged
merged 2 commits into from
Nov 21, 2018
Merged

fix($browser): normalize inputted URLs #16606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0903c7c
test($browser): update MockWindow to normalize URLs similar to real w…
jbedard Jun 23, 2018
fe0d4f3
fix($browser): normalize inputted URLs
jbedard Jun 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/ng/browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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
118 changes: 93 additions & 25 deletions test/ng/browserSpecs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ function MockWindow(options) {
}
var events = {};
var timeouts = this.timeouts = [];
var locationHref = 'http://server/';
var committedHref = 'http://server/';
var locationHref = window.document.createElement('a');
var committedHref = window.document.createElement('a');
locationHref.href = committedHref.href = 'http://server/';
var mockWindow = this;
var msie = options.msie;
var ieState;
Expand Down Expand Up @@ -60,28 +61,28 @@ function MockWindow(options) {

this.location = {
get href() {
return committedHref;
return committedHref.href;
},
set href(value) {
locationHref = value;
locationHref.href = value;
mockWindow.history.state = null;
historyEntriesLength++;
if (!options.updateAsync) this.flushHref();
},
get hash() {
return getHash(committedHref);
return getHash(committedHref.href);
},
set hash(value) {
locationHref = replaceHash(locationHref, value);
locationHref.href = replaceHash(locationHref.href, value);
if (!options.updateAsync) this.flushHref();
},
replace: function(url) {
locationHref = url;
locationHref.href = url;
mockWindow.history.state = null;
if (!options.updateAsync) this.flushHref();
},
flushHref: function() {
committedHref = locationHref;
committedHref.href = locationHref.href;
}
};

Expand All @@ -91,13 +92,13 @@ function MockWindow(options) {
historyEntriesLength++;
},
replaceState: function(state, title, url) {
locationHref = url;
if (!options.updateAsync) committedHref = locationHref;
locationHref.href = url;
if (!options.updateAsync) committedHref.href = locationHref.href;
mockWindow.history.state = copy(state);
if (!options.updateAsync) this.flushHref();
},
flushHref: function() {
committedHref = locationHref;
committedHref.href = locationHref.href;
}
};
// IE 10-11 deserialize history.state on each read making subsequent reads
Expand Down Expand Up @@ -398,26 +399,26 @@ describe('browser', function() {

it('should return current location.href', function() {
fakeWindow.location.href = 'http://test.com';
expect(browser.url()).toEqual('http://test.com');
expect(browser.url()).toEqual('http://test.com/');

fakeWindow.location.href = 'https://another.com';
expect(browser.url()).toEqual('https://another.com');
expect(browser.url()).toEqual('https://another.com/');
});

it('should strip an empty hash fragment', function() {
fakeWindow.location.href = 'http://test.com#';
expect(browser.url()).toEqual('http://test.com');
fakeWindow.location.href = 'http://test.com/#';
expect(browser.url()).toEqual('http://test.com/');

fakeWindow.location.href = 'https://another.com#foo';
expect(browser.url()).toEqual('https://another.com#foo');
fakeWindow.location.href = 'https://another.com/#foo';
expect(browser.url()).toEqual('https://another.com/#foo');
});

it('should use history.pushState when available', function() {
sniffer.history = true;
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 @@ -429,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 All @@ -440,7 +441,7 @@ describe('browser', function() {
sniffer.history = false;
browser.url('http://new.org');

expect(fakeWindow.location.href).toEqual('http://new.org');
expect(fakeWindow.location.href).toEqual('http://new.org/');

expect(pushState).not.toHaveBeenCalled();
expect(replaceState).not.toHaveBeenCalled();
Expand Down Expand Up @@ -473,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 @@ -507,9 +508,9 @@ describe('browser', function() {
it('should not set URL when the URL is already set', function() {
var current = fakeWindow.location.href;
sniffer.history = false;
fakeWindow.location.href = 'dontchange';
fakeWindow.location.href = 'http://dontchange/';
browser.url(current);
expect(fakeWindow.location.href).toBe('dontchange');
expect(fakeWindow.location.href).toBe('http://dontchange/');
});

it('should not read out location.href if a reload was triggered but still allow to change the url', function() {
Expand Down Expand Up @@ -688,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 @@ -812,7 +880,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();
Expand Down Expand Up @@ -1092,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