From c7e0dda45046c539d99b7e8c3bd3e79b3fab9776 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 23 Oct 2016 06:04:14 -0700 Subject: [PATCH] net/http: add Server.ReadHeaderTimeout, IdleTimeout, document WriteTimeout Updates #14204 Updates #16450 Updates #16100 Change-Id: Ic283bcec008a8e0bfbcfd8531d30fffe71052531 Reviewed-on: https://go-review.googlesource.com/32024 Reviewed-by: Tom Bergan Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/serve_test.go | 54 ++++++++++++++++++++++++ src/net/http/server.go | 84 ++++++++++++++++++++++++++++++++------ 2 files changed, 125 insertions(+), 13 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 2bdef9080a..7834478352 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -4778,3 +4778,57 @@ func TestConcurrentServerServe(t *testing.T) { go func() { srv.Serve(ln2) }() } } + +func TestServerIdleTimeout(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + defer afterTest(t) + ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, r *Request) { + io.Copy(ioutil.Discard, r.Body) + io.WriteString(w, r.RemoteAddr) + })) + ts.Config.ReadHeaderTimeout = 1 * time.Second + ts.Config.IdleTimeout = 2 * time.Second + ts.Start() + defer ts.Close() + + tr := &Transport{} + defer tr.CloseIdleConnections() + c := &Client{Transport: tr} + + get := func() string { + res, err := c.Get(ts.URL) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + slurp, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + return string(slurp) + } + + a1, a2 := get(), get() + if a1 != a2 { + t.Fatalf("did requests on different connections") + } + time.Sleep(3 * time.Second) + a3 := get() + if a2 == a3 { + t.Fatal("request three unexpectedly on same connection") + } + + // And test that ReadHeaderTimeout still works: + conn, err := net.Dial("tcp", ts.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + conn.Write([]byte("GET / HTTP/1.1\r\nHost: foo.com\r\n")) + time.Sleep(2 * time.Second) + if _, err := io.CopyN(ioutil.Discard, conn, 1); err == nil { + t.Fatal("copy byte succeeded; want err") + } +} diff --git a/src/net/http/server.go b/src/net/http/server.go index c47cc328fc..51a66c37d5 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -237,7 +237,6 @@ type conn struct { r *connReader // bufr reads from r. - // Users of bufr must hold mu. bufr *bufio.Reader // bufw writes to checkConnErrorWriter{c}, which populates werr on error. @@ -247,11 +246,11 @@ type conn struct { // on this connection, if any. lastMethod string - // mu guards hijackedv, use of bufr, (*response).closeNotifyCh. - mu sync.Mutex - curReq atomic.Value // of *response (which has a Request in it) + // mu guards hijackedv + mu sync.Mutex + // hijackedv is whether this connection has been hijacked // by a Handler with the Hijacker interface. // It is guarded by mu. @@ -426,7 +425,7 @@ type response struct { // closeNotifyCh is the channel returned by CloseNotify. // TODO(bradfitz): this is currently (for Go 1.8) always - // non-nil. Make this lazily-created again as it used to be. + // non-nil. Make this lazily-created again as it used to be? closeNotifyCh chan bool didCloseNotify int32 // atomic (only 0->1 winner should send) } @@ -847,9 +846,18 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) { return nil, ErrHijacked } + var ( + wholeReqDeadline time.Time // or zero if none + hdrDeadline time.Time // or zero if none + ) + t0 := time.Now() + if d := c.server.readHeaderTimeout(); d != 0 { + hdrDeadline = t0.Add(d) + } if d := c.server.ReadTimeout; d != 0 { - c.rwc.SetReadDeadline(time.Now().Add(d)) + wholeReqDeadline = t0.Add(d) } + c.rwc.SetReadDeadline(hdrDeadline) if d := c.server.WriteTimeout; d != 0 { defer func() { c.rwc.SetWriteDeadline(time.Now().Add(d)) @@ -857,14 +865,12 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) { } c.r.setReadLimit(c.server.initialReadLimitSize()) - c.mu.Lock() // while using bufr if c.lastMethod == "POST" { // RFC 2616 section 4.1 tolerance for old buggy clients. peek, _ := c.bufr.Peek(4) // ReadRequest will get err below c.bufr.Discard(numLeadingCRorLF(peek)) } req, err := readRequest(c.bufr, keepHostHeader) - c.mu.Unlock() if err != nil { if c.r.hitReadLimit() { return nil, errTooLarge @@ -910,6 +916,11 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) { body.doEarlyClose = true } + // Adjust the read deadline if necessary. + if !hdrDeadline.Equal(wholeReqDeadline) { + c.rwc.SetReadDeadline(wholeReqDeadline) + } + w = &response{ conn: c, cancelCtx: cancelCtx, @@ -1710,6 +1721,14 @@ func (c *conn) serve(ctx context.Context) { } c.setState(c.rwc, StateIdle) c.curReq.Store((*response)(nil)) + + if d := c.server.idleTimeout(); d != 0 { + c.rwc.SetReadDeadline(time.Now().Add(d)) + if _, err := c.bufr.Peek(4); err != nil { + return + } + } + c.rwc.SetReadDeadline(time.Time{}) } } @@ -2168,11 +2187,36 @@ func Serve(l net.Listener, handler Handler) error { // A Server defines parameters for running an HTTP server. // The zero value for Server is a valid configuration. type Server struct { - Addr string // TCP address to listen on, ":http" if empty - Handler Handler // handler to invoke, http.DefaultServeMux if nil - ReadTimeout time.Duration // maximum duration before timing out read of the request - WriteTimeout time.Duration // maximum duration before timing out write of the response - TLSConfig *tls.Config // optional TLS config, used by ListenAndServeTLS + Addr string // TCP address to listen on, ":http" if empty + Handler Handler // handler to invoke, http.DefaultServeMux if nil + TLSConfig *tls.Config // optional TLS config, used by ListenAndServeTLS + + // ReadTimeout is the maximum duration for reading the entire + // request, including the body. + // + // Because ReadTimeout does not let Handlers make per-request + // decisions on each request body's acceptable deadline or + // upload rate, most users will prefer to use + // ReadHeaderTimeout. It is valid to use them both. + ReadTimeout time.Duration + + // ReadHeaderTimeout is the amount of time allowed to read + // request headers. The connection's read deadline is reset + // after reading the headers and the Handler can decide what + // is considered too slow for the body. + ReadHeaderTimeout time.Duration + + // WriteTimeout is the maximum duration before timing out + // writes of the response. It is reset whenever a new + // request's header is read. Like ReadTimeout, it does not + // let Handlers make decisions on a per-request basis. + WriteTimeout time.Duration + + // IdleTimeout is the maximum amount of time to wait for the + // next request when keep-alives are enabled. If IdleTimeout + // is zero, the value of ReadTimeout is used. If both are + // zero, there is no timeout. + IdleTimeout time.Duration // MaxHeaderBytes controls the maximum number of bytes the // server will read parsing the request header's keys and @@ -2366,6 +2410,20 @@ func (srv *Server) Serve(l net.Listener) error { } } +func (s *Server) idleTimeout() time.Duration { + if s.IdleTimeout != 0 { + return s.IdleTimeout + } + return s.ReadTimeout +} + +func (s *Server) readHeaderTimeout() time.Duration { + if s.ReadHeaderTimeout != 0 { + return s.ReadHeaderTimeout + } + return s.ReadTimeout +} + func (s *Server) doKeepAlives() bool { return atomic.LoadInt32(&s.disableKeepAlives) == 0 } -- 2.48.1