Skip to content

Commit

Permalink
Remove TLSNextProto in Transport
Browse files Browse the repository at this point in the history
  • Loading branch information
@imroc
imroc committed Jul 5, 2022
1 parent bf92ee0 commit 5dcc414
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 231 deletions.
5 changes: 0 additions & 5 deletions internal/transport/transport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"crypto/tls"
"github.com/imroc/req/v3/internal/dump"
reqtls "github.com/imroc/req/v3/pkg/tls"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -91,8 +90,4 @@ type Interface interface {
// when reading from the transport.
// If zero, a default (currently 4KB) is used.
ReadBufferSize() int

TLSNextProto() map[string]func(authority string, c reqtls.Conn) http.RoundTripper

SetTLSNextProto(map[string]func(authority string, c reqtls.Conn) http.RoundTripper)
}
19 changes: 0 additions & 19 deletions transport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,18 +225,6 @@ type Transport struct {
// This time does not include the time to send the request header.
ExpectContinueTimeout time.Duration

// TLSNextProto specifies how the Transport switches to an
// alternate protocol (such as HTTP/2) after a TLS ALPN
// protocol negotiation. If Transport dials an TLS connection
// with a non-empty protocol name and TLSNextProto contains a
// map entry for that key (such as "h2"), then the func is
// called with the request's authority (such as "example.com"
// or "example.com:1234") and the TLS connection. The function
// must return a http.RoundTripper that then handles the request.
// If TLSNextProto is not nil, HTTP/2 support is not enabled
// automatically.
TLSNextProto map[string]func(authority string, c reqtls.Conn) http.RoundTripper

// ProxyConnectHeader optionally specifies headers to send to
// proxies during CONNECT requests.
// To set the header dynamically, see GetProxyConnectHeader.
Expand Down Expand Up @@ -491,13 +479,6 @@ func (t *Transport) Clone() *Transport {
if t.TLSClientConfig != nil {
t2.TLSClientConfig = t.TLSClientConfig.Clone()
}
if t.TLSNextProto != nil {
npm := map[string]func(authority string, c reqtls.Conn) http.RoundTripper{}
for k, v := range t.TLSNextProto {
npm[k] = v
}
t2.TLSNextProto = npm
}
return t2
}

Expand Down
77 changes: 0 additions & 77 deletions transport_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,10 @@
package req

import (
"bytes"
"context"
"crypto/tls"
"errors"
"github.com/imroc/req/v3/internal/http2"
"github.com/imroc/req/v3/internal/testcert"
"github.com/imroc/req/v3/internal/tests"
reqtls "github.com/imroc/req/v3/pkg/tls"
"io"
"net"
"net/http"
"strings"
Expand Down Expand Up @@ -191,75 +186,3 @@ type roundTripFunc func(r *http.Request) (*http.Response, error)
func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) {
return f(r)
}

// Issue 25009
func TestTransportBodyAltRewind(t *testing.T) {
cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey)
if err != nil {
t.Fatal(err)
}
ln := tests.NewLocalListener(t)
defer ln.Close()

go func() {
tln := tls.NewListener(ln, &tls.Config{
NextProtos: []string{"foo"},
Certificates: []tls.Certificate{cert},
})
for i := 0; i < 2; i++ {
sc, err := tln.Accept()
if err != nil {
t.Error(err)
return
}
if err := sc.(reqtls.Conn).Handshake(); err != nil {
t.Error(err)
return
}
sc.Close()
}
}()

addr := ln.Addr().String()
req, _ := http.NewRequest("POST", "https://example.org/", bytes.NewBufferString("request"))
roundTripped := false
tr := &Transport{
DisableKeepAlives: true,
TLSNextProto: map[string]func(string, reqtls.Conn) http.RoundTripper{
"foo": func(authority string, c reqtls.Conn) http.RoundTripper {
return roundTripFunc(func(r *http.Request) (*http.Response, error) {
n, _ := io.Copy(io.Discard, r.Body)
if n == 0 {
t.Error("body length is zero")
}
if roundTripped {
return &http.Response{
Body: NoBody,
StatusCode: 200,
}, nil
}
roundTripped = true
return nil, http2.ErrNoCachedConn
})
},
},
DialTLSContext: func(_ context.Context, _, _ string) (net.Conn, error) {
tc, err := tls.Dial("tcp", addr, &tls.Config{
InsecureSkipVerify: true,
NextProtos: []string{"foo"},
})
if err != nil {
return nil, err
}
if err := tc.Handshake(); err != nil {
return nil, err
}
return tc, nil
},
}
c := &http.Client{Transport: tr}
_, err = c.Do(req)
if err != nil {
t.Error(err)
}
}
126 changes: 5 additions & 121 deletions transport_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ import (
"fmt"
"github.com/imroc/req/v3/internal/common"
"github.com/imroc/req/v3/internal/http2"
"github.com/imroc/req/v3/internal/testcert"
"github.com/imroc/req/v3/internal/tests"
reqtls "github.com/imroc/req/v3/pkg/tls"
"go/token"
"golang.org/x/net/http/httpproxy"
nethttp2 "golang.org/x/net/http2"
Expand Down Expand Up @@ -4214,27 +4212,6 @@ func TestTransportPrefersResponseOverWriteError(t *testing.T) {
}
}

func TestTransportAutomaticHTTP2(t *testing.T) {
tr := tc().t
testTransportAutoHTTP(t, tr, true)
}

