From de964716b3cd3e8b8bcbd0fa2714935564663e20 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Thu, 26 Sep 2019 13:17:49 -0700 Subject: [PATCH] [release-branch.go1.13] net/http, doc/go1.13.html: revert TimeoutHandler.Flush Also added a test to ensure that any interactions between TimeoutHandler and Flusher result in the correct status code and body, but also that we don't get superfluous logs from stray writes as was seen in the bug report. Fixes #34560. Change-Id: I4af62db256742326f9353f98a2fcb5f71d2a5fd9 Reviewed-on: https://go-review.googlesource.com/c/go/+/197659 Run-TryBot: Emmanuel Odeke Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot (cherry picked from commit 4faf8a8dc44555c4fdbe4fb108f42144e58ae6b1) Reviewed-on: https://go-review.googlesource.com/c/go/+/197543 --- doc/go1.13.html | 2 +- src/net/http/serve_test.go | 48 ++++++++++++++++++++++++++++++++++++++ src/net/http/server.go | 9 ------- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/doc/go1.13.html b/doc/go1.13.html index 504937e112..d4a762721f 100644 --- a/doc/go1.13.html +++ b/doc/go1.13.html @@ -821,7 +821,7 @@ godoc

TimeoutHandler's ResponseWriter now implements the - Pusher and Flusher interfaces. + Pusher interface.

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 1d1449aa65..e86cc9bdd2 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -6161,6 +6161,54 @@ func TestUnsupportedTransferEncodingsReturn501(t *testing.T) { } } +// Issue 34439: ensure that TimeoutHandler doesn't implement Flusher +// and that any interaction with Flusher won't affect TimeoutHandler's behavior. +func TestTimeoutHandlerAndFlusher(t *testing.T) { + timeout := 50 * time.Millisecond + + handler := HandlerFunc(func(w ResponseWriter, r *Request) { + w.WriteHeader(StatusTeapot) + w.Write([]byte("line1\n")) + fl, ok := w.(Flusher) + if ok { + fl.Flush() + } + time.Sleep(timeout * 2) + w.Write([]byte("line2\n")) + }) + + cst := httptest.NewUnstartedServer(TimeoutHandler(handler, timeout, "TIMED OUT\n")) + // Provide a logger that will report an error on any superfluous log. + cst.Config.ErrorLog = log.New(&errorOnWrite{t: t}, "", 0) + cst.Start() + defer cst.Close() + + res, err := cst.Client().Get(cst.URL) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + + if g, w := res.StatusCode, StatusServiceUnavailable; g != w { + t.Errorf("Status code mismatch\ngot: %d\nwant: %d", g, w) + } + + slurp, _ := ioutil.ReadAll(res.Body) + if g, w := string(slurp), "TIMED OUT\n"; g != w { + t.Fatalf("Body mismatch\ngot: %q\nwant: %q", g, w) + } +} + +// errorOnWrite will invoke t.Error on any attempted write. +type errorOnWrite struct { + t *testing.T +} + +func (ew *errorOnWrite) Write(b []byte) (int, error) { + ew.t.Errorf("Unexpected write: %s\n", b) + return len(b), nil +} + // fetchWireResponse is a helper for dialing to host, // sending http1ReqBody as the payload and retrieving // the response as it was sent on the wire. diff --git a/src/net/http/server.go b/src/net/http/server.go index f554c81300..95a5eabaa2 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -3267,7 +3267,6 @@ type timeoutWriter struct { } var _ Pusher = (*timeoutWriter)(nil) -var _ Flusher = (*timeoutWriter)(nil) // Push implements the Pusher interface. func (tw *timeoutWriter) Push(target string, opts *PushOptions) error { @@ -3277,14 +3276,6 @@ func (tw *timeoutWriter) Push(target string, opts *PushOptions) error { return ErrNotSupported } -// Flush implements the Flusher interface. -func (tw *timeoutWriter) Flush() { - f, ok := tw.w.(Flusher) - if ok { - f.Flush() - } -} - func (tw *timeoutWriter) Header() Header { return tw.h } func (tw *timeoutWriter) Write(p []byte) (int, error) { -- 2.48.1