net/http: use 308 instead of 301 in Server to preserve POST payload in redirects

[vkuznet]

What version of Go are you using (go version)?

$ go version
go1.17.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

darwin/amd64

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/vk/Library/Caches/go-build"
GOENV="/Users/vk/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/vk/Work/Languages/Go/gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB="github.com/galeone/tensorflow"
GOOS="darwin"
GOPATH="/Users/vk/Work/Languages/Go/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/local/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/local/lib/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="/usr/bin/clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/vk/tmp/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vp/p89gqpvd4f93zzqpz4g5jnvm0000gp/T/go-build718658130=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Here is go server code:

package main

import (
    "fmt"
    "io/ioutil"
    "log"
    "net/http"
)

// ReqestHandler handles HTTP requests
func RequestHandler(w http.ResponseWriter, r *http.Request) {
    defer r.Body.Close()
    body, _ := ioutil.ReadAll(r.Body)
    data := fmt.Sprintf("%s method, body %v", r.Method, string(body))
    log.Println(data)
    w.Write([]byte(data))
}

// http server implementation
func main() {
    http.HandleFunc("/api", RequestHandler)
    log.Fatal(http.ListenAndServe(":9999", nil))
}

Here is curl client which issues POST request with json payload to api

curl -v -X POST "Content-type: application/json" -d '{"foo":1}' http://localhost:9999/api
Note: Unnecessary use of -X or --request, POST is already inferred.
* Closing connection -1
curl: (3) URL using bad/illegal format or missing URL
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:9999...
* Connected to localhost (127.0.0.1) port 9999 (#0)
> POST /api HTTP/1.1
> Host: localhost:9999
> User-Agent: curl/7.80.0
> Accept: */*
> Content-Length: 9
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Fri, 17 Dec 2021 13:05:31 GMT
< Content-Length: 27
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact
POST method, body {"foo":1}

At this point our server correctly identify POST payload and print it out on stdout.

Now, let's change URI on a client side (we will include double slashes) to force HTTP request redirect:

curl -v -L -X POST "Content-type: application/json" -d '{"foo":1}' http://localhost:9999//api
Note: Unnecessary use of -X or --request, POST is already inferred.
* Closing connection -1
curl: (3) URL using bad/illegal format or missing URL
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:9999...
* Connected to localhost (127.0.0.1) port 9999 (#0)
> POST //api HTTP/1.1
> Host: localhost:9999
> User-Agent: curl/7.80.0
> Accept: */*
> Content-Length: 9
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Location: /api
< Date: Fri, 17 Dec 2021 13:05:54 GMT
< Content-Length: 0
<
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:9999/api'
* Switch from POST to GET
* Found bundle for host localhost: 0x7f8657f10b90 [serially]
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (127.0.0.1) port 9999 (#0)
> POST /api HTTP/1.1
> Host: localhost:9999
> User-Agent: curl/7.80.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Fri, 17 Dec 2021 13:05:54 GMT
< Content-Length: 18
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact
POST method, body

Here, see last line, our server correctly identify POST request but not its payload, i.e. due to redirect to GET and then back to POST we lost our payload in HTTP request flow.

There are two possible redirects which server can use, the 301 StatusMovedPermanently and 308 StatusPermanentRedirect. The former, according to RFC7231 will change original POST request to GET during redirect, while latter (according to RFC7238) will not allow to change original POST request. Based on which status is used by Go http/server.go the behavior on a client side can change.

So far, Go net/http/serve.go uses 301 StatusMovedPermanently in different places, see:

• https://github.com/golang/go/blob/master/src/net/http/server.go#L2406
• https://github.com/golang/go/blob/master/src/net/http/server.go#L2420
• https://github.com/golang/go/blob/master/src/net/http/server.go#L2426

I suggest to change it to 308 StatusPermanentRedirect to preserve original HTTP request from the client.

What did you expect to see?

If we change StatusMovedPermanently to StatusPermanentRedirect in net/http/server.go code, then the client will properly follow the redirect and its payload will not be lost. After I changed that in net/http/server.go and recompiled/restarted my server code I see now that my client correctly follows the redirect and its payload is received by the server:

curl -v -L -X POST "Content-type: application/json" -d '{"foo":1}' http://localhost:9999//api
Note: Unnecessary use of -X or --request, POST is already inferred.
* Closing connection -1
curl: (3) URL using bad/illegal format or missing URL
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:9999...
* Connected to localhost (127.0.0.1) port 9999 (#0)
> POST //api HTTP/1.1
> Host: localhost:9999
> User-Agent: curl/7.80.0
> Accept: */*
> Content-Length: 9
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 308 Permanent Redirect
< Location: /api
< Date: Fri, 17 Dec 2021 13:32:04 GMT
< Content-Length: 0
<
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:9999/api'
* Found bundle for host localhost: 0x7fa0848043e0 [serially]
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (127.0.0.1) port 9999 (#0)
> POST /api HTTP/1.1
> Host: localhost:9999
> User-Agent: curl/7.80.0
> Accept: */*
> Content-Length: 9
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Fri, 17 Dec 2021 13:32:04 GMT
< Content-Length: 27
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact
POST method, body {"foo":1}

see last line here, the server correctly shows POST method and HTTP payload.

What did you see instead?

If we use 301 StatusMovedPermanently the clients POST payload will be lost on a server, while with 308 StatusPermanentRedirect it does not.