fix($http): add check for functions in request #16133

carlfranz · 2017-07-27T13:10:34Z

Avoid that functions in params object (like getter or setter) will be encoded in the url.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.

What is the current behavior? (You can also link to an open issue here)
Functions in class-object are encoded in url.

What is the new behavior (if this is a feature change)?
Functions in class-object are no more encoded in url

Does this PR introduce a breaking change?
no

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:
See example of bug here: https://embed.plnkr.co/tspyPWVcm6jMpVDBl6WK/ (open console)

Avoid that functions in `params` object (like getter or setter) will be encoded in the url.

googlebot · 2017-07-27T13:10:37Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

googlebot · 2017-07-27T13:10:37Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

carlfranz · 2017-07-27T13:20:52Z

I signed it!

googlebot · 2017-07-27T13:20:54Z

CLAs look good, thanks!

googlebot · 2017-07-27T13:20:55Z

CLAs look good, thanks!

gkalpak

The change looks reasonable. Thx 👍
We need:

Tests
To decide whether it is a breaking change or not.

default serializer should not serialize functions

carlfranz · 2017-07-28T18:48:48Z

shows me an error on ng-model O_o

gkalpak

Not directly related to this PR, but I realized that $httpParamSerializerJQLike treats functions differently than jQuery (jQuery calls functions and uses the return value).

gkalpak · 2017-07-29T22:44:02Z

test/ng/httpSpec.js

@@ -2345,6 +2345,10 @@ describe('$http param serializers', function() {
      expect(jqrSer({someDate: new Date('2014-07-15T17:30:00.000Z')})).toEqual('someDate=2014-07-15T17:30:00.000Z');
    });

+    it('should NOT serialize functions', function() {
+      expect(defSer({foo: 'foov', bar: function() {}})).toEqual('foo=foov');
+    });


This is not common functionality. It does not belong here, but in default array serialization section.

default serialization should not serialize functions

functions should NOT be serialized

gkalpak · 2017-07-31T08:03:30Z

@carlfranz, feel free to ping me when this is ready for another review.

carlfranz · 2017-07-31T11:00:42Z

ping? :)

Closes #16133

fix($http): add check for functions in request

23573f0

Avoid that functions in `params` object (like getter or setter) will be encoded in the url.

googlebot added the cla: no label Jul 27, 2017

googlebot added cla: yes and removed cla: no labels Jul 27, 2017

carlfranz mentioned this pull request Jul 27, 2017

fix($http): add check for functions in params #16132

Closed

3 tasks

gkalpak added component: $http frequency: low needs: breaking change needs: test severity: inconvenient type: bug labels Jul 28, 2017

gkalpak added this to the Backlog milestone Jul 28, 2017

gkalpak suggested changes Jul 28, 2017

View reviewed changes

carlfranz added 2 commits July 28, 2017 17:39

test($http): default serializer

4930476

default serializer should not serialize functions

test($http): default serializer

2d8b4f1

default serializer should not serialize functions

gkalpak suggested changes Jul 29, 2017

View reviewed changes

carlfranz added 2 commits July 30, 2017 10:24

test($http): avoid function serilization

ee57d52

default serialization should not serialize functions

test($http): default param serialization

ca6e0e0

functions should NOT be serialized

gkalpak approved these changes Jul 31, 2017

View reviewed changes

gkalpak pushed a commit that referenced this pull request Jul 31, 2017

fix($httpParamSerializer): ignore functions

b51ded6

Closes #16133

gkalpak closed this in 394dbcc Jul 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix($http): add check for functions in request #16133

fix($http): add check for functions in request #16133

carlfranz commented Jul 27, 2017

googlebot commented Jul 27, 2017

googlebot commented Jul 27, 2017

carlfranz commented Jul 27, 2017

googlebot commented Jul 27, 2017

googlebot commented Jul 27, 2017

gkalpak left a comment •

edited

Loading

carlfranz commented Jul 28, 2017

gkalpak left a comment

gkalpak Jul 29, 2017

carlfranz Jul 31, 2017

gkalpak commented Jul 31, 2017

carlfranz commented Jul 31, 2017

fix($http): add check for functions in request #16133

fix($http): add check for functions in request #16133

Conversation

carlfranz commented Jul 27, 2017

googlebot commented Jul 27, 2017

googlebot commented Jul 27, 2017

carlfranz commented Jul 27, 2017

googlebot commented Jul 27, 2017

googlebot commented Jul 27, 2017

gkalpak left a comment • edited Loading

Choose a reason for hiding this comment

carlfranz commented Jul 28, 2017

gkalpak left a comment

Choose a reason for hiding this comment

gkalpak Jul 29, 2017

Choose a reason for hiding this comment

carlfranz Jul 31, 2017

Choose a reason for hiding this comment

gkalpak commented Jul 31, 2017

carlfranz commented Jul 31, 2017

gkalpak left a comment •

edited

Loading