Extract redirect location handling and clarify behaviour.

Author: ioquatix

lib/async/http/relative_location.rb

@@ -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).
 		#
@@ -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
@@ -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:

test/async/http/relative_location.rb

@@ -30,7 +30,10 @@
 			end
 
 			it 'should redirect POST to GET' do
-				response = relative_location.post('/')
+				body = Protocol::HTTP::Body::Buffered.wrap(["Hello, World!"])
+				expect(body).to receive(:finish)
+				
+				response = relative_location.post('/', {}, body)
 
 				expect(response).to be(:success?)
 				expect(response.read).to be == "GET"
@@ -44,9 +47,22 @@
 				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
+			
+			with "handle_redirect returning false" do
+				before do
+					expect(relative_location).to receive(:handle_redirect).and_return(false)
+				end
+				
+				it "should not follow the redirect" do
+					response = relative_location.get('/')
+					response.finish
+					
+					expect(response).to be(:redirection?)
 				end
 			end
 		end