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

Option to use "Location:" header value verbatim for redirection #864

Closed
chanseokoh opened this issue Oct 30, 2019 · 4 comments · Fixed by #871
Closed

Option to use "Location:" header value verbatim for redirection #864

chanseokoh opened this issue Oct 30, 2019 · 4 comments · Fixed by #871
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@chanseokoh
Copy link
Contributor

chanseokoh commented Oct 30, 2019

Is your feature request related to a problem? Please describe.
There have been problems with interacting Azure Container Registry, OpenShift Container Registry, and Red Hat Quay, where these servers return a redirect location (the Location: header value) but they fail to honor a redirect URL if it doesn't match the original URL value char-by-char.

(FYI, the ACR issue has been taken care of by setting setNormalizeUri(false) on Apache HttpClient. However, Microsoft should fix their servers eventually.)

Examples:

Describe the solution you'd like
As described in #795 (comment), an option to take the Location: URL value and use it verbatim, without ever modifying any character.

Describe alternatives you've considered
Wait for Red Hat to fix their servers, which they said won't likely happen for a very long time.

Additional context
Fixing this may first require enhancing GenericUrl to disable encoding and decoding (but I am not sure).

@iocanel
Copy link
Contributor

iocanel commented Oct 31, 2019

@chanseokoh: Getting the Location: and using it as is, doesn't seem very hard to implement, does this also require additional work in httpcomponents-client side?

@chanseokoh
Copy link
Contributor Author

chanseokoh commented Oct 31, 2019

I quickly hacked GenericUrl and tried a call to see if the server side can get the URL as-is, and it seems to get it as-is. This library sets setNormalizeUri(false) on the Apache library, so perhaps the Apache library does not mangle with it either.

Implenenting this feature may not be terribly difficult, but it may need cascading changes to GenericUrl, UrlEncodedParser and some others, I don't know. And since the API is GA and public, I see such changes cannot be taken lightly.

@iocanel
Copy link
Contributor

iocanel commented Oct 31, 2019

@chanseokoh: I've started working on a pull request (modifying GenericUrl and UrlEncodedParser).
Please let me know if I should continue, or if you are going to pick this up.

@chanseokoh
Copy link
Contributor Author

chanseokoh commented Oct 31, 2019

Wonderful! You can continue on this, thanks a ton. And note that I believe this library has an automatic redirection handling (the user does not have to parse Location: and make another redirect call themselves), so make sure that is taken care of too. Or perhaps, you just take care of that execution path only, without making this URL-pass-through general for any calls. I am not sure.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Oct 31, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Nov 4, 2019
@codyoss codyoss added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants