socketry/async-http

`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!