From d248a079920e927b37dca8e97d433c655dc88139 Mon Sep 17 00:00:00 2001 From: Alessandro Diaferia Date: Mon, 29 Jun 2015 15:14:54 +0100 Subject: [PATCH 1/5] test(ngResource): test query url params encoding Make sure to test query url parameters encoding --- test/ngResource/resourceSpec.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index 0fa7fc52959b..9df67457bfc7 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -338,6 +338,13 @@ describe("resource", function() { }); + it('should handle url encoding in mapped params', function() { + var R = $resource('/api/myapp/resource?from=:from'); + $httpBackend.expect('GET', '/api/myapp/resource?from=bar%20%26%20blanks').respond({}); + R.get({from: 'bar & blanks'}); + }); + + it('should build resource with default param', function() { $httpBackend.expect('GET', '/Order/123/Line/456.visa?minimum=0.05').respond({id: 'abc'}); var LineItem = $resource('/Order/:orderId/Line/:id:verb', From f671ec0da8cfd04644af219f41e4a8dc3555e8c7 Mon Sep 17 00:00:00 2001 From: Alessandro Diaferia Date: Mon, 29 Jun 2015 15:15:55 +0100 Subject: [PATCH 2/5] fix(ngResource): fix query url params encoding Strictly encode query url parameters --- src/ngResource/resource.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 4c37ad48ce62..fff9ec6cf426 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -433,7 +433,7 @@ angular.module('ngResource', ['ng']). } if (!(new RegExp("^\\d+$").test(param)) && param && (new RegExp("(^|[^\\\\]):" + param + "(\\W|$)").test(url))) { - urlParams[param] = true; + urlParams[param] = { isQueryParam: (new RegExp("\\?(?:.*):" + param + "(?:\\W|$)")).test(url) }; } }); url = url.replace(/\\:/g, ':'); @@ -443,10 +443,14 @@ angular.module('ngResource', ['ng']). }); params = params || {}; - forEach(self.urlParams, function(_, urlParam) { + forEach(self.urlParams, function(paramInfo, urlParam) { val = params.hasOwnProperty(urlParam) ? params[urlParam] : self.defaults[urlParam]; if (angular.isDefined(val) && val !== null) { - encodedVal = encodeUriSegment(val); + if (paramInfo.isQueryParam === true) { + encodedVal = encodeUriQuery(val, true); + } else { + encodedVal = encodeUriSegment(val); + } url = url.replace(new RegExp(":" + urlParam + "(\\W|$)", "g"), function(match, p1) { return encodedVal + p1; }); From 175f5495ac9a9fcd140b9bf536efbb54a0dda254 Mon Sep 17 00:00:00 2001 From: Alessandro Diaferia Date: Fri, 11 Sep 2015 22:58:54 +0100 Subject: [PATCH 3/5] Update code to avoid breaking changes --- src/ngResource/resource.js | 4 ++-- test/ngResource/resourceSpec.js | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index fff9ec6cf426..b788c9d18cf7 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -433,7 +433,7 @@ angular.module('ngResource', ['ng']). } if (!(new RegExp("^\\d+$").test(param)) && param && (new RegExp("(^|[^\\\\]):" + param + "(\\W|$)").test(url))) { - urlParams[param] = { isQueryParam: (new RegExp("\\?(?:.*):" + param + "(?:\\W|$)")).test(url) }; + urlParams[param] = { isQueryParamValue: (new RegExp("\\?(?:.*)=:" + param + "(?:\\W|$)")).test(url) }; } }); url = url.replace(/\\:/g, ':'); @@ -446,7 +446,7 @@ angular.module('ngResource', ['ng']). forEach(self.urlParams, function(paramInfo, urlParam) { val = params.hasOwnProperty(urlParam) ? params[urlParam] : self.defaults[urlParam]; if (angular.isDefined(val) && val !== null) { - if (paramInfo.isQueryParam === true) { + if (paramInfo.isQueryParamiValue === true) { encodedVal = encodeUriQuery(val, true); } else { encodedVal = encodeUriSegment(val); diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index 9df67457bfc7..f94cecebfe87 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -339,9 +339,13 @@ describe("resource", function() { it('should handle url encoding in mapped params', function() { - var R = $resource('/api/myapp/resource?from=:from'); + var R1 = $resource('/api/myapp/resource?:query'); + $httpBackend.expect('GET', '/api/myapp/resource?foo&bar').respond({}); + R1.get({query: 'foo&bar'}); + + var R2 = $resource('/api/myapp/resource?from=:from'); $httpBackend.expect('GET', '/api/myapp/resource?from=bar%20%26%20blanks').respond({}); - R.get({from: 'bar & blanks'}); + R2.get({from: 'bar & blanks'}); }); From 87b8cc784d1eed9ecf8e5e2173091b20718d32a6 Mon Sep 17 00:00:00 2001 From: Alessandro Diaferia Date: Wed, 16 Sep 2015 12:19:26 +0100 Subject: [PATCH 4/5] Minor updates according to PR review suggestions --- scripts/npm/install-dependencies.sh | 2 +- src/ngResource/resource.js | 4 ++-- test/ngResource/resourceSpec.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/npm/install-dependencies.sh b/scripts/npm/install-dependencies.sh index 1851d4ea6ae4..d30b6af401be 100755 --- a/scripts/npm/install-dependencies.sh +++ b/scripts/npm/install-dependencies.sh @@ -10,7 +10,7 @@ if diff -q $SHRINKWRAP_FILE $SHRINKWRAP_CACHED_FILE; then else echo 'Blowing away node_modules and reinstalling npm dependencies...' rm -rf node_modules - npm install + npm install --registry http://registry.npmjs.org/ cp $SHRINKWRAP_FILE $SHRINKWRAP_CACHED_FILE echo 'npm install successful!' fi diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index b788c9d18cf7..58dc60f41ad4 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -433,7 +433,7 @@ angular.module('ngResource', ['ng']). } if (!(new RegExp("^\\d+$").test(param)) && param && (new RegExp("(^|[^\\\\]):" + param + "(\\W|$)").test(url))) { - urlParams[param] = { isQueryParamValue: (new RegExp("\\?(?:.*)=:" + param + "(?:\\W|$)")).test(url) }; + urlParams[param] = { isQueryParamValue: (new RegExp("\\?.*=:" + param + "(?:\\W|$)")).test(url) }; } }); url = url.replace(/\\:/g, ':'); @@ -446,7 +446,7 @@ angular.module('ngResource', ['ng']). forEach(self.urlParams, function(paramInfo, urlParam) { val = params.hasOwnProperty(urlParam) ? params[urlParam] : self.defaults[urlParam]; if (angular.isDefined(val) && val !== null) { - if (paramInfo.isQueryParamiValue === true) { + if (paramInfo.isQueryParamValue === true) { encodedVal = encodeUriQuery(val, true); } else { encodedVal = encodeUriSegment(val); diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index f94cecebfe87..b13a4e3522a1 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -338,7 +338,7 @@ describe("resource", function() { }); - it('should handle url encoding in mapped params', function() { + it('should encode & in query params unless in query param value', function() { var R1 = $resource('/api/myapp/resource?:query'); $httpBackend.expect('GET', '/api/myapp/resource?foo&bar').respond({}); R1.get({query: 'foo&bar'}); From 1ac0e7ef08b153a3b809254588b90bbf89d127e4 Mon Sep 17 00:00:00 2001 From: Alessandro Diaferia Date: Wed, 16 Sep 2015 14:07:46 +0100 Subject: [PATCH 5/5] Unify similar e2e tests Similar e2e tests assessing URL params encoding have been merged Dropped unwanted changes on install-dependencies.sh script --- scripts/npm/install-dependencies.sh | 2 +- test/ngResource/resourceSpec.js | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/scripts/npm/install-dependencies.sh b/scripts/npm/install-dependencies.sh index d30b6af401be..1851d4ea6ae4 100755 --- a/scripts/npm/install-dependencies.sh +++ b/scripts/npm/install-dependencies.sh @@ -10,7 +10,7 @@ if diff -q $SHRINKWRAP_FILE $SHRINKWRAP_CACHED_FILE; then else echo 'Blowing away node_modules and reinstalling npm dependencies...' rm -rf node_modules - npm install --registry http://registry.npmjs.org/ + npm install cp $SHRINKWRAP_FILE $SHRINKWRAP_CACHED_FILE echo 'npm install successful!' fi diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index b13a4e3522a1..d0892454e041 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -331,21 +331,18 @@ describe("resource", function() { }); - it('should encode & in url params', function() { - var R = $resource('/Path/:a'); - $httpBackend.expect('GET', '/Path/doh&foo?bar=baz%261').respond('{}'); - R.get({a: 'doh&foo', bar: 'baz&1'}); - }); - - it('should encode & in query params unless in query param value', function() { var R1 = $resource('/api/myapp/resource?:query'); - $httpBackend.expect('GET', '/api/myapp/resource?foo&bar').respond({}); + $httpBackend.expect('GET', '/api/myapp/resource?foo&bar').respond('{}'); R1.get({query: 'foo&bar'}); var R2 = $resource('/api/myapp/resource?from=:from'); - $httpBackend.expect('GET', '/api/myapp/resource?from=bar%20%26%20blanks').respond({}); + $httpBackend.expect('GET', '/api/myapp/resource?from=bar%20%26%20blanks').respond('{}'); R2.get({from: 'bar & blanks'}); + + var R3 = $resource('/Path/:a'); + $httpBackend.expect('GET', '/Path/doh&foo?bar=baz%261').respond('{}'); + R3.get({a: 'doh&foo', bar: 'baz&1'}); });