diff --git a/redirect_handler.go b/redirect_handler.go index 4d68f5a..f5efe37 100644 --- a/redirect_handler.go +++ b/redirect_handler.go @@ -132,6 +132,8 @@ const temporaryRedirect = 307 const permanentRedirect = 308 const locationHeader = "Location" +var errRedirectBodyNotReplayable = errors.New("redirect request body cannot be replayed") + // Intercept implements the interface and evaluates whether to follow a redirect response. func (middleware RedirectHandler) Intercept(pipeline Pipeline, middlewareIndex int, req *nethttp.Request) (*nethttp.Response, error) { obsOptions := GetObservabilityOptionsFromRequest(req) @@ -164,6 +166,9 @@ func (middleware RedirectHandler) redirectRequest(ctx context.Context, pipeline redirectCount++ redirectRequest, err := middleware.getRedirectRequest(req, response) if err != nil { + if errors.Is(err, errRedirectBodyNotReplayable) { + return response, nil + } return response, err } if observabilityName != "" { @@ -214,17 +219,58 @@ func (middleware RedirectHandler) getRedirectRequest(request *nethttp.Request, r result.Host = targetUrl.Host } + redirectMethod, includeBody := getRedirectMethodAndBody(request.Method, response.StatusCode) + result.Method = redirectMethod + if includeBody { + if request.Body != nil && request.Body != nethttp.NoBody { + if request.GetBody == nil { + return nil, errRedirectBodyNotReplayable + } + result.Body, err = request.GetBody() + if err != nil { + return nil, err + } + } + } else if request.Body != nil && request.Body != nethttp.NoBody || redirectMethod != request.Method { + dropRedirectRequestBody(result) + } + // Scrub sensitive headers before following the redirect scrubber := middleware.options.GetScrubSensitiveHeaders() if scrubber != nil { scrubber(result, request.URL) } - if response.StatusCode == seeOther { - result.Method = nethttp.MethodGet - result.Header.Del("Content-Type") - result.Header.Del("Content-Length") - result.Body = nil - } return result, nil } + +func getRedirectMethodAndBody(method string, statusCode int) (string, bool) { + switch statusCode { + case movedPermanently, found, seeOther: + if method != nethttp.MethodGet && method != nethttp.MethodHead { + return nethttp.MethodGet, false + } + return method, false + case temporaryRedirect, permanentRedirect: + return method, true + default: + return method, true + } +} + +func dropRedirectRequestBody(request *nethttp.Request) { + request.Body = nil + request.GetBody = nil + request.ContentLength = 0 + request.TransferEncoding = nil + request.Trailer = nil + for _, header := range []string{ + "Content-Length", + "Transfer-Encoding", + "Content-Type", + "Content-Encoding", + "Content-Language", + } { + request.Header.Del(header) + } +} diff --git a/redirect_handler_test.go b/redirect_handler_test.go index 8a27801..1ad5618 100644 --- a/redirect_handler_test.go +++ b/redirect_handler_test.go @@ -1,9 +1,11 @@ package nethttplibrary import ( + "io" nethttp "net/http" httptest "net/http/httptest" "net/url" + "strings" testing "testing" "strconv" @@ -327,3 +329,123 @@ func TestItKeepsHeadersOnRelativeUrlRedirect(t *testing.T) { assert.Equal(t, "session=SECRET", result.Header.Get("Cookie")) assert.Equal(t, "https://example.com/v2/api", result.URL.String()) } + +func TestRedirectMethodAndBodySemantics(t *testing.T) { + tests := []struct { + name string + statusCode int + method string + expectedMethod string + expectBody bool + }{ + {name: "301 POST becomes GET", statusCode: movedPermanently, method: nethttp.MethodPost, expectedMethod: nethttp.MethodGet}, + {name: "302 POST becomes GET", statusCode: found, method: nethttp.MethodPost, expectedMethod: nethttp.MethodGet}, + {name: "303 PUT becomes GET", statusCode: seeOther, method: nethttp.MethodPut, expectedMethod: nethttp.MethodGet}, + {name: "303 GET remains GET", statusCode: seeOther, method: nethttp.MethodGet, expectedMethod: nethttp.MethodGet}, + {name: "303 HEAD remains HEAD", statusCode: seeOther, method: nethttp.MethodHead, expectedMethod: nethttp.MethodHead}, + {name: "307 preserves POST and body", statusCode: temporaryRedirect, method: nethttp.MethodPost, expectedMethod: nethttp.MethodPost, expectBody: true}, + {name: "308 preserves POST and body", statusCode: permanentRedirect, method: nethttp.MethodPost, expectedMethod: nethttp.MethodPost, expectBody: true}, + } + + bodyHeaders := []string{ + "Content-Length", + "Transfer-Encoding", + "Content-Type", + "Content-Encoding", + "Content-Language", + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + handler := NewRedirectHandler() + req, err := nethttp.NewRequest(test.method, "https://example.com/source", strings.NewReader("request body")) + assert.NoError(t, err) + req.Header.Set("Content-Length", "12") + req.Header.Set("Transfer-Encoding", "chunked") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Content-Encoding", "gzip") + req.Header.Set("Content-Language", "en-US") + req.Header.Set("X-Custom", "preserved") + req.TransferEncoding = []string{"chunked"} + + _, err = io.ReadAll(req.Body) + assert.NoError(t, err) + + resp := &nethttp.Response{ + StatusCode: test.statusCode, + Header: nethttp.Header{"Location": []string{"https://example.com/target"}}, + } + result, err := handler.getRedirectRequest(req, resp) + assert.NoError(t, err) + assert.Equal(t, test.expectedMethod, result.Method) + assert.Equal(t, "preserved", result.Header.Get("X-Custom")) + + if test.expectBody { + replayedBody, readErr := io.ReadAll(result.Body) + assert.NoError(t, readErr) + assert.Equal(t, "request body", string(replayedBody)) + assert.NotNil(t, result.GetBody) + assert.Equal(t, req.ContentLength, result.ContentLength) + assert.Equal(t, req.TransferEncoding, result.TransferEncoding) + for _, header := range bodyHeaders { + assert.Equal(t, req.Header.Get(header), result.Header.Get(header)) + } + } else { + assert.Nil(t, result.Body) + assert.Nil(t, result.GetBody) + assert.Zero(t, result.ContentLength) + assert.Nil(t, result.TransferEncoding) + assert.Nil(t, result.Trailer) + for _, header := range bodyHeaders { + assert.Empty(t, result.Header.Get(header)) + } + } + }) + } +} + +func TestItDoesNotFollow307Or308WithUnreplayableBody(t *testing.T) { + for _, statusCode := range []int{temporaryRedirect, permanentRedirect} { + t.Run(strconv.Itoa(statusCode), func(t *testing.T) { + requestCount := 0 + testServer := httptest.NewServer(nethttp.HandlerFunc(func(res nethttp.ResponseWriter, req *nethttp.Request) { + requestCount++ + res.Header().Set("Location", "/redirected") + res.WriteHeader(statusCode) + })) + defer testServer.Close() + + req, err := nethttp.NewRequest(nethttp.MethodPost, testServer.URL, io.NopCloser(strings.NewReader("request body"))) + assert.NoError(t, err) + assert.Nil(t, req.GetBody) + + resp, err := NewRedirectHandler().Intercept(newNoopPipeline(), 0, req) + assert.NoError(t, err) + assert.Equal(t, statusCode, resp.StatusCode) + assert.Equal(t, 1, requestCount) + }) + } +} + +func TestRedirectDropsBodyHeadersAndSensitiveHeadersAcrossOrigins(t *testing.T) { + handler := NewRedirectHandler() + req, err := nethttp.NewRequest(nethttp.MethodPost, "https://example.com/source", strings.NewReader("request body")) + assert.NoError(t, err) + req.Header.Set("Authorization", "******") + req.Header.Set("Cookie", "session=SECRET") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Content-Encoding", "gzip") + + resp := &nethttp.Response{ + StatusCode: found, + Header: nethttp.Header{"Location": []string{"https://other.example.com/target"}}, + } + result, err := handler.getRedirectRequest(req, resp) + assert.NoError(t, err) + assert.Equal(t, nethttp.MethodGet, result.Method) + assert.Nil(t, result.Body) + assert.Empty(t, result.Header.Get("Content-Type")) + assert.Empty(t, result.Header.Get("Content-Encoding")) + assert.Empty(t, result.Header.Get("Authorization")) + assert.Empty(t, result.Header.Get("Cookie")) +}