Commit 10a27319 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: don't reject 0-lengthed bodies with Expect 100-continue

I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)

Add more tests, including a test of a chunked request.

Fixes #7625

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/84480045
parent 50368e9a
...@@ -933,31 +933,50 @@ func TestTLSServer(t *testing.T) { ...@@ -933,31 +933,50 @@ func TestTLSServer(t *testing.T) {
} }
type serverExpectTest struct { type serverExpectTest struct {
contentLength int // of request body contentLength int // of request body
chunked bool
expectation string // e.g. "100-continue" expectation string // e.g. "100-continue"
readBody bool // whether handler should read the body (if false, sends StatusUnauthorized) readBody bool // whether handler should read the body (if false, sends StatusUnauthorized)
expectedResponse string // expected substring in first line of http response expectedResponse string // expected substring in first line of http response
} }
func expectTest(contentLength int, expectation string, readBody bool, expectedResponse string) serverExpectTest {
return serverExpectTest{
contentLength: contentLength,
expectation: expectation,
readBody: readBody,
expectedResponse: expectedResponse,
}
}
var serverExpectTests = []serverExpectTest{ var serverExpectTests = []serverExpectTest{
// Normal 100-continues, case-insensitive. // Normal 100-continues, case-insensitive.
{100, "100-continue", true, "100 Continue"}, expectTest(100, "100-continue", true, "100 Continue"),
{100, "100-cOntInUE", true, "100 Continue"}, expectTest(100, "100-cOntInUE", true, "100 Continue"),
// No 100-continue. // No 100-continue.
{100, "", true, "200 OK"}, expectTest(100, "", true, "200 OK"),
// 100-continue but requesting client to deny us, // 100-continue but requesting client to deny us,
// so it never reads the body. // so it never reads the body.
{100, "100-continue", false, "401 Unauthorized"}, expectTest(100, "100-continue", false, "401 Unauthorized"),
// Likewise without 100-continue: // Likewise without 100-continue:
{100, "", false, "401 Unauthorized"}, expectTest(100, "", false, "401 Unauthorized"),
// Non-standard expectations are failures // Non-standard expectations are failures
{0, "a-pony", false, "417 Expectation Failed"}, expectTest(0, "a-pony", false, "417 Expectation Failed"),
// Expect-100 requested but no body // Expect-100 requested but no body (is apparently okay: Issue 7625)
{0, "100-continue", true, "400 Bad Request"}, expectTest(0, "100-continue", true, "200 OK"),
// Expect-100 requested but handler doesn't read the body
expectTest(0, "100-continue", false, "401 Unauthorized"),
// Expect-100 continue with no body, but a chunked body.
{
expectation: "100-continue",
readBody: true,
chunked: true,
expectedResponse: "100 Continue",
},
} }
// Tests that the server responds to the "Expect" request header // Tests that the server responds to the "Expect" request header
...@@ -986,21 +1005,38 @@ func TestServerExpect(t *testing.T) { ...@@ -986,21 +1005,38 @@ func TestServerExpect(t *testing.T) {
// Only send the body immediately if we're acting like an HTTP client // Only send the body immediately if we're acting like an HTTP client
// that doesn't send 100-continue expectations. // that doesn't send 100-continue expectations.
writeBody := test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" writeBody := test.contentLength != 0 && strings.ToLower(test.expectation) != "100-continue"
go func() { go func() {
contentLen := fmt.Sprintf("Content-Length: %d", test.contentLength)
if test.chunked {
contentLen = "Transfer-Encoding: chunked"
}
_, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\r\n"+ _, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\r\n"+
"Connection: close\r\n"+ "Connection: close\r\n"+
"Content-Length: %d\r\n"+ "%s\r\n"+
"Expect: %s\r\nHost: foo\r\n\r\n", "Expect: %s\r\nHost: foo\r\n\r\n",
test.readBody, test.contentLength, test.expectation) test.readBody, contentLen, test.expectation)
if err != nil { if err != nil {
t.Errorf("On test %#v, error writing request headers: %v", test, err) t.Errorf("On test %#v, error writing request headers: %v", test, err)
return return
} }
if writeBody { if writeBody {
var targ io.WriteCloser = struct {
io.Writer
io.Closer
}{
conn,
ioutil.NopCloser(nil),
}
if test.chunked {
targ = httputil.NewChunkedWriter(conn)
}
body := strings.Repeat("A", test.contentLength) body := strings.Repeat("A", test.contentLength)
_, err = fmt.Fprint(conn, body) _, err = fmt.Fprint(targ, body)
if err == nil {
err = targ.Close()
}
if err != nil { if err != nil {
if !test.readBody { if !test.readBody {
// Server likely already hung up on us. // Server likely already hung up on us.
......
...@@ -1158,16 +1158,10 @@ func (c *conn) serve() { ...@@ -1158,16 +1158,10 @@ func (c *conn) serve() {
// Expect 100 Continue support // Expect 100 Continue support
req := w.req req := w.req
if req.expectsContinue() { if req.expectsContinue() {
if req.ProtoAtLeast(1, 1) { if req.ProtoAtLeast(1, 1) && req.ContentLength != 0 {
// Wrap the Body reader with one that replies on the connection // Wrap the Body reader with one that replies on the connection
req.Body = &expectContinueReader{readCloser: req.Body, resp: w} req.Body = &expectContinueReader{readCloser: req.Body, resp: w}
} }
if req.ContentLength == 0 {
w.Header().Set("Connection", "close")
w.WriteHeader(StatusBadRequest)
w.finishRequest()
break
}
req.Header.Del("Expect") req.Header.Del("Expect")
} else if req.Header.get("Expect") != "" { } else if req.Header.get("Expect") != "" {
w.sendExpectationFailed() w.sendExpectationFailed()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment