http/client - edge case on POST and redirect

[Shnitzelil]

Go version

1.23.2

Output of go env in your module/workspace:

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\<username>\AppData\Local\go-build
set GOENV=C:\Users\<username>\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\<my user>\go\pkg\mod
set GONOPROXY=gitlab.<my org>.net
set GONOSUMDB=gitlab.<my org>.net
set GOOS=windows
set GOPATH=C:\Users\<my user>\go
set GOPRIVATE=gitlab.no longer exists>.net
set GOPROXY=https://proxy.no longer exists>.org,direct
set GOROOT=C:/Program Files/Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=local
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.23.2
set GODEBUG=
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\<my user>\AppData\Roaming\go\telemetry
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\scm\<my project>\go.mod
set GOWORK=
set CGO_CFLAGS=-IC:/scm/ZMQ/include
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-LC:/scm/ZMQ/bin
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\<my user>\AppData\Local\Temp\go-build2742211471=/tmp/go-build -gno-record-gcc-switches

What did you do?

Using simple http client to execute POST with body the server response with series of redirect and enter loop (till reached to 10th redirects).

I used Burp (Fiddler like) to see the communication b/w the client and server.

What did you see happen?

The go client received several redirect responses
302 -> 301 -> 308 -> 302 -> 301 -> 308... until it reached to the 10th redirects.

What did you expect to see?

It was supposed to stop after the 308.
The reason is simple it should send the last one w/o body and Content Length.

I was able to modify the Go SDK (net/http/client.go) with this change on function redirectBehavior

// redirectBehavior describes what should happen when the
// client encounters a 3xx status code from the server.
func redirectBehavior(reqMethod string, resp *Response, ireq *Request) (redirectMethod string, shouldRedirect, includeBody bool) {
	switch resp.StatusCode {
	case 301, 302, 303:
		redirectMethod = reqMethod
		shouldRedirect = true
		includeBody = false

		// RFC 2616 allowed automatic redirection only with GET and
		// HEAD requests. RFC 7231 lifts this restriction, but we still
		// restrict other methods to GET to maintain compatibility.
		// See Issue 18570.
		if reqMethod != "GET" && reqMethod != "HEAD" {
			redirectMethod = "GET"
		}
	case 307, 308:
		redirectMethod = reqMethod
		shouldRedirect = true
		includeBody = true

		if ireq.GetBody == nil && ireq.outgoingLength() != 0 {
			// We had a request body, and 307/308 require
			// re-sending it, but GetBody is not defined. So just
			// return this response to the user instead of an
			// error, like we did in Go 1.7 and earlier.
			shouldRedirect = false
		}
		
		if reqMethod == "GET" && ireq.Method == "POST" {
			includeBody = false
		}
	}
	return redirectMethod, shouldRedirect, includeBody
}

[gabyhelp]

Related Issues and Documentation

• net/http: use 308 instead of 301 in Server to preserve POST payload in redirects #50243
• net/http: handle 3xx redirects properly
• net/http: tweak the new Client 307/308 redirect behavior a bit
• net/http: 302 redirect of DELETE to GET isn't RFC compliant #41377
• net/http: don't send body on redirects for 301, 302, 303 when GetBody is set
• net/http: Post redirect can submit entity body when redirecting with GET request #57273
• net/http: regression in client redirect logic #18570 (closed)
• net/http: resp.Body should be closed when error occurred during redirection #19976 (closed)
• net/http: 307 and 308 status codes should not change method on redirect #16840 (closed)
• net/http: Redirect conditions are incorrect, support 307/308 #10767 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308

The request method and the body will not be modified by the client in the redirected request. A 301 Moved Permanently requires the request method and the body to remain unchanged when redirection is performed, but this is incorrectly handled by older clients to use the GET method instead.

modifying the request is incorrect for 308

[Shnitzelil]

So how it come and change my method to GET?

[seankhliao]

that's the 301

[Shnitzelil]

Expected behavior of handling HTTP 308 redirection is that the client must repeat the same type of request (either POST or GET) on the target location. And it works properly when server responds with HTTP 308 on POST request (the redirected request will be also POST with the same body and only different URL following value of Location header).

In our case the flow is different - after several "normal" redirections (following HTTP 302 and HTTP 301) client receives HTTP 308 response and following the logic should respond with the same method as in previous request (that was GET without body) to the new location.
This way it works in browsers.
Thus the handling of HTTP 308 response in our (and GO) code should refer the request that responded with HTTP 308 and not the initial one ("first") in the redirections sequence.

Please advise

[seankhliao]

Our handling of redirects is consistent with other tools such as curl. If you need different behaviour, CheckRedirect is available for you to set.

[Shnitzelil]

I've checked with curl...
Curl

1. Send POST with Body and receive status code 302
2. Send POST w/o Body and receive status code 200.

Go

1. Send POST with Body and receive status code 302
2. Send Get w/o Body and receive status code 301
3. Send Get with Body and receive status code 308
4. And back to 1

So you should check that.. there is something wrong with the redirections...