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.

Sign up for GitHub

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

Jump to bottom

Lowercase location header not recognized when constructing %HTTPoison.MaybeRedirect{} #453

Closed
gianluca-nitti opened this issue Jan 26, 2022 · 0 comments
Closed

Lowercase location header not recognized when constructing %HTTPoison.MaybeRedirect{} #453

gianluca-nitti opened this issue Jan 26, 2022 · 0 comments

Comments

@gianluca-nitti
Copy link
Contributor

I've noticed that #424 introduced the struct %HTTPoison.MaybeRedirect{} which is returned when 3xx status codes are returned from the servers in response to requests with methods other that GET/HEAD/POST and the follow_redirect: true option.

This struct's redirect_url field is however properly populated only if the Location header returned by the server has an uppercase L; if it is, for example, location: http://example.com we get redirect_url: nil.

Since RFC 2616 Section 4.2 states that Field names are case-insensitive, in my understanding the keys of the proplist here should be lowercased and then the location key should be accessed.
gianluca-nitti pushed a commit to gianluca-nitti/tus_client that referenced this issue Jan 26, 2022
[WIP] Update HTTPoison and fix PATCH 301 handling
bec5e6b 
Updating HTTPoison to 1.8 which includes edgurgel/httpoison#424,
implementing proper handling of redirects. However, WIP because:
- edgurgel/httpoison#453
- need to investigate whether the same (handling
  %HTTPoison.MaybeRedirect{} results) is needed for other requests
  (e.g. options)
gianluca-nitti pushed a commit to gianluca-nitti/tus_client that referenced this issue Jan 26, 2022
[WIP] Update HTTPoison and fix PATCH 301 handling
e27114e 
Updating HTTPoison to 1.8 which includes edgurgel/httpoison#424,
implementing proper handling of redirects. However, WIP because:
- edgurgel/httpoison#453
- need to investigate whether the same (handling
  %HTTPoison.MaybeRedirect{} results) is needed for other requests
  (e.g. options)
gianluca-nitti added a commit to gianluca-nitti/httpoison that referenced this issue Jan 26, 2022
@gianluca-nitti
Recognize lowercase location header (fix edgurgel#453)
0451d28
@gianluca-nitti gianluca-nitti mentioned this issue Jan 26, 2022
Merged
gianluca-nitti pushed a commit to gianluca-nitti/tus_client that referenced this issue Jan 26, 2022
Follow redirects for all HTTP methods
01f216e 
Notes:
- edgurgel/httpoison#453 is manually worked around
- there is no redirect loop detection, it may make sense to add an
  option (with a default) to limit the max number of redirects
- POST requests which receive a 303 respose are followed internally by
  HTTPoison but with the GET method, thus if the server replies 303 to a
  POST request the tus upload will not work properly
gianluca-nitti pushed a commit to gianluca-nitti/tus_client that referenced this issue Feb 1, 2022
Update HTTPoison and fix PATCH 301 handling
1710e32 
Updated HTTPoison to 1.8 which includes edgurgel/httpoison#424,
implementing proper handling of redirects. Additionally, we now
follow redirects for all HTTP methods (not just PATCH).

NOTES (in order of importance):
- POST requests which receive a 303 respose are followed internally by
  HTTPoison but with the GET method, thus if the server replies 303 to a
  POST request the tus upload will not work properly
- there is no redirect loop detection, it may make sense to add an
  option (with a default) to limit the max number of redirects
- edgurgel/httpoison#453 is manually worked around
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@gianluca-nitti