Commit a79df7bb authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: ignore 100-continue responses in Transport

"There are only two hard problems in computer science:
cache invalidation, naming things, and off-by-one errors."

The HTTP server code already strips Expect: 100-continue on
requests, so httputil.ReverseProxy should be unaffected, but
some servers send unsolicited HTTP/1.1 100 Continue responses,
so we need to skip over them if they're seen to avoid getting
off-by-one on Transport requests/responses.

This does change the behavior of people who were using Client
or Transport directly and explicitly setting "Expect: 100-continue"
themselves, but it didn't work before anyway. Now instead of the
user code seeing a 100 response and then things blowing up, now
it basically works, except the Transport will still blast away
the full request body immediately.  That's the part that needs
to be finished to close this issue.

This is the safe quick fix.

Update #3665

R=golang-dev, dsymonds, dave, jgrahamc
CC=golang-dev
https://golang.org/cl/8166045
parent 3197be48
......@@ -686,6 +686,14 @@ func (pc *persistConn) readLoop() {
var resp *Response
if err == nil {
resp, err = ReadResponse(pc.br, rc.req)
if err == nil && resp.StatusCode == 100 {
// Skip any 100-continue for now.
// TODO(bradfitz): if rc.req had "Expect: 100-continue",
// actually block the request body write and signal the
// writeLoop now to begin sending it. (Issue 2184) For now we
// eat it, since we're never expecting one.
resp, err = ReadResponse(pc.br, rc.req)
}
}
hasBody := resp != nil && rc.req.Method != "HEAD" && resp.ContentLength != 0
......
......@@ -7,6 +7,7 @@
package http_test
import (
"bufio"
"bytes"
"compress/gzip"
"crypto/rand"
......@@ -1399,6 +1400,99 @@ func TestTransportSocketLateBinding(t *testing.T) {
dialGate <- true
}
// Issue 2184
func TestTransportReading100Continue(t *testing.T) {
defer afterTest(t)
var writers struct {
sync.Mutex
list []*io.PipeWriter
}
registerPipe := func(pw *io.PipeWriter) {
writers.Lock()
defer writers.Unlock()
writers.list = append(writers.list, pw)
}
defer func() {
writers.Lock()
defer writers.Unlock()
for _, pw := range writers.list {
pw.Close()
}
}()
const numReqs = 5
reqBody := func(n int) string { return fmt.Sprintf("request body %d", n) }
reqID := func(n int) string { return fmt.Sprintf("REQ-ID-%d", n) }
send100Response := func(w *io.PipeWriter, r *io.PipeReader) {
defer w.Close()
defer r.Close()
br := bufio.NewReader(r)
n := 0
for {
n++
req, err := ReadRequest(br)
if err != nil {
t.Error(err)
return
}
slurp, err := ioutil.ReadAll(req.Body)
if err != nil || string(slurp) != reqBody(n) {
t.Errorf("Server got %q, %v; want 'body'", slurp, err)
return
}
id := req.Header.Get("Request-Id")
body := fmt.Sprintf("Response number %d", n)
v := []byte(strings.Replace(fmt.Sprintf(`HTTP/1.1 100 Continue
Date: Thu, 28 Feb 2013 17:55:41 GMT
HTTP/1.1 200 OK
Content-Type: text/html
Echo-Request-Id: %s
Content-Length: %d
%s`, id, len(body), body), "\n", "\r\n", -1))
w.Write(v)
if id == reqID(numReqs) {
return
}
}
}
tr := &Transport{
Dial: func(n, addr string) (net.Conn, error) {
pr, pw := io.Pipe()
registerPipe(pw)
conn := &rwTestConn{
Reader: pr,
Writer: pw,
}
go send100Response(pw, pr)
return conn, nil
},
DisableKeepAlives: false,
}
defer tr.CloseIdleConnections()
c := &Client{Transport: tr}
for i := 1; i <= numReqs; i++ {
req, _ := NewRequest("POST", "http://dummy.tld/", strings.NewReader(reqBody(i)))
req.Header.Set("Request-Id", reqID(i))
res, err := c.Do(req)
if err != nil {
t.Fatalf("Do (i=%d): %v", i, err)
}
if res.StatusCode != 200 {
t.Fatalf("Response Statuscode=%d; want 200 (i=%d): %v", res.StatusCode, i, err)
}
_, err = ioutil.ReadAll(res.Body)
if err != nil {
t.Fatalf("Slurp error (i=%d): %v", i, err)
}
}
}
type proxyFromEnvTest struct {
req string // URL to fetch; blank means "http://example.com"
env string
......
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