-
Notifications
You must be signed in to change notification settings - Fork 24
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
URL-encode param values in GET query strings #41
URL-encode param values in GET query strings #41
Conversation
Commit 00b6b67 (tag: v0.1.12) "Fix request helper for get requests to build URI based on how API expects it" attempted to solve some unclearly-indentified problem with encoding the query string in GET-based API calls. The commit leaves param values unencoded/as-is instead of URL-encoding them. Unfortunately, that causes a regression in that param values containing spaces lead to the error "EOFError: end of file reached". I'm not sure what regressions we will cause by _reverting_ 00b6b67, but _that_ implementation is clearly broken.
The main difference between the two implementations is whether they URL-encode the values, however there is another difference: how they handle |
Some tests are failing, but it seems to me that the implementation of im.rb is wrong. I don't see any mention of |
The `/api/v1/im.counters` endpoint does not support such a parameter.
9c5ecd5
to
7abfa2f
Compare
Would also be great to see a new spec to test this changed behaviour in the URI encoding.. ie passing through a value containing something that should be encoded (say an |
Refactor all im.counters tests
hey @nmagedman just checking if you were still good to add some tests for the changed behaviour in the URI encoding.. ie passing through a value containing something that should be encoded (say an &) ? |
03408c0
to
b4023b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @nmagedman 👍
unreadsFrom: '2019-01-01T12:34:56.789Z', | ||
msgs: 4, | ||
latest: '2019-01-23T01:23:45.678Z', | ||
userMentions: 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 nice
Released in v0.2.1 |
Commit 00b6b67 (tag: v0.1.12) "Fix request helper for get requests to build URI based on how API expects it" attempted to solve some unclearly-indentified problem with encoding the query string in GET-based API calls. The commit leaves param values unencoded/as-is instead of URL-encoding them.
Unfortunately, that causes a regression in that param values containing spaces lead to the error "EOFError: end of file reached".
I'm not sure what regressions we will cause by reverting 00b6b67, but that implementation is clearly broken.