290 lines
10 KiB
Go
290 lines
10 KiB
Go
package httpapi
|
|
|
|
import (
|
|
"bufio"
|
|
"context"
|
|
"fmt"
|
|
"io"
|
|
"log/slog"
|
|
"net"
|
|
"net/http"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// start runs a REAL http.Server, the same one main builds, on a loopback port. Everything below
|
|
// needs that: the defects this file pins live in the server's connection handling, and a mux under
|
|
// httptest never touches it.
|
|
func start(t *testing.T, h http.Handler, to Timeouts) net.Listener {
|
|
t.Helper()
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
srv := serverWithTimeouts("127.0.0.1:0", h, quietLogger(), to)
|
|
ln, err := srv.Listen(ctx)
|
|
if err != nil {
|
|
cancel()
|
|
t.Fatalf("listen: %v", err)
|
|
}
|
|
done := make(chan error, 1)
|
|
go func() { done <- srv.Run(ctx, ln) }()
|
|
t.Cleanup(func() {
|
|
cancel()
|
|
select {
|
|
case err := <-done:
|
|
if err != nil {
|
|
t.Errorf("run: %v", err)
|
|
}
|
|
case <-time.After(5 * time.Second):
|
|
t.Error("server did not stop")
|
|
}
|
|
})
|
|
return ln
|
|
}
|
|
|
|
func quietLogger() *slog.Logger {
|
|
return slog.New(slog.NewTextHandler(io.Discard, nil))
|
|
}
|
|
|
|
func fastTimeouts() Timeouts {
|
|
return Timeouts{ReadHeader: time.Second, Read: 250 * time.Millisecond, Idle: time.Second, Shutdown: 3 * time.Second}
|
|
}
|
|
|
|
// PD-2. A client that announces a body and then stops sending pins the connection: net/http drains
|
|
// the unread body inside the response's header write, and that read inherits the connection
|
|
// deadline. With no ReadTimeout the server waits forever and the connection is held until the
|
|
// CLIENT decides to leave. Mutation caught: delete ReadTimeout from NewServer.
|
|
func TestHalfFedRequestIsDroppedByTheServer(t *testing.T) {
|
|
t.Parallel()
|
|
ln := start(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
// Reads nothing, exactly like the guard that rejects an unauthenticated POST.
|
|
w.WriteHeader(http.StatusUnauthorized)
|
|
}), fastTimeouts())
|
|
|
|
var d net.Dialer
|
|
conn, err := d.DialContext(t.Context(), "tcp", ln.Addr().String())
|
|
if err != nil {
|
|
t.Fatalf("dial: %v", err)
|
|
}
|
|
defer conn.Close()
|
|
if _, err := fmt.Fprint(conn, "POST /v0/books HTTP/1.1\r\nHost: x\r\nContent-Length: 4096\r\n\r\nhalf"); err != nil {
|
|
t.Fatalf("write: %v", err)
|
|
}
|
|
|
|
// Generous relative to the 250ms ReadTimeout and short relative to "forever": the assertion is
|
|
// that the SERVER let go, not that it was fast.
|
|
if err := conn.SetReadDeadline(time.Now().Add(3 * time.Second)); err != nil {
|
|
t.Fatalf("deadline: %v", err)
|
|
}
|
|
start := time.Now()
|
|
if _, err := io.ReadAll(conn); err != nil {
|
|
t.Fatalf("server never closed the connection after %s: %v (connection pinned — PD-2)", time.Since(start), err)
|
|
}
|
|
}
|
|
|
|
// PD-9. A signal must START the drain, not end every in-flight request. With the signal context
|
|
// used as BaseContext, a context-aware handler is cancelled the instant the signal arrives and the
|
|
// grace period is decorative. Mutation caught: BaseContext returning the ctx passed to Run.
|
|
func TestShutdownDrainsInFlightRequests(t *testing.T) {
|
|
t.Parallel()
|
|
entered := make(chan struct{})
|
|
h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
close(entered)
|
|
select {
|
|
case <-time.After(300 * time.Millisecond):
|
|
w.WriteHeader(http.StatusOK)
|
|
_, _ = w.Write([]byte("finished"))
|
|
case <-r.Context().Done():
|
|
w.WriteHeader(http.StatusServiceUnavailable)
|
|
}
|
|
})
|
|
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
srv := serverWithTimeouts("127.0.0.1:0", h, quietLogger(), fastTimeouts())
|
|
ln, err := srv.Listen(ctx)
|
|
if err != nil {
|
|
cancel()
|
|
t.Fatalf("listen: %v", err)
|
|
}
|
|
runDone := make(chan error, 1)
|
|
go func() { runDone <- srv.Run(ctx, ln) }()
|
|
|
|
type result struct {
|
|
status int
|
|
body string
|
|
}
|
|
resp := make(chan result, 1)
|
|
go func() {
|
|
r, err := get(t.Context(), "http://"+ln.Addr().String()+"/slow")
|
|
if err != nil {
|
|
resp <- result{-1, err.Error()}
|
|
return
|
|
}
|
|
defer r.Body.Close()
|
|
b, _ := io.ReadAll(r.Body)
|
|
resp <- result{r.StatusCode, string(b)}
|
|
}()
|
|
|
|
<-entered
|
|
cancel() // the signal
|
|
|
|
select {
|
|
case got := <-resp:
|
|
if got.status != http.StatusOK || got.body != "finished" {
|
|
t.Fatalf("in-flight request was cut by the shutdown: status %d body %q (PD-9)", got.status, got.body)
|
|
}
|
|
case <-time.After(5 * time.Second):
|
|
t.Fatal("no response")
|
|
}
|
|
select {
|
|
case err := <-runDone:
|
|
if err != nil {
|
|
t.Fatalf("run: %v", err)
|
|
}
|
|
case <-time.After(5 * time.Second):
|
|
t.Fatal("Run did not return")
|
|
}
|
|
}
|
|
|
|
// The other half of the PD-2 fix: a response that takes longer than ReadTimeout to write must
|
|
// still arrive whole. It does so with no help from the handler — net/http clears the connection's
|
|
// read deadline once the request has arrived and nothing re-arms it (server.go:2059-2062) — so what
|
|
// is pinned here is that property and the wrappers the response controller reaches through.
|
|
// Mutation caught: removing ReadTimeout's harmlessness (any re-arming), or statusRecorder.Unwrap,
|
|
// without which Flush cannot find the real writer and the frames sit in the buffer.
|
|
func TestStreamOutlivesReadTimeout(t *testing.T) {
|
|
t.Parallel()
|
|
to := fastTimeouts()
|
|
flushed := make(chan error, 1)
|
|
streamed := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.Header().Set("Content-Type", "text/event-stream")
|
|
w.WriteHeader(http.StatusOK)
|
|
rc := http.NewResponseController(w)
|
|
flushed <- rc.Flush()
|
|
select {
|
|
case <-time.After(3 * to.Read): // well past the read deadline the connection started with
|
|
case <-r.Context().Done():
|
|
return // the stream was cut: the client below will see EOF without the late frame
|
|
}
|
|
_, _ = fmt.Fprint(w, "data: late\n\n")
|
|
_ = rc.Flush()
|
|
})
|
|
// Wrapped in the same chain a real route gets, because the wrappers are what the response
|
|
// controller has to reach through.
|
|
ln := start(t, AccessLog(quietLogger())(Recover(quietLogger())(streamed)), to)
|
|
|
|
resp, err := get(t.Context(), "http://"+ln.Addr().String()+"/stream")
|
|
if err != nil {
|
|
t.Fatalf("get: %v", err)
|
|
}
|
|
defer resp.Body.Close()
|
|
select {
|
|
case err := <-flushed:
|
|
if err != nil {
|
|
t.Errorf("flush through the middleware wrappers: %v (statusRecorder.Unwrap)", err)
|
|
}
|
|
case <-time.After(5 * time.Second):
|
|
t.Fatal("handler never flushed")
|
|
}
|
|
sc := bufio.NewScanner(resp.Body)
|
|
for sc.Scan() {
|
|
if strings.Contains(sc.Text(), "late") {
|
|
return
|
|
}
|
|
}
|
|
t.Fatal("stream ended before the late frame: the read deadline cut a long-lived response")
|
|
}
|
|
|
|
// PD-51, the case that decided the fate of the zone's ClearReadDeadline helper. On a request whose
|
|
// body was announced and never finished, the drain inside the response header write is the ONLY
|
|
// thing bounding the connection, and it is bounded by Timeouts.Read. A handler that clears the read
|
|
// deadline first — which the helper's doc comment used to call mandatory for streaming — removes
|
|
// that bound and hangs inside WriteHeader for as long as the client keeps the socket: PD-2 again,
|
|
// re-created by the fix for it. Measured: handler still stuck 4s after the client had gone.
|
|
// Mutation caught: deleting ReadTimeout from NewServer; reintroducing a deadline clear here.
|
|
func TestHalfFedStreamingRequestIsCutLoose(t *testing.T) {
|
|
t.Parallel()
|
|
to := fastTimeouts()
|
|
woke := make(chan error, 1)
|
|
streamed := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.Header().Set("Content-Type", "text/event-stream")
|
|
w.WriteHeader(http.StatusOK)
|
|
// The flush is what blocks: WriteHeader only records the status, and the drain of the body
|
|
// the client never finished happens when the header is actually put on the wire. A handler
|
|
// that never flushes never reaches it — which is why this is the streaming shape, not a
|
|
// bare WriteHeader.
|
|
_ = http.NewResponseController(w).Flush()
|
|
select {
|
|
case <-r.Context().Done():
|
|
woke <- r.Context().Err()
|
|
case <-time.After(2 * time.Second):
|
|
woke <- nil
|
|
}
|
|
})
|
|
ln := start(t, streamed, to)
|
|
|
|
var d net.Dialer
|
|
conn, err := d.DialContext(t.Context(), "tcp", ln.Addr().String())
|
|
if err != nil {
|
|
t.Fatalf("dial: %v", err)
|
|
}
|
|
defer conn.Close()
|
|
if _, err := fmt.Fprint(conn, "POST /stream HTTP/1.1\r\nHost: x\r\nContent-Length: 4096\r\n\r\nhalf"); err != nil {
|
|
t.Fatalf("write: %v", err)
|
|
}
|
|
|
|
select {
|
|
case err := <-woke:
|
|
if err == nil {
|
|
t.Fatal("a half-fed connection outlived the read timeout: nothing bounds it once the drain does not (PD-51)")
|
|
}
|
|
case <-time.After(5 * time.Second):
|
|
t.Fatal("handler never woke: stuck in WriteHeader draining a body that never arrives")
|
|
}
|
|
}
|
|
|
|
// PD-46. The behavioural tests above build their own short deadlines, so a defect in the ones the
|
|
// daemon ships is invisible to them — the exact shape in which PD-2 survived P0, a property checked
|
|
// on an object that is not the one shipped.
|
|
//
|
|
// This asserts on NewServer, which is now the ONLY way to build the serving server and takes no
|
|
// timeouts, so main cannot pass different ones: the value and the wiring are the same call. An
|
|
// earlier version of this test passed DefaultTimeouts() itself and therefore proved only half —
|
|
// replacing main's argument left it green (found by review, measured).
|
|
// Mutation caught: DefaultTimeouts().Read = 0; dropping any timeout line from serverWithTimeouts.
|
|
func TestTheServerTheDaemonRunsHasEveryDeadlineSet(t *testing.T) {
|
|
t.Parallel()
|
|
srv := NewServer("127.0.0.1:0", http.NotFoundHandler(), quietLogger())
|
|
for _, c := range []struct {
|
|
name string
|
|
got time.Duration
|
|
}{
|
|
{"ReadHeaderTimeout", srv.http.ReadHeaderTimeout},
|
|
{"ReadTimeout", srv.http.ReadTimeout},
|
|
{"IdleTimeout", srv.http.IdleTimeout},
|
|
} {
|
|
if c.got <= 0 {
|
|
t.Errorf("%s is %v: a connection with no deadline is held for as long as the client likes (PD-2)",
|
|
c.name, c.got)
|
|
}
|
|
}
|
|
// Absent ON PURPOSE, and the absence is as load-bearing as the values above: a write deadline
|
|
// set here cuts an SSE response at a fixed age. Setting it "for symmetry" is the regression.
|
|
if srv.http.WriteTimeout != 0 {
|
|
t.Errorf("WriteTimeout is %v, want unset: it would cut a streaming response", srv.http.WriteTimeout)
|
|
}
|
|
if srv.grace <= 0 {
|
|
t.Errorf("shutdown grace is %v: a drain would give in-flight requests no time at all", srv.grace)
|
|
}
|
|
if srv.http.ReadHeaderTimeout > srv.http.ReadTimeout {
|
|
t.Errorf("ReadHeaderTimeout %v exceeds ReadTimeout %v: the header deadline can never fire",
|
|
srv.http.ReadHeaderTimeout, srv.http.ReadTimeout)
|
|
}
|
|
}
|
|
|
|
func get(ctx context.Context, url string) (*http.Response, error) {
|
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
return http.DefaultClient.Do(req)
|
|
}
|