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: Transport.Clone breaks HTTP/2 #39302

Open
rittneje opened this issue May 28, 2020 · 8 comments
Open

net/http: Transport.Clone breaks HTTP/2 #39302

rittneje opened this issue May 28, 2020 · 8 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rittneje
Copy link

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

$ go version
go version go1.13.10 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/tmp/.gocache"
GOENV="/Users/jrittner/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jrittner/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/jrittner/go1.13.10"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/jrittner/go1.13.10/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qz/jbyzqww512dfd00_vfwk74l4zy5gvy/T/go-build718180347=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I set up an http.Transport with a non-nil, empty map for TLSNextProto in order to disable HTTP/2, as per the documentation.

https://golang.org/pkg/net/http/

Programs that must disable HTTP/2 can do so by setting Transport.TLSNextProto (for clients) or Server.TLSNextProto (for servers) to a non-nil, empty map.

 transport := http.DefaultTransport.(*http.Transport).Clone()
 transport.TLSNextProto = make(map[string]func(authority string, c *tls.Conn) http.RoundTripper)
 httpClient := &http.Client{
   Transport: transport,
 }
 resp, err := httpClient.Get("https://google.com")
 if err != nil {
   panic(err)
 }
 defer resp.Body.Close()
 fmt.Println("Response status:", resp.Status)

What did you expect to see?

I expected it to do HTTP/1.1 over TLS.

What did you see instead?

panic: Get https://google.com: net/http: HTTP/1.x transport connection broken: malformed HTTP response "\x00\x00\x12\x04\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00d\x00\x04\x00\x10\x00\x00\x00\x06\x00\x00@\x00\x00\x00\x04\b\x00\x00\x00\x00\x00\x00\x0f\x00\x01\x00\x00\x1e\a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01http2_handshake_failed"
@andybons
Copy link
Member

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2020
@andybons andybons added this to the Unplanned milestone May 28, 2020
@odeke-em
Copy link
Member

Thank you for this report @rittneje!

It does work as advertised, except that you are using a http.DefaultTransport.Clone() which clones only the exported fields of that Transport and yet http.DefaultTransport is initialized with some internal variables. The problem you are encountering is an inconsistency in what's expected internally and the TLS handling function is returned as nil which causes that error.

You can ensure that this works by creating a fresh transport

package main

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

func main() {
	httpClient := &http.Client{
		Transport: &http.Transport{
			TLSNextProto: make(map[string]func(authority string, c *tls.Conn) http.RoundTripper),
		},
	}
	resp, err := httpClient.Get("https://google.com/")
	if err != nil {
		panic(err)
	}
	defer resp.Body.Close()
	fmt.Println("Response status:", resp.Status)
}

or alternatively if you want to entirely disable HTTP/2, you can set in your environment

GODEBUG=http2client=0

I am going to send a CL showing clear examples for what I've talked about and that'll perhaps serve as a reference for disabling HTTP/2.

@gopherbot
Copy link

Change https://golang.org/cl/235523 mentions this issue: net/http: add examples for disabling HTTP/2 in Transport

@odeke-em
Copy link
Member

odeke-em commented May 29, 2020

I forgot to mention, that if you also want to use a cloned http.DefaultTransport with HTTP/1, you'll need to ensure:
a) ForceAttemptHTTP2 = false
b) TLSClientConfig = new(tls.Config)

package main

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

func main() {
	tr := http.DefaultTransport.(*http.Transport).Clone()
	tr.ForceAttemptHTTP2 = false
	tr.TLSNextProto = make(map[string]func(authority string, c *tls.Conn) http.RoundTripper)
	tr.TLSClientConfig = &tls.Config{}
	httpClient := &http.Client{
		Transport: tr,
	}
	resp, err := httpClient.Get("https://google.com/")
	if err != nil {
		panic(err)
	}
	defer resp.Body.Close()
	fmt.Println("Response status:", resp.Status)
}

which will directly reflect in your programs.

@sugymt
Copy link

sugymt commented Dec 4, 2022

Hope it helps anyone having this issue with go 1.19.

If you first set ForceAttemptHTTP2 to false in the shallow copy, TLSConfig will be nil.

func main() {
	htShallow := http.DefaultTransport.(*http.Transport)
	htShallow.ForceAttemptHTTP2 = false
	fmt.Printf("shallow: %+v\n", htShallow.TLSClientConfig)

	htClone := http.DefaultTransport.(*http.Transport).Clone()
	fmt.Printf("clone:   %+v\n", htClone.TLSClientConfig)
}

// shallow: <nil>
// clone:   <nil>

