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

#7127 | Uri: inconsistent query param names (url-encode) when .renderString-ing #7243

Open
wants to merge 2 commits into
base: series/0.23
Choose a base branch
from

Conversation

tothpeti
Copy link

This PR contains the fix for #7127

  • Add encode method in Query object
  • Encode query in Uri's fromString method
  • Add new test case
  • Refactored other ones to match the RFC 3986 requirements

@tothpeti
Copy link
Author

tothpeti commented Sep 3, 2023

Currently I feel like I am a bit stuck with this issue. At the current version of http4s, there are multiple ways of creating URI and add/update Query parameters.
Firstly, when we create a Uri via the Uri case class (or via any of the helper methods of Uri class), the given query parameters aren't encoded at all. However, when we would like to modify the query parameters by using any of the QueryOps methods, the query parameters will be instantly encoded. In my opinion, this inconsistency should be solved as well, because I am pretty sure that this inconsistency will come up again in the future.
My question is: Should I try to do a large refactor to solve the previously mentioned inconsistency? @armanbilge @danicheg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:core series/0.23 PRs targeting 0.23.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant