summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrad Fitzpatrick <bradfitz@golang.org>2013-03-29 20:25:11 -0700
committerBrad Fitzpatrick <bradfitz@golang.org>2013-03-29 20:25:11 -0700
commit2a91051f843f7449b5ff0cea49a7865cc16d0c82 (patch)
treecd9526cc974e644dfe137244eb6cb2822e482863
parent9a03bbb22c842fae25af564b1942eb9917d06148 (diff)
downloadgo-2a91051f843f7449b5ff0cea49a7865cc16d0c82.tar.gz
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 Issue 3665 R=golang-dev, dsymonds, dave, jgrahamc CC=golang-dev https://codereview.appspot.com/8166045
-rw-r--r--src/pkg/net/http/transport.go8
-rw-r--r--src/pkg/net/http/transport_test.go94
2 files changed, 102 insertions, 0 deletions
diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index 08ced2c3d..ea02ffb53 100644
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -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
diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go
index c6baf797c..3caa3845d 100644
--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -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