func TestTransportAutomaticHTTP2_TLSNextProto(t *testing.T) {
tr := tc().t
tr.TLSNextProto = make(map[string]func(string, reqtls.Conn) http.RoundTripper)
testTransportAutoHTTP(t, tr, false)
}

func testTransportAutoHTTP(t *testing.T, tr *Transport, wantH2 bool) {
_, err := tr.RoundTrip(new(http.Request))
if err == nil {
t.Error("expected error from RoundTrip")
}
if reg := tr.TLSNextProto["h2"] != nil; reg != wantH2 {
t.Errorf("HTTP/2 registered = %v; want %v", reg, wantH2)
}
}

// Issue 13633: there was a race where we returned bodyless responses
// to callers before recycling the persistent connection, which meant
// a client doing two subsequent requests could end up on different
Expand Down Expand Up @@ -4269,89 +4246,6 @@ func TestTransportReuseConnEmptyResponseBody(t *testing.T) {
}
}

// Issue 13839
func TestNoCrashReturningTransportAltConn(t *testing.T) {
cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey)
if err != nil {
t.Fatal(err)
}
ln := tests.NewLocalListener(t)
defer ln.Close()

var wg sync.WaitGroup
SetPendingDialHooks(func() { wg.Add(1) }, wg.Done)
defer SetPendingDialHooks(nil, nil)

testDone := make(chan struct{})
defer close(testDone)
go func() {
tln := tls.NewListener(ln, &tls.Config{
NextProtos: []string{"foo"},
Certificates: []tls.Certificate{cert},
})
sc, err := tln.Accept()
if err != nil {
t.Error(err)
return
}
if err := sc.(*tls.Conn).Handshake(); err != nil {
t.Error(err)
return
}
<-testDone
sc.Close()
}()

addr := ln.Addr().String()

req, _ := http.NewRequest("GET", "https://fake.tld/", nil)
cancel := make(chan struct{})
req.Cancel = cancel

doReturned := make(chan bool, 1)
madeRoundTripper := make(chan bool, 1)

tr := &Transport{
DisableKeepAlives: true,
TLSNextProto: map[string]func(string, reqtls.Conn) http.RoundTripper{
"foo": func(authority string, c reqtls.Conn) http.RoundTripper {
madeRoundTripper <- true
return funcRoundTripper(func() {
t.Error("foo http.RoundTripper should not be called")
})
},
},
DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
panic("shouldn't be called")
},
DialTLSContext: func(_ context.Context, _, _ string) (net.Conn, error) {
tc, err := tls.Dial("tcp", addr, &tls.Config{
InsecureSkipVerify: true,
NextProtos: []string{"foo"},
})
if err != nil {
return nil, err
}
if err := tc.Handshake(); err != nil {
return nil, err
}
close(cancel)
<-doReturned
return tc, nil
},
}
c := &http.Client{Transport: tr}

_, err = c.Do(req)
if ue, ok := err.(*url.Error); !ok || ue.Err != errRequestCanceledConn {
t.Fatalf("Do error = %v; want url.Error with errRequestCanceledConn", err)
}

doReturned <- true
<-madeRoundTripper
wg.Wait()
}

func TestTransportReuseConnectionGzipChunked(t *testing.T) {
testTransportReuseConnectionGzip(t, true)
}
Expand Down Expand Up @@ -5600,14 +5494,11 @@ func TestTransportClone(t *testing.T) {
GetProxyConnectHeader: func(context.Context, *url.URL, string) (http.Header, error) { return nil, nil },
MaxResponseHeaderBytes: 1,
ForceAttemptHTTP2: true,
TLSNextProto: map[string]func(authority string, c reqtls.Conn) http.RoundTripper{
"foo": func(authority string, c reqtls.Conn) http.RoundTripper { panic("") },
},
ReadBufferSize: 1,
WriteBufferSize: 1,
ForceHttpVersion: HTTP1,
ResponseOptions: &ResponseOptions{},
Debugf: func(format string, v ...interface{}) {},
ReadBufferSize: 1,
WriteBufferSize: 1,
ForceHttpVersion: HTTP1,
ResponseOptions: &ResponseOptions{},
Debugf: func(format string, v ...interface{}) {},
}
tr2 := tr.Clone()
rv := reflect.ValueOf(tr2).Elem()
Expand All @@ -5622,16 +5513,9 @@ func TestTransportClone(t *testing.T) {
}
}

if _, ok := tr2.TLSNextProto["foo"]; !ok {
t.Errorf("cloned Transport lacked TLSNextProto 'foo' key")
}

// But test that a nil TLSNextProto is kept nil:
tr = new(Transport)
tr2 = tr.Clone()
if tr2.TLSNextProto != nil {
t.Errorf("Transport.TLSNextProto unexpected non-nil")
}
}

func TestIs408(t *testing.T) {
Expand Down
9 changes: 0 additions & 9 deletions transport_wrapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"crypto/tls"
"github.com/imroc/req/v3/internal/dump"
reqtls "github.com/imroc/req/v3/pkg/tls"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -102,11 +101,3 @@ func (t transportImpl) WriteBufferSize() int {
func (t transportImpl) ReadBufferSize() int {
return t.t.ReadBufferSize
}

func (t transportImpl) TLSNextProto() map[string]func(authority string, c reqtls.Conn) http.RoundTripper {
return t.t.TLSNextProto
}

func (t transportImpl) SetTLSNextProto(m map[string]func(authority string, c reqtls.Conn) http.RoundTripper) {
t.t.TLSNextProto = m
}

0 comments on commit 5dcc414

Please sign in to comment.