Skip to content

Commit 8844327

Browse files
committed
internal/httpcommon: don't depend on net/http
When the http2 package is bundled into net/http, it imports httpcommon, so httpcommon must not depend on net/http. Change-Id: I2aa34e913a0df757fa83deb56f650394a924933e Reviewed-on: https://go-review.googlesource.com/c/net/+/649415 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
1 parent cd9d661 commit 8844327

File tree

7 files changed

+271
-174
lines changed

7 files changed

+271
-174
lines changed

http2/transport.go

+33-9
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,19 @@ func (cc *ClientConn) responseHeaderTimeout() time.Duration {
12861286
return 0
12871287
}
12881288

1289+
// actualContentLength returns a sanitized version of
1290+
// req.ContentLength, where 0 actually means zero (not unknown) and -1
1291+
// means unknown.
1292+
func actualContentLength(req *http.Request) int64 {
1293+
if req.Body == nil || req.Body == http.NoBody {
1294+
return 0
1295+
}
1296+
if req.ContentLength != 0 {
1297+
return req.ContentLength
1298+
}
1299+
return -1
1300+
}
1301+
12891302
func (cc *ClientConn) decrStreamReservations() {
12901303
cc.mu.Lock()
12911304
defer cc.mu.Unlock()
@@ -1310,15 +1323,15 @@ func (cc *ClientConn) roundTrip(req *http.Request, streamf func(*clientStream))
13101323
reqCancel: req.Cancel,
13111324
isHead: req.Method == "HEAD",
13121325
reqBody: req.Body,
1313-
reqBodyContentLength: httpcommon.ActualContentLength(req),
1326+
reqBodyContentLength: actualContentLength(req),
13141327
trace: httptrace.ContextClientTrace(ctx),
13151328
peerClosed: make(chan struct{}),
13161329
abort: make(chan struct{}),
13171330
respHeaderRecv: make(chan struct{}),
13181331
donec: make(chan struct{}),
13191332
}
13201333

1321-
cs.requestedGzip = httpcommon.IsRequestGzip(req, cc.t.disableCompression())
1334+
cs.requestedGzip = httpcommon.IsRequestGzip(req.Method, req.Header, cc.t.disableCompression())
13221335

13231336
go cs.doRequest(req, streamf)
13241337

