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

Jump to bottom

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
Closed

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

jefferai opened this issue Oct 28, 2017 · 4 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@jefferai
Copy link

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
@jefferai jefferai mentioned this issue Oct 28, 2017
Closed
@tombergan
Copy link
Contributor

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.

@tombergan tombergan added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 30, 2017
@jefferai
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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!

@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.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jefferai @gopherbot @tombergan