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
Milestone

Comments

@chancez
Copy link

@chancez 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

This comment has been minimized.

Copy link
Contributor

@fraenkel fraenkel commented Mar 27, 2019

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

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Mar 27, 2019

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
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 1, 2019

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

@gopherbot gopherbot closed this in 79f79c3 May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.