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/crypto/acme/autocert: NewListener does not enable HTTP2. #20572

Closed
johanbrandhorst opened this issue Jun 4, 2017 · 7 comments
Closed

x/crypto/acme/autocert: NewListener does not enable HTTP2. #20572

johanbrandhorst opened this issue Jun 4, 2017 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@johanbrandhorst
Copy link
Member

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

go version go1.8.3 linux/amd64

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

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

What did you do?

I deployed a server using autocert.NewListener to acquire a certificate. I was surprised to see the server did not have HTTP2 enabled.

What did you expect to see?

I expected the server to have HTTP2 enabled.

What did you see instead?

The server did not have HTTP2 enabled.

There's a fairly simple fix for this, simply adding

NextProtos:     []string{"h2"},

after line 72 in listener.go enables HTTP2 support on the server.

@gopherbot gopherbot added this to the Unreleased milestone Jun 4, 2017
@johanbrandhorst
Copy link
Member Author

If this is indeed considered a bug worth fixing, I would like to fix it.

@johanbrandhorst johanbrandhorst changed the title x/crypto/acme/autocert: Listener should set NextProtos: []string{"h2"} to enable HTTP2. x/crypto/acme/autocert: NewListener does not enable HTTP2. Jun 4, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Jun 5, 2017

While that would work for you, I wonder if it would break

(1) anybody not expecting HTTP/2 (people not using net/http?) or whether

(2) there are net/http paths where the http2.ConfigureServer path isn't invoked. I worry about the case where the client thinks it can do h2 but the server isn't wired up to accept h2.

For (1), we can just document it. It's new enough and I imagine everybody is using net/http for it. And if you're running, say, an SMTP server, the client won't be trying to do h2.

For (2), I'd just want to audit the various paths again. I forget how the auto-http2.ConfigureServer code works.

But yeah, no real objections.

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 5, 2017
@johanbrandhorst
Copy link
Member Author

I will work on this

@johanbrandhorst
Copy link
Member Author

johanbrandhorst commented Jun 5, 2017

@bradfitz Just to clarify what needs to be done for (2), would you like me to audit all net/http functions accepting net.Listener to verify that they all call to http2ConfigureServer at some point?

@bradfitz
Copy link
Contributor

bradfitz commented Jun 5, 2017

Yes.

@johanbrandhorst
Copy link
Member Author

Serve in net/http/fcgi accepts a net.Listener but does not appear to call to http2ConfigureServer, is this a case we should be concerned about? All other exported functions in net/http and subpackages call to http2ConfigureServer.

@gopherbot
Copy link

CL https://golang.org/cl/45630 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 13, 2018
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
Enables HTTP/2 on any servers used with the autocert listener
by setting "h2" in NextProtos of the listener *tls.Config.
Also adds a warning to the listener documentation that it
enables HTTP/2.

Fixes golang/go#20572

Change-Id: If7c0f5722f0b1781789219fc4e84da3f19a89ab7
Reviewed-on: https://go-review.googlesource.com/45630
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Enables HTTP/2 on any servers used with the autocert listener
by setting "h2" in NextProtos of the listener *tls.Config.
Also adds a warning to the listener documentation that it
enables HTTP/2.

Fixes golang/go#20572

Change-Id: If7c0f5722f0b1781789219fc4e84da3f19a89ab7
Reviewed-on: https://go-review.googlesource.com/45630
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
Enables HTTP/2 on any servers used with the autocert listener
by setting "h2" in NextProtos of the listener *tls.Config.
Also adds a warning to the listener documentation that it
enables HTTP/2.

Fixes golang/go#20572

Change-Id: If7c0f5722f0b1781789219fc4e84da3f19a89ab7
Reviewed-on: https://go-review.googlesource.com/45630
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
Enables HTTP/2 on any servers used with the autocert listener
by setting "h2" in NextProtos of the listener *tls.Config.
Also adds a warning to the listener documentation that it
enables HTTP/2.

Fixes golang/go#20572

Change-Id: If7c0f5722f0b1781789219fc4e84da3f19a89ab7
Reviewed-on: https://go-review.googlesource.com/45630
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants