X Tutup
Skip to content

Commit b992c39

Browse files
bradfitzbradfitz
committed
net/http: add NoBody, don't return nil from NewRequest on zero bodies
This is an alternate solution to https://golang.org/cl/31445 Instead of making NewRequest return a request with Request.Body == nil to signal a zero byte body, add a well-known variable that means explicitly zero. Too many tests inside Google (and presumably the outside world) broke. Change-Id: I78f6ecca8e8aa1e12179c234ccfb6bcf0ee29ba8 Reviewed-on: https://go-review.googlesource.com/31726 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
1 parent 448e1db commit b992c39

File tree

7 files changed

+47
-42
lines changed

7 files changed

+47
-42
lines changed

‎src/net/http/http.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package http
66

77
import (
8+
"io"
89
"strconv"
910
"strings"
1011
"time"
@@ -81,3 +82,21 @@ func hexEscapeNonASCII(s string) string {
8182
}
8283
return string(b)
8384
}
85+
86+
// NoBody is an io.ReadCloser with no bytes. Read always returns EOF
87+
// and Close always returns nil. It can be used in an outgoing client
88+
// request to explicitly signal that a request has zero bytes.
89+
// An alternative, however, is to simply set Request.Body to nil.
90+
var NoBody = noBody{}
91+
92+
type noBody struct{}
93+
94+
func (noBody) Read([]byte) (int, error) { return 0, io.EOF }
95+
func (noBody) Close() error { return nil }
96+
func (noBody) WriteTo(io.Writer) (int64, error) { return 0, nil }
97+
98+
var (
99+
// verify that an io.Copy from NoBody won't require a buffer:
100+
_ io.WriterTo = NoBody
101+
_ io.ReadCloser = NoBody
102+
)

‎src/net/http/readrequest_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type reqTest struct {
2525
}
2626

2727
var noError = ""
28-
var noBody = ""
28+
var noBodyStr = ""
2929
var noTrailer Header = nil
3030

3131
var reqTests = []reqTest{
@@ -95,7 +95,7 @@ var reqTests = []reqTest{
9595
RequestURI: "/",
9696
},
9797

98-
noBody,
98+
noBodyStr,
9999
noTrailer,
100100
noError,
101101
},
@@ -121,7 +121,7 @@ var reqTests = []reqTest{
121121
RequestURI: "//user@host/is/actually/a/path/",
122122
},
123123

124-
noBody,
124+
noBodyStr,
125125
noTrailer,
126126
noError,
127127
},
@@ -131,7 +131,7 @@ var reqTests = []reqTest{
131131
"GET ../../../../etc/passwd HTTP/1.1\r\n" +
132132
"Host: test\r\n\r\n",
133133
nil,
134-
noBody,
134+
noBodyStr,
135135
noTrailer,
136136
"parse ../../../../etc/passwd: invalid URI for request",
137137
},
@@ -141,7 +141,7 @@ var reqTests = []reqTest{
141141
"GET HTTP/1.1\r\n" +
142142
"Host: test\r\n\r\n",
143143
nil,
144-
noBody,
144+
noBodyStr,
145145
noTrailer,
146146
"parse : empty url",
147147
},
@@ -227,7 +227,7 @@ var reqTests = []reqTest{
227227
RequestURI: "www.google.com:443",
228228
},
229229

230-
noBody,
230+
noBodyStr,
231231
noTrailer,
232232
noError,
233233
},
@@ -251,7 +251,7 @@ var reqTests = []reqTest{
251251
RequestURI: "127.0.0.1:6060",
252252
},
253253

254-
noBody,
254+
noBodyStr,
255255
noTrailer,
256256
noError,
257257
},
@@ -275,7 +275,7 @@ var reqTests = []reqTest{
275275
RequestURI: "/_goRPC_",
276276
},
277277

278-
noBody,
278+
noBodyStr,
279279
noTrailer,
280280
noError,
281281
},
@@ -299,7 +299,7 @@ var reqTests = []reqTest{
299299
RequestURI: "*",
300300
},
301301

302-
noBody,
302+
noBodyStr,
303303
noTrailer,
304304
noError,
305305
},
@@ -323,7 +323,7 @@ var reqTests = []reqTest{
323323
RequestURI: "*",
324324
},
325325

326-
noBody,
326+
noBodyStr,
327327
noTrailer,
328328
noError,
329329
},
@@ -350,7 +350,7 @@ var reqTests = []reqTest{
350350
RequestURI: "/",
351351
},
352352

353-
noBody,
353+
noBodyStr,
354354
noTrailer,
355355
noError,
356356
},
@@ -376,7 +376,7 @@ var reqTests = []reqTest{
376376
RequestURI: "/",
377377
},
378378

379-
noBody,
379+
noBodyStr,
380380
noTrailer,
381381
noError,
382382
},
@@ -397,7 +397,7 @@ var reqTests = []reqTest{
397397
ContentLength: -1,
398398
Close: true,
399399
},
400-
noBody,
400+
noBodyStr,
401401
noTrailer,
402402
noError,
403403
},

