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: Request method does not default to GET with http2 #31061

Closed
chancez opened this issue Mar 26, 2019 · 3 comments
Closed

net/http: Request method does not default to GET with http2 #31061

chancez opened this issue Mar 26, 2019 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@chancez
Copy link

chancez commented Mar 26, 2019

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

$ go version
go version go1.11.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/chance/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/chance/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/opt/go/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/97/dvst051s0hqd_f4flg41sc300000gn/T/go-build599780417=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I ran the following program twice, once with HTTP2 disabled and once with it enabled. The program makes a simple request to google.com with the http.Request.Method unset (which should default to GET according to godocs).

https://play.golang.org/p/P89q1NGch2a

I saved the file as request_main.go and ran it once with:

GODEBUG=http2client=1 go run request_main.go

and once with:

GODEBUG=http2client=0 go run request_main.go

What did you expect to see?

With HTTP2 and HTTP1 I expect it to succeed.

What did you see instead?

It only succeeds for HTTP1, returning 200:

&{200 OK 200 HTTP/1.1 1 1 map[Set-Cookie:[1P_JAR=2019-03-26-19; expires=Thu, 25-Apr-2019 19:17:24 GMT; path=/; domain=.google.com NID=168=M_CvFU3r1oM7lmS_GNK6nJ2fpnvSC9yIDhxPcZVBC26mhR3Tu5AU-Fp__zZij0nM6SO1g5Pxj3HPUvBUXOA-kH2mYbpmn-SW4rg--Oa84dZ01K3VCZdxTwUaZGGbtSyJoaTLlLGUV67RSH7ryxJaKFKz3M92piybvv-u01B-_FA; expires=Wed, 25-Sep-2019 19:17:24 GMT; path=/; domain=.google.com; HttpOnly] Date:[Tue, 26 Mar 2019 19:17:24 GMT] Cache-Control:[private, max-age=0] Content-Type:[text/html; charset=ISO-8859-1] Server:[gws] X-Xss-Protection:[1; mode=block] X-Frame-Options:[SAMEORIGIN] Alt-Svc:[quic=":443"; ma=2592000; v="46,44,43,39"] Expires:[-1] P3p:[CP="This is not a P3P policy! See g.co/p3phelp for more info."]] 0xc4201be1e0 -1 [chunked] false true map[] 0xc420478100 0xc4200d46e0} <nil>

For HTTP2 it fails with 405 method not allowed:

&{405 Method Not Allowed 405 HTTP/2.0 2 0 map[Content-Type:[text/html; charset=UTF-8] Referrer-Policy:[no-referrer] Content-Length:[1585] Date:[Tue, 26 Mar 2019 19:16:41 GMT] Alt-Svc:[quic=":443"; ma=2592000; v="46,44,43,39"]] {0xc42044fa40} 1585 [] false false map[] 0xc42014a100 0xc42036ac60} <nil>

But, in other cases with a Go HTTP2 server (Kubernetes, with a less minimal example) returned a protocol error when receiving a similar request ( I have no example for this, but it's likely that the HTTP2 server needs to check for the request method being set, and return 405 instead of a protocol error ).

When I used mitmproxy and wireshark I was able to see the HTTP method header value is indeed, unset when http.Request.Method is unset when the client is using HTTP2. In the godoc, it states For client requests, an empty string means GET when referring to the Request type:

type Request struct {
// Method specifies the HTTP method (GET, POST, PUT, etc.).
// For client requests, an empty string means GET.
//
// Go's HTTP client does not support sending a request with
// the CONNECT method. See the documentation on Transport for
// details.
Method string

However, based on my testing, this is not happening for http2.

@fraenkel
Copy link
Contributor

@chancez You are correct. There is a missing conversion from "" to "GET" when building the headers for http/2.

@gopherbot
Copy link

Change https://golang.org/cl/169557 mentions this issue: http2: make empty method mean GET

gopherbot pushed a commit to golang/net that referenced this issue Mar 27, 2019
We document that "" means "GET" for Request.Method.

Updates golang/go#31061

Change-Id: I41d0c7361e6ad14e9c04c120aed8a30295b1f974
Reviewed-on: https://go-review.googlesource.com/c/net/+/169557
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@julieqiu julieqiu changed the title http.Request method does not default to GET with http2 net/http: Request method does not default to GET with http2 Apr 23, 2019
@julieqiu julieqiu added this to the Go1.13 milestone Apr 23, 2019
@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 23, 2019
@gopherbot
Copy link

Change https://golang.org/cl/174677 mentions this issue: net/http: update bundled x/net/http2

@golang golang locked and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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

4 participants