Escape special chars in QueryParams

[lagunoff]

closes #1584

I was investigating a bug in our application and it turned out it was caused by servant-client not escaping plus signs in query params. After some digging down the code in servant-client I noticed servant-client uses toEncodedUrlPiece to encode the values of QueryParams, which is wrong because it should be used for path pieces only and it does not escape characters :@&=+$,. Later I found #1584 and #1586. Here I'm suggesting a workaround — using urlEncodeBuilder True directly instead of toEncodedUrlPiece.

At first I thought the toEncodedUrlPiece is itself the source of the problem and created a PR to fizruk/http-api-data#119. The conclusion there was that it is wrong to encode query params with toEncodedUrlPiece and that this should be changed in servant-client, even though it's weird that ToHttpAPIData only has method for encoding path pieces

[jkarni]

Thanks! And sorry this took so long.

[jkarni]

@lagunoff @tomsmalley it ike this PR causes a failure (https://github.com/haskell-servant/servant/actions/runs/4752900295/jobs/8443824136#step:8:352) - not exactly sure why it didn't fail in the previous run though. Any thoughts on how to fix this?

[tomsmalley]

The failing test (binary data in query param) was introduced by the change which broke query param text encoding (#1432). I think it's either one or the other unless we add toEncodedQuery to ToHttpApiData. If the decision is to walk back #1432 then removing the test case seems fine.

[lagunoff]

Hi, I'm afraid I can't provide any help with this issue. It's not clear what's going on under the hood in the test case it seems it sends a request and compares the response body, but I'm not familiar with the codebase can't dig deeper where exactly encoding/decoding fails

[ysangkok]

For any subscribers on this issue, the failing test is proposed to be removed in #1679