From c9adf165822b29a6d875fad35415e65fa6d6823f Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sat, 23 Jun 2018 20:00:30 -0700 Subject: [PATCH 1/2] fix($location): fix infinite recursion/digest on URLs with special characters Some characters are treated differently by `$location` compared to `$browser` and the native browser. When comparing URLs across these two services this must be taken into account. Fixes #16592 Closes #16611 --- src/ng/location.js | 9 +++- test/ng/locationSpec.js | 112 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index 09f08c09cdfe..aeb3a3f7ca9c 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -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; @@ -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) { diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 3dbfcd3af22c..42c07b3ea3f8 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -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() { @@ -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); + } }); }); } @@ -950,6 +1005,25 @@ 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($browser.url()).toEqual('http://server/app/?q=%27'); + expect($location.path()).toEqual('/'); + expect($location.search()).toEqual({q: '\''}); + expect($browserUrl).toHaveBeenCalledTimes(1); + }); + }); }); }); @@ -1140,6 +1214,42 @@ 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, $browser, $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($browser.url()).toEqual('http://server/app/Home?q=%27'); + 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, $browser, $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($browser.url()).toEqual('http://server/app/Home?q=%27'); + 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/'}); From 7aac363e4bd08e927fbd3f9321aeb7cbd14a827c Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 6 Jul 2018 22:17:12 -0700 Subject: [PATCH 2/2] fixup! fix($location): fix infinite recursion/digest on URLs with special characters --- test/ng/locationSpec.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 42c07b3ea3f8..5814b405b090 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -1018,7 +1018,6 @@ describe('$location', function() { $rootScope.$digest(); - expect($browser.url()).toEqual('http://server/app/?q=%27'); expect($location.path()).toEqual('/'); expect($location.search()).toEqual({q: '\''}); expect($browserUrl).toHaveBeenCalledTimes(1); @@ -1218,14 +1217,13 @@ describe('$location', function() { 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, $browser, $window) { + 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($browser.url()).toEqual('http://server/app/Home?q=%27'); expect($location.absUrl()).toEqual('http://server/app/Home?q=\''); expect($location.path()).toEqual('/Home'); expect($location.search()).toEqual({q: '\''}); @@ -1236,14 +1234,13 @@ describe('$location', function() { 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, $browser, $window) { + 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($browser.url()).toEqual('http://server/app/Home?q=%27'); expect($location.absUrl()).toEqual('http://server/app/Home?q=\''); expect($location.path()).toEqual('/Home'); expect($location.search()).toEqual({q: '\''});