QueryParams.__str__ does not round-trip % characters from input

[jhominal]

The issue is similar to #3094 but there is a case to be made that the behavior in the other discussion may be working as intended.

However, I currently have an issue that % does not roundtrip when using httpx.QueryParams:

• QueryParams.__init__ will decode a %25 sequence in a str to %;
• QueryParams.__str__ considers % as being "safe", so will not reencode % to %25;

This has the following consequences:

# The next line has the expected behavior - these are different query strings, and have different items after being parsed
assert httpx.QueryParams("%257F=%257F") != httpx.QueryParams("%7F=%7F")

# However, encoding using str returns the same value for both of these objects:
assert str(httpx.QueryParams("%257F=%257F")) == str(httpx.QueryParams("%7F=%7F")) == "%7F=%7F"

# As a related issue, it becomes possible to mix encoded and unencoded parameter keys so that they will be confused after URL encoding:
assert str(httpx.QueryParams({"%7F": "a", "\x7f": "b"})) == '%7F=a&%7F=b'

This issue is particularly notable when using e.g. URL.copy_add_param and its related functions, as they rely on QueryParams.__str__ round-tripping back to its original value:

url = httpx.URL("https://www.example.com/?encoded%257F=value%257F")

# httpx.URL stores the query component as a string so it does not get altered
assert str(url) == "https://www.example.com/?encoded%257F=value%257F"

# However, manipulating the query parameters using copy_add_param changes the previously-inserted URL parameters
assert str(url.copy_add_param("a", "b")) == "https://www.example.com/?encoded%7F=value%7F&a=b"

I believe that this behavior is a bug in httpx and should be resolved.

The potential bug described here was, as far as I can tell, introduced in httpx version 0.24.0.

[jhominal]

@encode/maintainers I would really like some kind of reply on this discussion, especially given that there is already pending PR #3141 which, in my opinion, is at least a neighbor of the underlying issue.

[Kludex]

@tomchristie Is the most suitable person to answer, I guess.

[tomchristie]

👋 Yep, will get around to looking at this. Just... busy.

[jhominal]

@tomchristie hello, if you are still busy feel free to ignore this message, but if you have some time to look at this, I would be interested in at least your opinion on whether it is a bug to be resolved (In which case, I can advance to making this an issue and work on a PR).

[tomchristie]

Review... Yes, our behavior here is incorrect.

[tomchristie]

Oaky, resolution is here... #3187

Description, new tests, and test change incoming.