@@ -1349,7 +1362,7 @@ func (cc *ClientConn) roundTrip(req *http.Request, streamf func(*clientStream))
13491362
}
13501363
res.Request = req
13511364
res.TLS = cc.tlsState
1352-
if res.Body == noBody && httpcommon.ActualContentLength(req) == 0 {
1365+
if res.Body == noBody && actualContentLength(req) == 0 {
13531366
// If there isn't a request or response body still being
13541367
// written, then wait for the stream to be closed before
13551368
// RoundTrip returns.
@@ -1596,12 +1609,7 @@ func (cs *clientStream) encodeAndWriteHeaders(req *http.Request) error {
15961609
// sent by writeRequestBody below, along with any Trailers,
15971610
// again in form HEADERS{1}, CONTINUATION{0,})
15981611
cc.hbuf.Reset()
1599-
res, err := httpcommon.EncodeHeaders(httpcommon.EncodeHeadersParam{
1600-
Request: req,
1601-
AddGzipHeader: cs.requestedGzip,
1602-
PeerMaxHeaderListSize: cc.peerMaxHeaderListSize,
1603-
DefaultUserAgent: defaultUserAgent,
1604-
}, func(name, value string) {
1612+
res, err := encodeRequestHeaders(req, cs.requestedGzip, cc.peerMaxHeaderListSize, func(name, value string) {
16051613
cc.writeHeader(name, value)
16061614
})
16071615
if err != nil {
@@ -1617,6 +1625,22 @@ func (cs *clientStream) encodeAndWriteHeaders(req *http.Request) error {
16171625
return err
16181626
}
16191627

1628+
func encodeRequestHeaders(req *http.Request, addGzipHeader bool, peerMaxHeaderListSize uint64, headerf func(name, value string)) (httpcommon.EncodeHeadersResult, error) {
1629+
return httpcommon.EncodeHeaders(req.Context(), httpcommon.EncodeHeadersParam{
1630+
Request: httpcommon.Request{
1631+
Header: req.Header,
1632+
Trailer: req.Trailer,
1633+
URL: req.URL,
1634+
Host: req.Host,
1635+
Method: req.Method,
1636+
ActualContentLength: actualContentLength(req),
1637+
},
1638+
AddGzipHeader: addGzipHeader,
1639+
PeerMaxHeaderListSize: peerMaxHeaderListSize,
1640+
DefaultUserAgent: defaultUserAgent,
1641+
}, headerf)
1642+
}
1643+
16201644
// cleanupWriteRequest performs post-request tasks.
16211645
//
16221646
// If err (the result of writeRequest) is non-nil and the stream is not closed,

http2/transport_test.go

+46-12
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"time"
3737

3838
"golang.org/x/net/http2/hpack"
39-
"golang.org/x/net/internal/httpcommon"
4039
)
4140

4241
var (
@@ -571,6 +570,45 @@ func randString(n int) string {
571570
return string(b)
572571
}
573572

573+
type panicReader struct{}
574+
575+
func (panicReader) Read([]byte) (int, error) { panic("unexpected Read") }
576+
func (panicReader) Close() error { panic("unexpected Close") }
577+
578+
func TestActualContentLength(t *testing.T) {
579+
tests := []struct {
580+
req *http.Request
581+
want int64
582+
}{
583+
// Verify we don't read from Body:
584+
0: {
585+
req: &http.Request{Body: panicReader{}},
586+
want: -1,
587+
},
588+
// nil Body means 0, regardless of ContentLength:
589+
1: {
590+
req: &http.Request{Body: nil, ContentLength: 5},
591+
want: 0,
592+
},
593+
// ContentLength is used if set.
594+
2: {
595+
req: &http.Request{Body: panicReader{}, ContentLength: 5},
596+
want: 5,
597+
},
598+
// http.NoBody means 0, not -1.
599+
3: {
600+
req: &http.Request{Body: http.NoBody},
601+
want: 0,
602+
},
603+
}
604+
for i, tt := range tests {
605+
got := actualContentLength(tt.req)
606+
if got != tt.want {
607+
t.Errorf("test[%d]: got %d; want %d", i, got, tt.want)
608+
}
609+
}
610+
}
611+
574612
func TestTransportBody(t *testing.T) {
575613
bodyTests := []struct {
576614
body string
@@ -1405,12 +1443,9 @@ func TestTransportChecksRequestHeaderListSize(t *testing.T) {
14051443
}
14061444
}
14071445
headerListSizeForRequest := func(req *http.Request) (size uint64) {
1408-
_, err := httpcommon.EncodeHeaders(httpcommon.EncodeHeadersParam{
1409-
Request: req,
1410-
AddGzipHeader: true,
1411-
PeerMaxHeaderListSize: 0xffffffffffffffff,
1412-
DefaultUserAgent: defaultUserAgent,
1413-
}, func(name, value string) {
1446+
const addGzipHeader = true
1447+
const peerMaxHeaderListSize = 0xffffffffffffffff
1448+
_, err := encodeRequestHeaders(req, addGzipHeader, peerMaxHeaderListSize, func(name, value string) {
14141449
hf := hpack.HeaderField{Name: name, Value: value}
14151450
size += uint64(hf.Size())
14161451
})
@@ -2808,11 +2843,10 @@ func TestTransportRequestPathPseudo(t *testing.T) {
28082843
for i, tt := range tests {
28092844
hbuf := &bytes.Buffer{}
28102845
henc := hpack.NewEncoder(hbuf)
2811-
_, err := httpcommon.EncodeHeaders(httpcommon.EncodeHeadersParam{
2812-
Request: tt.req,
2813-
AddGzipHeader: false,
2814-
PeerMaxHeaderListSize: 0xffffffffffffffff,
2815-
}, func(name, value string) {
2846+
2847+
const addGzipHeader = false
2848+
const peerMaxHeaderListSize = 0xffffffffffffffff
2849+
_, err := encodeRequestHeaders(tt.req, addGzipHeader, peerMaxHeaderListSize, func(name, value string) {
28162850
henc.WriteField(hpack.HeaderField{Name: name, Value: value})
28172851
})
28182852
hdrs := hbuf.Bytes()

internal/http3/roundtrip.go

+24-3
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,19 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (_ *http.Response, err error)
8282
st.stream.SetReadContext(req.Context())
8383
st.stream.SetWriteContext(req.Context())
8484

85+
contentLength := actualContentLength(req)
86+
8587
var encr httpcommon.EncodeHeadersResult
8688
headers := cc.enc.encode(func(yield func(itype indexType, name, value string)) {
87-
encr, err = httpcommon.EncodeHeaders(httpcommon.EncodeHeadersParam{
88-
Request: req,
89+
encr, err = httpcommon.EncodeHeaders(req.Context(), httpcommon.EncodeHeadersParam{
90+
Request: httpcommon.Request{
91+
URL: req.URL,
92+
Method: req.Method,
93+
Host: req.Host,
94+
Header: req.Header,
95+
Trailer: req.Trailer,
96+
ActualContentLength: contentLength,
97+
},
8998
AddGzipHeader: false, // TODO: add when appropriate
9099
PeerMaxHeaderListSize: 0,
91100
DefaultUserAgent: "Go-http-client/3",
@@ -110,7 +119,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (_ *http.Response, err error)
110119
// TODO: Defer sending the request body when "Expect: 100-continue" is set.
111120
rt.reqBody = req.Body
112121
rt.reqBodyWriter.st = st
113-
rt.reqBodyWriter.remain = httpcommon.ActualContentLength(req)
122+
rt.reqBodyWriter.remain = contentLength
114123
rt.reqBodyWriter.flush = true
115124
rt.reqBodyWriter.name = "request"
116125
go copyRequestBody(rt)
@@ -165,6 +174,18 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (_ *http.Response, err error)
165174
}
166175
}
167176

177+
// actualContentLength returns a sanitized version of req.ContentLength,
178+
// where 0 actually means zero (not unknown) and -1 means unknown.
179+
func actualContentLength(req *http.Request) int64 {
180+
if req.Body == nil || req.Body == http.NoBody {
181+
return 0
182+
}
183+
if req.ContentLength != 0 {
184+
return req.ContentLength
185+
}
186+
return -1
187+
}
188+
168189
func copyRequestBody(rt *roundTripState) {
169190
defer rt.closeReqBody()
170191
_, err := io.Copy(&rt.reqBodyWriter, rt.reqBody)

internal/httpcommon/headermap.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
package httpcommon
66

77
import (
8-
"net/http"
8+
"net/textproto"
99
"sync"
1010
)
1111

@@ -82,7 +82,7 @@ func buildCommonHeaderMaps() {
8282
commonLowerHeader = make(map[string]string, len(common))
8383
commonCanonHeader = make(map[string]string, len(common))
8484
for _, v := range common {
85-
chk := http.CanonicalHeaderKey(v)
85+
chk := textproto.CanonicalMIMEHeaderKey(v)
8686
commonLowerHeader[chk] = v
8787
commonCanonHeader[v] = chk
8888
}
@@ -104,7 +104,7 @@ func CanonicalHeader(v string) string {
104104
if s, ok := commonCanonHeader[v]; ok {
105105
return s
106106
}
107-
return http.CanonicalHeaderKey(v)
107+
return textproto.CanonicalMIMEHeaderKey(v)
108108
}
109109

110110
// CachedCanonicalHeader returns the canonical form of a well-known header name.
+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package httpcommon_test
6+
7+
import (
8+
"bytes"
9+
"os"
10+
"path/filepath"
11+
"strings"
12+
"testing"
13+
)
14+
15+
// This package is imported by the net/http package,
16+
// and therefore must not itself import net/http.
17+
func TestNoNetHttp(t *testing.T) {
18+
files, err := filepath.Glob("*.go")
19+
if err != nil {
20+
t.Fatal(err)
21+
}
22+
for _, file := range files {
23+
if strings.HasSuffix(file, "_test.go") {
24+
continue
25+
}
26+
// Could use something complex like go/build or x/tools/go/packages,
27+
// but there's no reason for "net/http" to appear (in quotes) in the source
28+
// otherwise, so just use a simple substring search.
29+
data, err := os.ReadFile(file)
30+
if err != nil {
31+
t.Fatal(err)
32+
}
33+
if bytes.Contains(data, []byte(`"net/http"`)) {
34+
t.Errorf(`%s: cannot import "net/http"`, file)
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)