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

Redirect handling and GenericUrl not woriking well together (resulting in 404) when redirected to any URL with a path containing the '+' character #296

Closed
VICO-Research opened this issue Aug 13, 2015 · 2 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@VICO-Research
Copy link

When handling redirects for GET requests, the next request is made with an GenericUrl instance constructed from the value of the Location header like this: new GenericUrl(url.toURL(redirectLocation)). If redirectLocation contains a '+' character in the path it effectively gets converted to a space and encoded as %20 which breaks the URL. This is a rather big issue trying to work Google+. In my opinion the root of the problem is the toPathParts method in GenericUrl which uses CharEscapers.decodeUri on each part despite of its documentation which states it should not be used on path because it replaces '+' with ' '.

@dpapworth
Copy link

dpapworth commented Nov 18, 2016

+1 this issue. @VICO-Research 's suggestion will cause '+' to be encoded as %2B, as the path encoding in appendRawPathFromParts is also incorrect. The CharEscapers.escapeUriPath method doesn't recognise '+' as a subdelimiter character (see https://tools.ietf.org/html/rfc3986).

@ejona86 ejona86 added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Feb 22, 2017
@JustinBeckwith JustinBeckwith added priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. labels Jun 6, 2018
@ajaaym
Copy link
Contributor

ajaaym commented Dec 10, 2018

We are tracking this issue here closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

5 participants