If you first set ForceAttemptHTTP2 to false in cloned default transport, both TLSConfig will be initialized.
At this time, commenting out htClone.ForceAttemptHTTP2 = false does not change the result.
Interesting is NextProtos:[h2 http/1.1].

func main() {
	htClone := http.DefaultTransport.(*http.Transport).Clone()
	htClone.ForceAttemptHTTP2 = false
	fmt.Printf("clone:   %+v\n", htClone.TLSClientConfig)

	htShallow := http.DefaultTransport.(*http.Transport)
	fmt.Printf("shallow: %+v\n", htShallow.TLSClientConfig)
}

// clone:   &{Rand:<nil> Time:<nil> Certificates:[] NameToCertificate:map[] GetCertificate:<nil> GetClientCertificate:<nil> GetConfigForClient:<nil> VerifyPeerCertificate:<nil> VerifyConnection:<nil> RootCAs:<nil> NextProtos:[h2 http/1.1] ServerName: ClientAuth:NoClientCert ClientCAs:<nil> InsecureSkipVerify:false CipherSuites:[] PreferServerCipherSuites:false SessionTicketsDisabled:false SessionTicketKey:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] ClientSessionCache:<nil> MinVersion:0 MaxVersion:0 CurvePreferences:[] DynamicRecordSizingDisabled:false Renegotiation:0 KeyLogWriter:<nil> mutex:{w:{state:0 sema:0} writerSem:0 readerSem:0 readerCount:0 readerWait:0} sessionTicketKeys:[] autoSessionTicketKeys:[]}
// shallow: &{Rand:<nil> Time:<nil> Certificates:[] NameToCertificate:map[] GetCertificate:<nil> GetClientCertificate:<nil> GetConfigForClient:<nil> VerifyPeerCertificate:<nil> VerifyConnection:<nil> RootCAs:<nil> NextProtos:[h2 http/1.1] ServerName: ClientAuth:NoClientCert ClientCAs:<nil> InsecureSkipVerify:false CipherSuites:[] PreferServerCipherSuites:false SessionTicketsDisabled:false SessionTicketKey:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] ClientSessionCache:<nil> MinVersion:0 MaxVersion:0 CurvePreferences:[] DynamicRecordSizingDisabled:false Renegotiation:0 KeyLogWriter:<nil> mutex:{w:{state:0 sema:0} writerSem:0 readerSem:0 readerCount:0 readerWait:0} sessionTicketKeys:[] autoSessionTicketKeys:[]}

If you use "ForceAttemptHTTP2 is false and NextProtos contains h2" Transport, the request to the server that ALPN responds with h2 will result in an error.
Commenting out htClone.ForceAttemptHTTP2 = false will be ok.

func main() {
        htClone := http.DefaultTransport.(*http.Transport).Clone()
        htClone.ForceAttemptHTTP2 = false

        client := &http.Client{Transport: htClone}
        req, _ := http.NewRequest("GET", "https://google.com", nil)
        if _, err := client.Do(req); err != nil {
                fmt.Println(err)
        } else {
                fmt.Println("ok")
        }
}

// Get "https://google.com": net/http: HTTP/1.x transport connection broken: malformed HTTP response "\x00\x00\x12\x04\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00d\x00\x04\x00\x10\x00\x00\x00\x06\x00\x01\x00\x00\x00\x00\x04\b\x00\x00\x00\x00\x00\x00\x0f\x00\x01\x00\x00\x1e\a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01http2_handshake_failed"

My choice is to override NextProtos to http/1.1 only.

func main() {
        htClone := http.DefaultTransport.(*http.Transport).Clone()
        htClone.ForceAttemptHTTP2 = false
        htClone.TLSClientConfig.NextProtos = []string{"http/1.1"}

        client := &http.Client{Transport: htClone}
        req, _ := http.NewRequest("GET", "https://google.com", nil)
        if _, err := client.Do(req); err != nil {
                fmt.Println(err)
        } else {
                fmt.Println("ok")
        }
}

// ok

I made a request to the https server below and confirmed that the Proto is http/1.1.

func main() {
        http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
                fmt.Printf("%s\n", r.Proto)
        })
        http.ListenAndServeTLS(":3000", "./cert.pem", "./cert.key", nil)
}

It's hard for me to explain why this behavior happens around DefaultTransport, but I find it confusing.
I hope the documentation is better or #39299 is implemented.

@scudette

This comment was marked as off-topic.

@rittneje

This comment was marked as off-topic.

@andreasschulze

This comment was marked as off-topic.

@seankhliao seankhliao changed the title net/http: disabling HTTP/2 as per documentation does not work net/http: Transport.Clone breaks HTTP/2 May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 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

7 participants