‎src/net/http/request.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -771,10 +771,14 @@ func NewRequest(method, urlStr string, body io.Reader) (*Request, error) {
771771
// For client requests, Request.ContentLength of 0
772772
// means either actually 0, or unknown. The only way
773773
// to explicitly say that the ContentLength is zero is
774-
// to set the Body to nil.
774+
// to set the Body to nil. But turns out too much code
775+
// depends on NewRequest returning a non-nil Body,
776+
// so we use a well-known ReadCloser variable instead
777+
// and have the http package also treat that sentinel
778+
// variable to mean explicitly zero.
775779
if req.ContentLength == 0 {
776-
req.Body = nil
777-
req.GetBody = nil
780+
req.Body = NoBody
781+
req.GetBody = func() (io.ReadCloser, error) { return NoBody, nil }
778782
}
779783
}
780784

@@ -1252,7 +1256,7 @@ func (r *Request) isReplayable() bool {
12521256
// outgoingLength reports the Content-Length of this outgoing (Client) request.
12531257
// It maps 0 into -1 (unknown) when the Body is non-nil.
12541258
func (r *Request) outgoingLength() int64 {
1255-
if r.Body == nil {
1259+
if r.Body == nil || r.Body == NoBody {
12561260
return 0
12571261
}
12581262
if r.ContentLength != 0 {

‎src/net/http/request_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ func TestNewRequestContentLength(t *testing.T) {
511511
if req.ContentLength != tt.want {
512512
t.Errorf("test[%d]: ContentLength(%T) = %d; want %d", i, tt.r, req.ContentLength, tt.want)
513513
}
514-
if (req.ContentLength == 0) != (req.Body == nil) {
514+
if (req.ContentLength == 0) != (req.Body == NoBody) {
515515
t.Errorf("test[%d]: ContentLength = %d but Body non-nil is %v", i, req.ContentLength, req.Body != nil)
516516
}
517517
}

‎src/net/http/response.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func (r *Response) Write(w io.Writer) error {
261261
if n == 0 {
262262
// Reset it to a known zero reader, in case underlying one
263263
// is unhappy being read repeatedly.
264-
r1.Body = eofReader
264+
r1.Body = NoBody
265265
} else {
266266
r1.ContentLength = -1
267267
r1.Body = struct {

‎src/net/http/server.go‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,7 +1776,7 @@ func registerOnHitEOF(rc io.ReadCloser, fn func()) {
17761776
// requestBodyRemains reports whether future calls to Read
17771777
// on rc might yield more data.
17781778
func requestBodyRemains(rc io.ReadCloser) bool {
1779-
if rc == eofReader {
1779+
if rc == NoBody {
17801780
return false
17811781
}
17821782
switch v := rc.(type) {
@@ -2702,24 +2702,6 @@ func (globalOptionsHandler) ServeHTTP(w ResponseWriter, r *Request) {
27022702
}
27032703
}
27042704

2705-
type eofReaderWithWriteTo struct{}
2706-
2707-
func (eofReaderWithWriteTo) WriteTo(io.Writer) (int64, error) { return 0, nil }
2708-
func (eofReaderWithWriteTo) Read([]byte) (int, error) { return 0, io.EOF }
2709-
2710-
// eofReader is a non-nil io.ReadCloser that always returns EOF.
2711-
// It has a WriteTo method so io.Copy won't need a buffer.
2712-
var eofReader = &struct {
2713-
eofReaderWithWriteTo
2714-
io.Closer
2715-
}{
2716-
eofReaderWithWriteTo{},
2717-
ioutil.NopCloser(nil),
2718-
}
2719-
2720-
// Verify that an io.Copy from an eofReader won't require a buffer.
2721-
var _ io.WriterTo = eofReader
2722-
27232705
// initNPNRequest is an HTTP handler that initializes certain
27242706
// uninitialized fields in its *Request. Such partially-initialized
27252707
// Requests come from NPN protocol handlers.
@@ -2734,7 +2716,7 @@ func (h initNPNRequest) ServeHTTP(rw ResponseWriter, req *Request) {
27342716
*req.TLS = h.c.ConnectionState()
27352717
}
27362718
if req.Body == nil {
2737-
req.Body = eofReader
2719+
req.Body = NoBody
27382720
}
27392721
if req.RemoteAddr == "" {
27402722
req.RemoteAddr = h.c.RemoteAddr().String()

‎src/net/http/transfer.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,12 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
367367
switch {
368368
case chunked(t.TransferEncoding):
369369
if noBodyExpected(t.RequestMethod) {
370-
t.Body = eofReader
370+
t.Body = NoBody
371371
} else {
372372
t.Body = &body{src: internal.NewChunkedReader(r), hdr: msg, r: r, closing: t.Close}
373373
}
374374
case realLength == 0:
375-
t.Body = eofReader
375+
t.Body = NoBody
376376
case realLength > 0:
377377
t.Body = &body{src: io.LimitReader(r, realLength), closing: t.Close}
378378
default:
@@ -382,7 +382,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
382382
t.Body = &body{src: r, closing: t.Close}
383383
} else {
384384
// Persistent connection (i.e. HTTP/1.1)
385-
t.Body = eofReader
385+
t.Body = NoBody
386386
}
387387
}
388388

0 commit comments

Comments
 (0)
X Tutup