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

fix($location): fix infinite recursion/digest on URLs with special characters #16611

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion src/ng/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,13 @@ function $LocationProvider() {

var IGNORE_URI_REGEXP = /^\s*(javascript|mailto):/i;

// Determine if two URLs are equal despite potentially having different encoding/normalizing
// such as $location.absUrl() vs $browser.url()
// See https://github.com/angular/angular.js/issues/16592
function urlsEqual(a, b) {
return a === b || urlResolve(a).href === urlResolve(b).href;
}

function setBrowserUrlWithFallback(url, replace, state) {
var oldUrl = $location.url();
var oldState = $location.$$state;
Expand Down Expand Up @@ -996,7 +1003,7 @@ function $LocationProvider() {
var newUrl = trimEmptyHash($location.absUrl());
var oldState = $browser.state();
var currentReplace = $location.$$replace;
var urlOrStateChanged = oldUrl !== newUrl ||
var urlOrStateChanged = !urlsEqual(oldUrl, newUrl) ||
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);

if (initializing || urlOrStateChanged) {
Expand Down
109 changes: 108 additions & 1 deletion test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,58 @@ describe('$location', function() {
});


//https://github.com/angular/angular.js/issues/16592
it('should not infinitely digest when initial params contain a quote', function() {
initService({html5Mode:true,supportHistory:true});
mockUpBrowser({initialUrl:'http://localhost:9876/?q=\'', baseHref:'/'});
inject(function($location, $browser, $rootScope) {
expect(function() {
$rootScope.$digest();
}).not.toThrow();
});
});


//https://github.com/angular/angular.js/issues/16592
it('should not infinitely digest when initial params contain an escaped quote', function() {
initService({html5Mode:true,supportHistory:true});
mockUpBrowser({initialUrl:'http://localhost:9876/?q=%27', baseHref:'/'});
inject(function($location, $browser, $rootScope) {
expect(function() {
$rootScope.$digest();
}).not.toThrow();
});
});


//https://github.com/angular/angular.js/issues/16592
it('should not infinitely digest when updating params containing a quote (via $browser.url)', function() {
initService({html5Mode:true,supportHistory:true});
mockUpBrowser({initialUrl:'http://localhost:9876/', baseHref:'/'});
inject(function($location, $browser, $rootScope) {
$rootScope.$digest();
$browser.url('http://localhost:9876/?q=\'');
expect(function() {
$rootScope.$digest();
}).not.toThrow();
});
});


//https://github.com/angular/angular.js/issues/16592
it('should not infinitely digest when updating params containing a quote (via window.location + popstate)', function() {
initService({html5Mode:true,supportHistory:true});
mockUpBrowser({initialUrl:'http://localhost:9876/', baseHref:'/'});
inject(function($window, $location, $browser, $rootScope) {
$rootScope.$digest();
$window.location.href = 'http://localhost:9876/?q=\'';
expect(function() {
jqLite($window).triggerHandler('popstate');
}).not.toThrow();
});
});


describe('when changing the browser URL/history directly during a `$digest`', function() {

beforeEach(function() {
Expand Down Expand Up @@ -804,10 +856,13 @@ describe('$location', function() {
});


function updatePathOnLocationChangeSuccessTo(newPath) {
function updatePathOnLocationChangeSuccessTo(newPath, newParams) {
inject(function($rootScope, $location) {
$rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) {
$location.path(newPath);
if (newParams) {
$location.search(newParams);
}
});
});
}
Expand Down Expand Up @@ -950,6 +1005,24 @@ describe('$location', function() {
expect($browserUrl).not.toHaveBeenCalled();
});
});

//https://github.com/angular/angular.js/issues/16592
it('should not infinite $digest when going to base URL with trailing slash when $locationChangeSuccess watcher changes query params to contain quote', function() {
initService({html5Mode: true, supportHistory: true});
mockUpBrowser({initialUrl:'http://server/app/', baseHref:'/app/'});
inject(function($rootScope, $injector, $browser) {
var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').and.callThrough();

var $location = $injector.get('$location');
updatePathOnLocationChangeSuccessTo('/', {q: '\''});

$rootScope.$digest();

expect($location.path()).toEqual('/');
expect($location.search()).toEqual({q: '\''});
expect($browserUrl).toHaveBeenCalledTimes(1);
});
});
});

});
Expand Down Expand Up @@ -1140,6 +1213,40 @@ describe('$location', function() {
});
});

//https://github.com/angular/angular.js/issues/16592
it('should not infinite $digest on pushState() with quote in param', function() {
initService({html5Mode: true, supportHistory: true});
mockUpBrowser({initialUrl:'http://server/app/', baseHref:'/app/'});
inject(function($rootScope, $injector, $window) {
var $location = $injector.get('$location');
$rootScope.$digest(); //allow $location initialization to finish

$window.history.pushState({}, null, 'http://server/app/Home?q=\'');
$rootScope.$digest();

expect($location.absUrl()).toEqual('http://server/app/Home?q=\'');
expect($location.path()).toEqual('/Home');
expect($location.search()).toEqual({q: '\''});
});
});

//https://github.com/angular/angular.js/issues/16592
it('should not infinite $digest on popstate event with quote in param', function() {
initService({html5Mode: true, supportHistory: true});
mockUpBrowser({initialUrl:'http://server/app/', baseHref:'/app/'});
inject(function($rootScope, $injector, $window) {
var $location = $injector.get('$location');
$rootScope.$digest(); //allow $location initialization to finish

$window.location.href = 'http://server/app/Home?q=\'';
jqLite($window).triggerHandler('popstate');

expect($location.absUrl()).toEqual('http://server/app/Home?q=\'');
expect($location.path()).toEqual('/Home');
expect($location.search()).toEqual({q: '\''});
});
});

it('should replace browser url & state when replace() was called at least once', function() {
initService({html5Mode:true, supportHistory: true});
mockUpBrowser({initialUrl:'http://new.com/a/b/', baseHref:'/a/b/'});
Expand Down