Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw when query is encoded incorrectly #410

Merged
merged 1 commit into from
Feb 19, 2016

Conversation

Marco129
Copy link
Contributor

Fix for #101 #211
Since the original req.query did not decode correctly, curl with --data-urlencode will fail.

@flovilmart
Copy link
Contributor

@Marco129 can you add tests to emphasize on the fix?, also given the API from expressjs here: http://expressjs.com/en/4x/api.html#req.query the req.query object should be set.

@Marco129
Copy link
Contributor Author

Looks like I wrongly encoded the = sign in url parameters which is not a common practise (at least not stated in the current REST API doc)

However, current Parse API do decode the = sign correctly and return the filtered results while parse server will ignore the query constraint.

  • classes/Test?limit%3D2 [will work in Parse API]
  • classes/Test?limit=2 [will work in Parse API & Parse Server]

Should we maintain this compatibility?

@flovilmart
Copy link
Contributor

Yeah the = signs are not supposed to be encoded unless they are part of a value to make the difference between the parameters and the value they hold.

There is a risk with the double encoding as you may unencode a 'wrong' = sign and then render the params unusable.

If we handle the = sign encoding (classes/Test?limit%3D2), that should be handled in a middleware instead of at the place you put it there. That will simplify bug tracking.
For the encoded = sign handing, write appropriate tests to make sure the 'embedded' '=' are properly double encoded and won't interfere with the default query params.

@facebook-github-bot
Copy link

@Marco129 updated the pull request.

@facebook-github-bot
Copy link

@Marco129 updated the pull request.

@Marco129
Copy link
Contributor Author

After several test, I found that Parse API also buggy in handling the encoded = sign.

As encoding = sign is not a correct method, and it is odd that when I put something like ?limit%3D2 and the result is not return as expected. Rather than support this special case, I think throwing error will be better.

@facebook-github-bot
Copy link

@Marco129 updated the pull request.

@gfosco gfosco changed the title Fix query constraints not working Throw when query is encoded incorrectly Feb 19, 2016
gfosco added a commit that referenced this pull request Feb 19, 2016
Throw when query is encoded incorrectly
@gfosco gfosco merged commit 39f8143 into parse-community:master Feb 19, 2016
@gfosco
Copy link
Contributor

gfosco commented Feb 19, 2016

👍

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

Successfully merging this pull request may close these issues.

4 participants