Refine how redirect method is set (#829)

Fixes #825. TIL: not all redirects should use the same method as
the original request, and in fact, most http client implementations
will default the new request method to GET. I included links to
a few useful discussions/references, but essentially the behavior
we now support is:
  * For 307/308 response status, use original request method; these
    statuses are specifically for indicating the original method is ok
  * For 303 response status, must use GET method
  * For other 3xx responses, it's actually legal to use the original
    method, but most client implementations will switch to GET,
    which means most servers have adapted to expect this and may even
    error if the original request method is used (as is the case in
    the originally reported issue). We now follow this behavior, though,
    like curl, allow overriding this behavior by a new `redirect_method`
    keyword arg to allow specifying the redirect method behavior.

src/HTTP.jl

@@ -149,6 +149,11 @@ Redirect options
 
  - `redirect = true`, follow 3xx redirect responses.
  - `redirect_limit = 3`, number of times to redirect.
+ - `redirect_method = nothing`, the method to use for the redirected request; by default,
+    GET will be used, only responses with 307/308 will use the same original request method.
+    Pass `:same` to pass the same method as the orginal request though note that some servers
+    may not response/accept the same method. It's also valid to pass the exact method to use
+    as a string, like `redirect_method="PUT"`.
  - `forwardheaders = true`, forward original headers on redirect.
 
 

src/Messages.jl

@@ -71,7 +71,7 @@ using ..Pairs
 using ..IOExtras
 using ..Parsers
 import ..@require, ..precondition_error
-import ..bytes
+import ..bytes, ..Form
 
 include("ascii.jl")
 
@@ -597,6 +597,12 @@ body_show_max = 1000
 The first chunk of the Message Body (for display purposes).
 """
 bodysummary(body) = isbytes(body) ? view(bytes(body), 1:min(nbytes(body), body_show_max)) : "[Message Body was streamed]"
+function bodysummary(body::Form)
+    if length(body.data) == 1 && isa(body.data[1], IOBuffer)
+        return body.data[1].data[1:body.data[1].ptr-1]
+    end
+    return "[Message Body was streamed]"
+end
 
 function compactstartline(m::Message)
     b = IOBuffer()

src/RedirectRequest.jl

@@ -14,7 +14,7 @@ export redirectlayer, nredirects
 Redirects the request in the case of 3xx response status.
 """
 function redirectlayer(handler)
-    return function(req; redirect::Bool=true, redirect_limit::Int=3, forwardheaders::Bool=true, response_stream=nothing, kw...)
+    return function(req; redirect::Bool=true, redirect_limit::Int=3, redirect_method=nothing, forwardheaders::Bool=true, response_stream=nothing, kw...)
         if !redirect || redirect_limit == 0
             # no redirecting
             return handler(req; redirect_limit=redirect_limit, kw...)
@@ -35,7 +35,9 @@ function redirectlayer(handler)
             # follow redirect
             oldurl = req.url
             url = resolvereference(req.url, location)
-            req = Request(req.method, resource(url), copy(req.headers), req.body;
+            method = newmethod(req.method, res.status, redirect_method)
+            body = method == "GET" ? UInt8[] : req.body
+            req = Request(method, resource(url), copy(req.headers), body;
                 url=url, version=req.version, responsebody=response_stream, parent=res, context=req.context)
             if forwardheaders
                 req.headers = filter(req.headers) do (header, _)
@@ -44,6 +46,8 @@ function redirectlayer(handler)
                         return false
                     elseif (header in SENSITIVE_HEADERS && !isdomainorsubdomain(url.host, oldurl.host))
                         return false
+                    elseif method == "GET" && header in ("Content-Type", "Content-Length")
+                        return false
                     else
                         return true
                     end
@@ -84,4 +88,21 @@ function verify_url(url::URI)
     end
 end
 
+function newmethod(request_method, response_status, redirect_method)
+    # using https://everything.curl.dev/http/redirects#get-or-post as a reference
+    # also reference: https://github.com/curl/curl/issues/5237#issuecomment-618293609
+    if response_status == 307 || response_status == 308
+        # specific status codes that indicate an identical request should be made to new location
+        return request_method
+    elseif response_status == 303
+        # 303 means it's a new/different URI, so only GET allowed
+        return "GET"
+    elseif redirect_method == :same
+        return request_method
+    elseif redirect_method !== nothing && redirect_method in ("GET", "POST", "PUT", "DELETE", "HEAD", "OPTIONS", "PATCH")
+        return redirect_method
+    end
+    return "GET"
+end
+
 end # module RedirectRequest