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

x/net/http2: Incompatibility between library and built-in net/http transport ("bogus greeting") #22481

Closed
jefferai opened this issue Oct 28, 2017 · 4 comments

Comments

@jefferai
Copy link

@jefferai jefferai commented Oct 28, 2017

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

1.9.2

Does this issue reproduce with the latest release?

Yes (1.9.2 is currently the latest.)

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jeff/go"
GORACE=""
GOROOT="/home/jeff/src/go"
GOTOOLDIR="/home/jeff/src/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build323001895=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Created an http.Server and called http2.ConfigureServer on it with nil configuration.

What did you expect to see?

Working behavior regardless of whether an http.Transport is using the Go net/http package's built-in H2 support or the http2 package's support.

What did you see instead?

When an http.Client is used with an http.Transport using the built-in net/http H2 support, a bogus greeting message:

2017/10/27 16:44:08 http2: server: error reading preface from client 127.0.0.1:51101: bogus greeting "GET /v1/auth/token/looku"
--- FAIL: TestServer (2.59s)
        sandbox_test.go:46: Get https://127.0.0.1:51096/v1/auth/token/lookup-self: net/http: HTTP/1.x transport connection broken: malformed HTTP response "\x00\x00\x18\x04\x00\x00\x00\x00\x00\x00\x05\x00\x10\x00\x00\x00\x03\x00\x00\x00\xfa\x00\x06\x00\x10\x01@\x00\x04\x00\x10\x00\x00"
FAIL

If the http.Transport has http2.ConfigureTransport called on it, everything works normally.

This appears to have started happening somewhere between September 5th and October 27th, since I first started seeing this behavior after a vendored dependency update. Before then, the transport worked fine regardless of whether it was using http2.ConfigureTransport or not.

@jefferai jefferai changed the title x/net/http2: Incompatibility between library and Go 1.9 ("bogus greeting") x/net/http2: Incompatibility between library and built-in net/http transport ("bogus greeting") Oct 28, 2017
@tombergan

This comment has been minimized.

Copy link
Contributor

@tombergan tombergan commented Oct 30, 2017

Can you post a complete repro with code? It's possible that you're not using TLS. Currently, net/http's H2 support requires TLS.

@jefferai

This comment has been minimized.

Copy link
Author

@jefferai jefferai commented Oct 30, 2017

I don't have a minimal repro yet but can try to work one up (if you're willing to source from some external packages I can make one pretty trivially).

However, TLS is always used.

@tombergan

This comment has been minimized.

Copy link
Contributor

@tombergan tombergan commented Oct 31, 2017

This works just fine for me using a self-signed cert:

package main

import (
    "crypto/tls"
    "fmt"
    "net/http"

    "golang.org/x/net/http2"
)

func main() {
    http.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
        fmt.Printf("Got request %v\n", req.URL)
        rw.WriteHeader(204)
    })

    go func() {
        var srv http.Server
        srv.Addr = "localhost:8999"
        http2.ConfigureServer(&srv, nil)
        srv.ListenAndServeTLS(cert, key)
    }()

    t := http.DefaultTransport
    t.(*http.Transport).TLSClientConfig = &tls.Config{
        InsecureSkipVerify: true,
    }
    req, err := http.NewRequest("GET", "https://localhost:8999/foo", nil)
    if err != nil {
        fmt.Printf("NewRequest: %v\n", err)
        return
    }
    resp, err := t.RoundTrip(req)
    if err != nil {
        fmt.Printf("RoundTrip: %v\n", err)
        return
    }
    fmt.Printf("Got response %v\n", resp.StatusCode)
}

Tested with go 1.9.1 and HEAD. I haven't tried go 1.9.2, but I'm not aware of any HTTP changes in 1.9.2. Does the above program work for you?

@jefferai

This comment has been minimized.

Copy link
Author

@jefferai jefferai commented Oct 31, 2017

After a bunch more testing and digging in, I think what has happened here is that due to some unrelated changes we ended up in a state where http2.ConfigureTransport was no longer being called but the TLS config for the connection was specifying a NextProtos of ["h2", "http/1.1"], leading to a situation where the underlying transport would advertise H2 support but not have an upgrade handler configured.

Sorry for the noise and thanks for helping!

@jefferai jefferai closed this Oct 31, 2017
@golang golang locked and limited conversation to collaborators Oct 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.