Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vkuznet opened this issue Dec 17, 2021 · 0 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@vkuznet
Copy link

vkuznet commented Dec 17, 2021

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:

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.

@vkuznet vkuznet changed the title affected/package: http Lost payload in HTTP POST request via redirect, affected/package: net/http Dec 17, 2021
@seankhliao seankhliao changed the title Lost payload in HTTP POST request via redirect, affected/package: net/http net/http: Lost payload in HTTP POST request via redirect, Dec 17, 2021
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 21, 2021
@seankhliao seankhliao changed the title net/http: Lost payload in HTTP POST request via redirect, net/http: use 308 instead of 301 in Server to preserve POST payload in redirects Feb 8, 2022
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants