Skip to content

Commit

Permalink
fix($browser): normalize inputted URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
jbedard committed Oct 3, 2018
1 parent 5dad293 commit 768105f
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 5 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
107 changes: 102 additions & 5 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,103 @@ describe('browser', function() {
expect(replaceState).not.toHaveBeenCalled();
expect(locationReplace).not.toHaveBeenCalled();
});

it('should not detect changes on $$checkUrlChange() due to input vs actual encoding', function() {
var callback = jasmine.createSpy('onUrlChange');
browser.onUrlChange(callback);

browser.url('http://server/abc?q=\'');
browser.$$checkUrlChange();
expect(callback).not.toHaveBeenCalled();

browser.url('http://server/abc?q=%27');
browser.$$checkUrlChange();
expect(callback).not.toHaveBeenCalled();
});

it('should not do pushState with a URL only different in encoding (less)', function() {
// A URL from something such as window.location.href
browser.url('http://server/abc?q=%27');

pushState.calls.reset();
replaceState.calls.reset();
locationReplace.calls.reset();

// A prettier URL from something such as $location
browser.url('http://server/abc?q=\'');
expect(pushState).not.toHaveBeenCalled();
expect(replaceState).not.toHaveBeenCalled();
expect(locationReplace).not.toHaveBeenCalled();
});

it('should not do pushState with a URL only different in encoding (more)', function() {
// A prettier URL from something such as $location
browser.url('http://server/abc?q=\'');

pushState.calls.reset();
replaceState.calls.reset();
locationReplace.calls.reset();

// A URL from something such as window.location.href
browser.url('http://server/abc?q=%27');
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 @@ -813,7 +910,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 @@ -1093,7 +1190,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 768105f

Please sign in to comment.