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

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

Closed
wants to merge 4 commits into from
Closed

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

wants to merge 4 commits into from

Conversation

carlfranz
Copy link
Contributor

@carlfranz carlfranz commented Jul 27, 2017

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

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
Copy link

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.

1 similar comment
@googlebot
Copy link

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 carlfranz changed the title Update http.js fix($http): add check for functions in params Jul 27, 2017
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable. (TBD if it is a breaking change or not.)
Could you add some tests?

src/ng/http.js Outdated
@@ -42,6 +42,7 @@ function $HttpParamSerializerProvider() {
var parts = [];
forEachSorted(params, function(value, key) {
if (value === null || isUndefined(value)) return;
if (typeof value === 'function') return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can incorporate with the previous line:

if (value === null || isUndefined(value) || isFunction(value)) return;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done! Can't find how to rename wrong commit :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlfranz, you can amend your last commit with git commit --amend.
(Or if you want to change/reorganize multiple commits, git rebase --interactive <COMMIT_RANGE> is powerful tool. Too powerful - ue at your own risk 😛)

when submitting params as class-object checks and remove functions
default serializer should NOT serialize functions
default serializer shoud not serialize functions
@carlfranz carlfranz closed this Jul 27, 2017
@carlfranz
Copy link
Contributor Author

I created new one with the right commit format at #16133 please delete this. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants