Skip to content

Commit

Permalink
net/http: Transport: be paranoid about any non-100 1xx response
Browse files Browse the repository at this point in the history
Since we can't properly handle anything except 100, treat all
1xx informational responses as sketchy and don't reuse the
connection for future requests.

The only other 1xx response code currently in use in the wild
is WebSockets' use of "101 Switching Protocols", but our
code.google.com/p/go.net/websockets doesn't use Client or
Transport: it uses ReadResponse directly, so is unaffected by
this CL.  (and its tests still pass)

So this CL is entirely just future-proofing paranoia.
Also: the Internet is weird.

Update #2184
Update #3665

R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/8208043
  • Loading branch information
bradfitz committed Mar 31, 2013
1 parent 59ae9d9 commit b80ce20
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
7 changes: 6 additions & 1 deletion src/pkg/net/http/serve_test.go
Expand Up @@ -77,10 +77,15 @@ type rwTestConn struct {
io.Reader
io.Writer
noopConn
closec chan bool // if non-nil, send value to it on close

closeFunc func() error // called if non-nil
closec chan bool // else, if non-nil, send value to it on close
}

func (c *rwTestConn) Close() error {
if c.closeFunc != nil {
return c.closeFunc()
}
select {
case c.closec <- true:
default:
Expand Down
5 changes: 4 additions & 1 deletion src/pkg/net/http/transport.go
Expand Up @@ -715,7 +715,10 @@ func (pc *persistConn) readLoop() {
resp.Body = &bodyEOFSignal{body: resp.Body}
}

if err != nil || resp.Close || rc.req.Close {
if err != nil || resp.Close || rc.req.Close || resp.StatusCode <= 199 {
// Don't do keep-alive on error if either party requested a close
// or we get an unexpected informational (1xx) response.
// StatusCode 100 is already handled above.
alive = false
}

Expand Down
54 changes: 43 additions & 11 deletions src/pkg/net/http/transport_test.go
Expand Up @@ -1416,26 +1416,36 @@ func TestTransportReading100Continue(t *testing.T) {
for {
n++
req, err := ReadRequest(br)
if err == io.EOF {
return
}
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)
if err != nil {
t.Errorf("Server request body slurp: %v", err)
return
}
id := req.Header.Get("Request-Id")
resCode := req.Header.Get("X-Want-Response-Code")
if resCode == "" {
resCode = "100 Continue"
if string(slurp) != reqBody(n) {
t.Errorf("Server got %q, %v; want %q", slurp, err, reqBody(n))
}
}
body := fmt.Sprintf("Response number %d", n)
v := []byte(strings.Replace(fmt.Sprintf(`HTTP/1.1 100 Continue
v := []byte(strings.Replace(fmt.Sprintf(`HTTP/1.1 %s
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))
%s`, resCode, id, len(body), body), "\n", "\r\n", -1))
w.Write(v)
if id == reqID(numReqs) {
return
Expand All @@ -1451,6 +1461,11 @@ Content-Length: %d
conn := &rwTestConn{
Reader: cr,
Writer: sw,
closeFunc: func() error {
sw.Close()
cw.Close()
return nil
},
}
go send100Response(cw, sr)
return conn, nil
Expand All @@ -1459,21 +1474,38 @@ Content-Length: %d
}
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))

testResponse := func(req *Request, name string, wantCode int) {
res, err := c.Do(req)
if err != nil {
t.Fatalf("Do (i=%d): %v", i, err)
t.Fatalf("%s: Do: %v", name, err)
}
if res.StatusCode != 200 {
t.Fatalf("Response Statuscode=%d; want 200 (i=%d): %v", res.StatusCode, i, err)
if res.StatusCode != wantCode {
t.Fatalf("%s: Response Statuscode=%d; want %d", name, res.StatusCode, wantCode)
}
if id, idBack := req.Header.Get("Request-Id"), res.Header.Get("Echo-Request-Id"); id != "" && id != idBack {
t.Errorf("%s: response id %q != request id %q", name, idBack, id)
}
_, err = ioutil.ReadAll(res.Body)
if err != nil {
t.Fatalf("Slurp error (i=%d): %v", i, err)
t.Fatalf("%s: Slurp error: %v", name, err)
}
}

// Few 100 responses, making sure we're not off-by-one.
for i := 1; i <= numReqs; i++ {
req, _ := NewRequest("POST", "http://dummy.tld/", strings.NewReader(reqBody(i)))
req.Header.Set("Request-Id", reqID(i))
testResponse(req, fmt.Sprintf("100, %d/%d", i, numReqs), 200)
}

// And some other informational 1xx but non-100 responses, to test
// we return them but don't re-use the connection.
for i := 1; i <= numReqs; i++ {
req, _ := NewRequest("POST", "http://other.tld/", strings.NewReader(reqBody(i)))
req.Header.Set("X-Want-Response-Code", "123 Sesame Street")
testResponse(req, fmt.Sprintf("123, %d/%d", i, numReqs), 123)
}
}

type proxyFromEnvTest struct {
Expand Down

0 comments on commit b80ce20

Please sign in to comment.