Okhttp: Query parameters with pipes (|) result in invalid calls

[GuiForget]

This is a follow up of the bugs initially reported here and here

The workaround provided in the latter works but I would think okhttp should be able to do that without having to write an interceptor.

I chased the bug all the way to this code:
https://github.com/square/okhttp/blob/7135628c645892faf1a48a8cff464e0ed4ad88cb/okhttp/src/main/java/okhttp3/internal/http/RequestLine.java#L37-L41

This uses the URL set of characters, not the URI set of characters. I'm not entirely sure of the consequences of changing the code and why I didn't submit a pull request.

[swankjesse]

What action would you like for us to take? OkHttp’s treatment of the | character in a query is consistent with Chrome, Firefox, and Safari.

http://tinyurl.com/url-escaping

[GuiForget]

I would expect it to be escaped like you do already for some characters. Namely right now you already encode the characters present in okhttp3.HttpUrl.QUERY_COMPONENT_ENCODE_SET. But you could encode more aggressively and also encode the characters in okhttp3.HttpUrl.QUERY_COMPONENT_ENCODE_SET_URI

And if you don't provide it by default, at least make this an option so that client would want to use the more strict RFCs can.

[swankjesse]

Unfortunately, we can’t just encode | without breaking servers that expect it to not be encoded. So we choose to be consistent with the major web browsers which don’t encode |.

It’s frustrating that this keeps coming up. Unfortunately I think the best place to fix this is server-side.