Fix: Fixing redirect 301 handling

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)
+	}
+}

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"))
+}