Skip to content

Commit 961116a

Browse files
bradfitzbradfitz
committed
http2: support CONNECT requests
Support CONNECT requests in both the server & transport. See https://httpwg.github.io/specs/rfc7540.html#CONNECT When I bundle this into the main Go repo I will also add h1-vs-h2 compatibility tests there, making sure they match behavior. (I now expect that they do match) Updates golang/go#13717 Change-Id: I0c65ad47b029419027efb616fed3d8e0e2a363f4 Reviewed-on: https://go-review.googlesource.com/18266 Reviewed-by: Andrew Gerrand <[email protected]>
1 parent 3b90a77 commit 961116a

File tree

4 files changed

+164
-19
lines changed

4 files changed

+164
-19
lines changed

‎http2/server.go

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,7 +1545,17 @@ func (sc *serverConn) resetPendingRequest() {
15451545
func (sc *serverConn) newWriterAndRequest() (*responseWriter, *http.Request, error) {
15461546
sc.serveG.check()
15471547
rp := &sc.req
1548-
if rp.invalidHeader || rp.method == "" || rp.path == "" ||
1548+
1549+
if rp.invalidHeader {
1550+
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
1551+
}
1552+
1553+
isConnect := rp.method == "CONNECT"
1554+
if isConnect {
1555+
if rp.path != "" || rp.scheme != "" || rp.authority == "" {
1556+
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
1557+
}
1558+
} else if rp.method == "" || rp.path == "" ||
15491559
(rp.scheme != "https" && rp.scheme != "http") {
15501560
// See 8.1.2.6 Malformed Requests and Responses:
15511561
//
@@ -1559,12 +1569,14 @@ func (sc *serverConn) newWriterAndRequest() (*responseWriter, *http.Request, err
15591569
// pseudo-header fields"
15601570
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
15611571
}
1572+
15621573
bodyOpen := rp.stream.state == stateOpen
15631574
if rp.method == "HEAD" && bodyOpen {
15641575
// HEAD requests can't have bodies
15651576
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
15661577
}
15671578
var tlsState *tls.ConnectionState // nil if not scheme https
1579+
15681580
if rp.scheme == "https" {
15691581
tlsState = sc.tlsState
15701582
}
@@ -1605,18 +1617,26 @@ func (sc *serverConn) newWriterAndRequest() (*responseWriter, *http.Request, err
16051617
stream: rp.stream,
16061618
needsContinue: needsContinue,
16071619
}
1608-
// TODO: handle asterisk '*' requests + test
1609-
url, err := url.ParseRequestURI(rp.path)
1610-
if err != nil {
1611-
// TODO: find the right error code?
1612-
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
1620+
var url_ *url.URL
1621+
var requestURI string
1622+
if isConnect {
1623+
url_ = &url.URL{Host: rp.authority}
1624+
requestURI = rp.authority // mimic HTTP/1 server behavior
1625+
} else {
1626+
var err error
1627+
// TODO: handle asterisk '*' requests + test
1628+
url_, err = url.ParseRequestURI(rp.path)
1629+
if err != nil {
1630+
return nil, nil, StreamError{rp.stream.id, ErrCodeProtocol}
1631+
}
1632+
requestURI = rp.path
16131633
}
16141634
req := &http.Request{
16151635
Method: rp.method,
1616-
URL: url,
1636+
URL: url_,
16171637
RemoteAddr: sc.remoteAddrStr,
16181638
Header: rp.header,
1619-
RequestURI: rp.path,
1639+
RequestURI: requestURI,
16201640
Proto: "HTTP/2.0",
16211641
ProtoMajor: 2,
16221642
ProtoMinor: 0,

‎http2/server_test.go

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,60 @@ func testRejectRequest(t *testing.T, send func(*serverTester)) {
901901
st.wantRSTStream(1, ErrCodeProtocol)
902902
}
903903

904+
func TestServer_Request_Connect(t *testing.T) {
905+
testServerRequest(t, func(st *serverTester) {
906+
st.writeHeaders(HeadersFrameParam{
907+
StreamID: 1,
908+
BlockFragment: st.encodeHeaderRaw(
909+
":method", "CONNECT",
910+
":authority", "example.com:123",
911+
),
912+
EndStream: true,
913+
EndHeaders: true,
914+
})
915+
}, func(r *http.Request) {
916+
if g, w := r.Method, "CONNECT"; g != w {
917+
t.Errorf("Method = %q; want %q", g, w)
918+
}
919+
if g, w := r.RequestURI, "example.com:123"; g != w {
920+
t.Errorf("RequestURI = %q; want %q", g, w)
921+
}
922+
if g, w := r.URL.Host, "example.com:123"; g != w {
923+
t.Errorf("URL.Host = %q; want %q", g, w)
924+
}
925+
})
926+
}
927+
928+
func TestServer_Request_Connect_InvalidPath(t *testing.T) {
929+
testServerRejectsStream(t, ErrCodeProtocol, func(st *serverTester) {
930+
st.writeHeaders(HeadersFrameParam{
931+
StreamID: 1,
932+
BlockFragment: st.encodeHeaderRaw(
933+
":method", "CONNECT",
934+
":authority", "example.com:123",
935+
":path", "/bogus",
936+
),
937+
EndStream: true,
938+
EndHeaders: true,
939+
})
940+
})
941+
}
942+
943+
func TestServer_Request_Connect_InvalidScheme(t *testing.T) {
944+
testServerRejectsStream(t, ErrCodeProtocol, func(st *serverTester) {
945+
st.writeHeaders(HeadersFrameParam{
946+
StreamID: 1,
947+
BlockFragment: st.encodeHeaderRaw(
948+
":method", "CONNECT",
949+
":authority", "example.com:123",
950+
":scheme", "https",
951+
),
952+
EndStream: true,
953+
EndHeaders: true,
954+
})
955+
})
956+
}
957+
904958
func TestServer_Ping(t *testing.T) {
905959
st := newServerTester(t, nil)
906960
defer st.Close()
@@ -1222,7 +1276,7 @@ func TestServer_StateTransitions(t *testing.T) {
12221276

12231277
// test HEADERS w/o EndHeaders + another HEADERS (should get rejected)
12241278
func TestServer_Rejects_HeadersNoEnd_Then_Headers(t *testing.T) {
1225-
testServerRejects(t, func(st *serverTester) {
1279+
testServerRejectsConn(t, func(st *serverTester) {
12261280
st.writeHeaders(HeadersFrameParam{
12271281
StreamID: 1,
12281282
BlockFragment: st.encodeHeader(),
@@ -1240,7 +1294,7 @@ func TestServer_Rejects_HeadersNoEnd_Then_Headers(t *testing.T) {
12401294

12411295
// test HEADERS w/o EndHeaders + PING (should get rejected)
12421296
func TestServer_Rejects_HeadersNoEnd_Then_Ping(t *testing.T) {
1243-
testServerRejects(t, func(st *serverTester) {
1297+
testServerRejectsConn(t, func(st *serverTester) {
12441298
st.writeHeaders(HeadersFrameParam{
12451299
StreamID: 1,
12461300
BlockFragment: st.encodeHeader(),
@@ -1255,7 +1309,7 @@ func TestServer_Rejects_HeadersNoEnd_Then_Ping(t *testing.T) {
12551309

12561310
// test HEADERS w/ EndHeaders + a continuation HEADERS (should get rejected)
12571311
func TestServer_Rejects_HeadersEnd_Then_Continuation(t *testing.T) {
1258-
testServerRejects(t, func(st *serverTester) {
1312+
testServerRejectsConn(t, func(st *serverTester) {
12591313
st.writeHeaders(HeadersFrameParam{
12601314
StreamID: 1,
12611315
BlockFragment: st.encodeHeader(),
@@ -1271,7 +1325,7 @@ func TestServer_Rejects_HeadersEnd_Then_Continuation(t *testing.T) {
12711325

12721326
// test HEADERS w/o EndHeaders + a continuation HEADERS on wrong stream ID
12731327
func TestServer_Rejects_HeadersNoEnd_Then_ContinuationWrongStream(t *testing.T) {
1274-
testServerRejects(t, func(st *serverTester) {
1328+
testServerRejectsConn(t, func(st *serverTester) {
12751329
st.writeHeaders(HeadersFrameParam{
12761330
StreamID: 1,
12771331
BlockFragment: st.encodeHeader(),
@@ -1286,7 +1340,7 @@ func TestServer_Rejects_HeadersNoEnd_Then_ContinuationWrongStream(t *testing.T)
12861340

12871341
// No HEADERS on stream 0.
12881342
func TestServer_Rejects_Headers0(t *testing.T) {
1289-
testServerRejects(t, func(st *serverTester) {
1343+
testServerRejectsConn(t, func(st *serverTester) {
12901344
st.fr.AllowIllegalWrites = true
12911345
st.writeHeaders(HeadersFrameParam{
12921346
StreamID: 0,
@@ -1299,7 +1353,7 @@ func TestServer_Rejects_Headers0(t *testing.T) {
12991353

13001354
// No CONTINUATION on stream 0.
13011355
func TestServer_Rejects_Continuation0(t *testing.T) {
1302-
testServerRejects(t, func(st *serverTester) {
1356+
testServerRejectsConn(t, func(st *serverTester) {
13031357
st.fr.AllowIllegalWrites = true
13041358
if err := st.fr.WriteContinuation(0, true, st.encodeHeader()); err != nil {
13051359
t.Fatal(err)
@@ -1308,7 +1362,7 @@ func TestServer_Rejects_Continuation0(t *testing.T) {
13081362
}
13091363

13101364
func TestServer_Rejects_PushPromise(t *testing.T) {
1311-
testServerRejects(t, func(st *serverTester) {
1365+
testServerRejectsConn(t, func(st *serverTester) {
13121366
pp := PushPromiseParam{
13131367
StreamID: 1,
13141368
PromiseID: 3,
@@ -1319,10 +1373,10 @@ func TestServer_Rejects_PushPromise(t *testing.T) {
13191373
})
13201374
}
13211375

1322-
// testServerRejects tests that the server hangs up with a GOAWAY
1376+
// testServerRejectsConn tests that the server hangs up with a GOAWAY
13231377
// frame and a server close after the client does something
13241378
// deserving a CONNECTION_ERROR.
1325-
func testServerRejects(t *testing.T, writeReq func(*serverTester)) {
1379+
func testServerRejectsConn(t *testing.T, writeReq func(*serverTester)) {
13261380
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {})
13271381
st.addLogFilter("connection error: PROTOCOL_ERROR")
13281382
defer st.Close()
@@ -1348,6 +1402,16 @@ func testServerRejects(t *testing.T, writeReq func(*serverTester)) {
13481402
}
13491403
}
13501404

1405+
// testServerRejectsStream tests that the server sends a RST_STREAM with the provided
1406+
// error code after a client sends a bogus request.
1407+
func testServerRejectsStream(t *testing.T, code ErrCode, writeReq func(*serverTester)) {
1408+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {})
1409+
defer st.Close()
1410+
st.greet()
1411+
writeReq(st)
1412+
st.wantRSTStream(1, code)
1413+
}
1414+
13511415
// testServerRequest sets up an idle HTTP/2 connection and lets you
13521416
// write a single request with writeReq, and then verify that the
13531417
// *http.Request is built correctly in checkReq.

‎http2/transport.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,10 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail
775775
// [RFC3986]).
776776
cc.writeHeader(":authority", host) // probably not right for all sites
777777
cc.writeHeader(":method", req.Method)
778-
cc.writeHeader(":path", req.URL.RequestURI())
779-
cc.writeHeader(":scheme", "https")
778+
if req.Method != "CONNECT" {
779+
cc.writeHeader(":path", req.URL.RequestURI())
780+
cc.writeHeader(":scheme", "https")
781+
}
780782
if trailers != "" {
781783
cc.writeHeader("trailer", trailers)
782784
}

‎http2/transport_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,3 +760,62 @@ func TestTransportFullDuplex(t *testing.T) {
760760
t.Fatal(err)
761761
}
762762
}
763+
764+
func TestTransportConnectRequest(t *testing.T) {
765+
gotc := make(chan *http.Request, 1)
766+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
767+
gotc <- r
768+
}, optOnlyServer)
769+
defer st.Close()
770+
771+
u, err := url.Parse(st.ts.URL)
772+
if err != nil {
773+
t.Fatal(err)
774+
}
775+
776+
tr := &Transport{TLSClientConfig: tlsConfigInsecure}
777+
defer tr.CloseIdleConnections()
778+
c := &http.Client{Transport: tr}
779+
780+
tests := []struct {
781+
req *http.Request
782+
want string
783+
}{
784+
{
785+
req: &http.Request{
786+
Method: "CONNECT",
787+
Header: http.Header{},
788+
URL: u,
789+
},
790+
want: u.Host,
791+
},
792+
{
793+
req: &http.Request{
794+
Method: "CONNECT",
795+
Header: http.Header{},
796+
URL: u,
797+
Host: "example.com:123",
798+
},
799+
want: "example.com:123",
800+
},
801+
}
802+
803+
for i, tt := range tests {
804+
res, err := c.Do(tt.req)
805+
if err != nil {
806+
t.Errorf("%d. RoundTrip = %v", i, err)
807+
continue
808+
}
809+
res.Body.Close()
810+
req := <-gotc
811+
if req.Method != "CONNECT" {
812+
t.Errorf("method = %q; want CONNECT", req.Method)
813+
}
814+
if req.Host != tt.want {
815+
t.Errorf("Host = %q; want %q", req.Host, tt.want)
816+
}
817+
if req.URL.Host != tt.want {
818+
t.Errorf("URL.Host = %q; want %q", req.URL.Host, tt.want)
819+
}
820+
}
821+
}

0 commit comments

Comments
 (0)