`Async::HTTP::RelativeLocation` doesn't seem to work for absolute URLs
Closed this issue · 2 comments
In RelativeLocation
, we immediately return if a URI is absolute:
while hops <= @maximum_hops
response = super(request)
if response.redirection?
hops += 1
# Get the redirect location:
unless location = response.headers['location']
return response
end
response.finish
uri = URI.parse(location)
if uri.absolute?
return response
else
...
Perhaps I am using it wrong, but it seems like this doesn't follow the absolute redirect as it should?
If I change this block, the while
loop correctly iterates and the redirect is transparently followed as I would expect:
if uri.absolute?
endpoint = Endpoint[uri]
request.scheme = endpoint.scheme
request.authority = endpoint.authority
request.path = endpoint.path
else
...
Maybe this isn't the intended behavior (please close this issue and disregard) or I am using the library incorrectly - I didn't see a test for absolute redirects, but the comments seem to indicate that this is a "Client wrapper which transparently handles both relative and absolute redirects to a given maximum number of hops."
Ah yes, that comment is incorrect.
It's a security issue to redirect to absolute URLs IMHO.
I'll fix the documentation.
I agree that following redirects shouldn't be implicit and could potentially present security issues.
We've found there are plenty of places where absolute redirects are necessary and safe: following redirects to S3 pre-signed URLs being an easy, obvious case.
URI::HTTP.open
does seem to follow all same-scheme redirects by default.
#172 should make it simple and clean to build additional behavior/filtered redirects/etc. on top of RelativeLocation
though.
Thanks for the quick follow-up!