Fix inconsistent query_to_string with +
#874
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi!
Currently, the documentation indicates in
uri.parse_query"The opposite operation isuri.query_to_string."In
uri.query_to_string, we can read "The opposite operation isuri.parse_query.".Unfortunately, when writing this
The code fails. It fails because — from my understanding — while
+is an allowed character in query strings in the form%2B,uri.query_to_stringdoes not follows the URI query string convertion and instead replaces%2Bwith+. Indeed, outside of query strings,+has another meaning, and should not be converted. It makes sense to not convert it inuri.percent_encode, but it should be inuri.query_to_string.The above code, when changing
1 + 1to1 %2B 1works. I think it's not the desired behaviour here, and it's confusing for developers. From my understanding, when manipulating query strings as keyed lists, the value associated with the key can be any string, including one with+. Usinguri.query_to_stringwithout the knowledge that+won't be perserved in percent encoding lead to hard to find bugs.The proposition here is to properly encode the query strings, and to add a test case with the special characters, where some of them are "unreserved characters", like
~for example.I wanted to directly open the PR to talk about that with some code to illustrate, but anything can be changed or the PR can be closed if you estimate it's not desired in the standard library. 🙂
In any case, I think it is important to find a way to communicate how to use URI correctly, as it can be confusing to understand when to use
percent_encode, etc. when the framework/libraries you're using is not doing the hard work for you. I wanted to write some documentation but I couldn't find a proper place to do it. Do you have any recommandation on that topic?For information, that bug surfaced when we had to handle
+in query strings in our application, and currently we're just happily usinguri.query_to_stringbefore setting the query string in auri.Uri. We could simply run thestring.replace(_, each: "+", with: "%2B"), but I suspect others could end up with the same issue, and struggle to understand why it bugs, asuri.query_to_stringis not the exact opposite function ofuri.parse_query. 😄