net/http: don't reuse a server connection after any Write errors

Fixes #8534

LGTM=adg
R=adg
CC=golang-codereviews
https://golang.org/cl/149340044
This commit is contained in:
Brad Fitzpatrick 2014-10-15 17:51:12 +02:00
parent a681749ab5
commit 9d51cd0fee
2 changed files with 126 additions and 3 deletions

View file

@ -114,6 +114,8 @@ type conn struct {
remoteAddr string // network address of remote side
server *Server // the Server on which the connection arrived
rwc net.Conn // i/o connection
w io.Writer // checkConnErrorWriter's copy of wrc, not zeroed on Hijack
werr error // any errors writing to w
sr liveSwitchReader // where the LimitReader reads from; usually the rwc
lr *io.LimitedReader // io.LimitReader(sr)
buf *bufio.ReadWriter // buffered(lr,rwc), reading from bufio->limitReader->sr->rwc
@ -432,13 +434,14 @@ func (srv *Server) newConn(rwc net.Conn) (c *conn, err error) {
c.remoteAddr = rwc.RemoteAddr().String()
c.server = srv
c.rwc = rwc
c.w = rwc
if debugServerConnections {
c.rwc = newLoggingConn("server", c.rwc)
}
c.sr = liveSwitchReader{r: c.rwc}
c.lr = io.LimitReader(&c.sr, noLimit).(*io.LimitedReader)
br := newBufioReader(c.lr)
bw := newBufioWriterSize(c.rwc, 4<<10)
bw := newBufioWriterSize(checkConnErrorWriter{c}, 4<<10)
c.buf = bufio.NewReadWriter(br, bw)
return c, nil
}
@ -956,8 +959,10 @@ func (w *response) bodyAllowed() bool {
// 2. (*response).w, a *bufio.Writer of bufferBeforeChunkingSize bytes
// 3. chunkWriter.Writer (whose writeHeader finalizes Content-Length/Type)
// and which writes the chunk headers, if needed.
// 4. conn.buf, a bufio.Writer of default (4kB) bytes
// 5. the rwc, the net.Conn.
// 4. conn.buf, a bufio.Writer of default (4kB) bytes, writing to ->
// 5. checkConnErrorWriter{c}, which notes any non-nil error on Write
// and populates c.werr with it if so. but otherwise writes to:
// 6. the rwc, the net.Conn.
//
// TODO(bradfitz): short-circuit some of the buffering when the
// initial header contains both a Content-Type and Content-Length.
@ -1027,6 +1032,12 @@ func (w *response) finishRequest() {
// Did not write enough. Avoid getting out of sync.
w.closeAfterReply = true
}
// There was some error writing to the underlying connection
// during the request, so don't re-use this conn.
if w.conn.werr != nil {
w.closeAfterReply = true
}
}
func (w *response) Flush() {
@ -2068,3 +2079,18 @@ func (c *loggingConn) Close() (err error) {
log.Printf("%s.Close() = %v", c.name, err)
return
}
// checkConnErrorWriter writes to c.rwc and records any write errors to c.werr.
// It only contains one field (and a pointer field at that), so it
// fits in an interface value without an extra allocation.
type checkConnErrorWriter struct {
c *conn
}
func (w checkConnErrorWriter) Write(p []byte) (n int, err error) {
n, err = w.c.w.Write(p) // c.w == c.rwc, except after a hijack, when rwc is nil.
if err != nil && w.c.werr == nil {
w.c.werr = err
}
return
}