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

Fix plus signs in User fields becoming spaces in the set user request #572

Merged
merged 5 commits into from Oct 28, 2020

Conversation

cardoso
Copy link
Contributor

@cardoso cardoso commented Oct 28, 2020

The default encoding when building the query parameter with URLQueryItem will not encode the + symbols, thus making them become spaces. I fixed it by appending the json query parameter manually with the proper percent encoding to include the plus sign.

This can be specially annoying if a field is a base64 string, like an encoded public key, but it's also preventing creating a user with a plus in the name which is valid in other clients such as the JS client.

Example:
mhPcgwhH8L8FqFni20qLZ/bIHbwleWvaqm+zcByNp4FWQj0ufl8H9cHV/pbAYo74gCEUdH/MxXdtX+SmW2o3Rg==
becomes
mhPcgwhH8L8FqFni20qLZ/bIHbwleWvaqm zcByNp4FWQj0ufl8H9cHV/pbAYo74gCEUdH/MxXdtX SmW2o3Rg==
which is invalid base64

@cardoso cardoso added the 🐞 Bug An issue or PR related to a bug label Oct 28, 2020
@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Merging #572 into main will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #572   +/-   ##
=======================================
  Coverage   89.27%   89.28%           
=======================================
  Files         157      157           
  Lines        5940     5945    +5     
=======================================
+ Hits         5303     5308    +5     
  Misses        637      637           
Impacted Files Coverage Δ
...urces_v3/StreamChat/APIClient/RequestEncoder.swift 87.61% <100.00%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f916b21...283ccc4. Read the comment docs.

@VojtaStavik
Copy link
Contributor

@cardoso I added a test for a similar case for v3. It seems to be working fine. Can you confirm the change in a4124f4 is the problem you observed?

@cardoso
Copy link
Contributor Author

cardoso commented Oct 28, 2020

@VojtaStavik the bug also existed in v3. Your test was passing because you weren't checking the final absolute string to see if the plus was encoded. Should be fine now.

@cardoso cardoso merged commit 5a475ab into main Oct 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/v2/set_user_encoding branch October 28, 2020 23:16
cardoso added a commit that referenced this pull request Feb 10, 2021
…#572)

* Fix JSON encoding of query-allowed characters in set(user:) request

* Update CHANGELOG.md

* Add non-aplhanumeric characters to GET body encoding test

* Fix plus sign encoding in v3 GET request body

Co-authored-by: Vojta Stavik <stavik@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants