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

fixes issue with useRawUrl, queryString decode/encode and redirecting #14

Merged

Conversation

mbknor
Copy link
Contributor

@mbknor mbknor commented Jun 8, 2011

Hi,

Play Framework uses AHC. When we needed AHC to support multiple encodings, we had to encode everything ourself before passing it to AHC. For this to work we must use "useRawUrl" to prevent AHC from destroying our preencoded QueryStrring-params by trying to reencode it with utf-8 right before using the rebuilded url. We managed to get around all problems by tweaking how/when we gave the info to AHC.

The only problem that we found no solution for was how AHC was handling redirects with useRawUrl on.

If AHC gets the following redirect-Location when useRawUrls is on:
http://localhost:9003/rest/redirecttarget?data2=x%3D%26y
it wil first decode x%3D%26y into x=&y, then later add it to the rebuilded url as is.

The url used in the redirect will end up like this:
http://localhost:9003/rest/redirecttarget?data2=x=&y

This pullrequest fixes this issue by preventing AHC from decoding queryString-param names and values before adding them with addQueryParameter when useRawUrl is on.

This patch it by no means perfect, but it do fix the problem..

@jfarcand
Copy link
Contributor

jfarcand commented Jun 8, 2011

Sound good (except for the pom.xml changes :-)). Working on integrating the patch to the 1.7.0 (master).

@mbknor
Copy link
Contributor Author

mbknor commented Jun 8, 2011

thanks

@jfarcand jfarcand merged commit d9b690d into AsyncHttpClient:master Jun 8, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants