Skip to content

Commit

Permalink
Extract redirect location handling and clarify behaviour.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Aug 14, 2024
1 parent ae73872 commit 51c4c14
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 13 deletions.
47 changes: 36 additions & 11 deletions lib/async/http/relative_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ module HTTP
class TooManyRedirects < StandardError
end

# A client wrapper which transparently handles both relative and absolute redirects to a given maximum number of hops.
# A client wrapper which transparently handles redirects to a given maximum number of hops.
#
# The default implementation will only follow relative locations (i.e. those without a scheme) and will switch to GET if the original request was not a GET.
#
# The best reference for these semantics is defined by the [Fetch specification](https://fetch.spec.whatwg.org/#http-redirect-fetch).
#
Expand Down Expand Up @@ -58,14 +60,38 @@ def redirect_with_get?(request, response)
end
end

# Handle a redirect to a relative location.
#
# @parameter request [Protocol::HTTP::Request] The original request, which you can modify if you want to handle the redirect.
# @parameter location [String] The relative location to redirect to.
# @returns [Boolean] True if the redirect was handled, false if it was not.
def handle_redirect(request, location)
uri = URI.parse(location)

if uri.absolute?
return false
end

# Update the path of the request:
request.path = Reference[request.path] + location

# Follow the redirect:
return true
end

def call(request)
# We don't want to follow redirects for HEAD requests:
return super if request.head?

if body = request.body
# We need to cache the body as it might be submitted multiple times if we get a response status of 307 or 308:
body = ::Protocol::HTTP::Body::Rewindable.new(body)
request.body = body
if body.respond_to?(:rewind)
# The request body was already rewindable, so use it as is:
body = request.body
else
# The request body was not rewindable, and we might need to resubmit it if we get a response status of 307 or 308, so make it rewindable:
body = ::Protocol::HTTP::Body::Rewindable.new(body)
request.body = body
end
end

hops = 0
Expand All @@ -83,23 +109,22 @@ def call(request)

response.finish

uri = URI.parse(location)

if uri.absolute?
unless handle_redirect(request, location)
return response
else
request.path = Reference[request.path] + location
end

# Ensure the request (body) is finished and set to nil before we manipulate the request:
request.finish

if request.method == GET or response.preserve_method?
# We (might) need to rewind the body so that it can be submitted again:
body&.rewind
request.body = body
else
# We are changing the method to GET:
request.method = GET

# Clear the request body:
request.finish
# We will no longer be submitting the body:
body = nil

# Remove any headers which are not allowed in a GET request:
Expand Down
4 changes: 2 additions & 2 deletions test/async/http/relative_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
end

it 'should fail with maximum redirects' do
expect{
expect do
response = relative_location.get('/home')
}.to raise_exception(Async::HTTP::TooManyRedirects, message: be =~ /maximum/)
end.to raise_exception(Async::HTTP::TooManyRedirects, message: be =~ /maximum/)
end
end
end
Expand Down

0 comments on commit 51c4c14

Please sign in to comment.