diff --git a/internal/proxy/request.go b/internal/proxy/request.go index 5eedc12..8ddd13a 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -710,6 +710,16 @@ func (rq *request) requestTimedOut(limit string) { } if rq.body == nil || !rq.body.waiting.Load() { + // Once the app has answered, the transport, ReverseProxy and Go's + // server each close the client's body, and closing it reads what + // is left of a body under 256 KiB without requestBody.Read, so + // waiting is false while that read waits for the client. That + // read ends now, before refuse: should smallwebwaf still answer + // itself, answer then sets the deadline it gives its own answers. + if !rq.answerStarted.IsZero() { + rq.stopReadingBody(time.Now()) + } + rq.refuse(refusal{ status: http.StatusGatewayTimeout, action: requestlog.ActionTimedOut, diff --git a/internal/proxy/timeouts_test.go b/internal/proxy/timeouts_test.go index ff00c8c..639f86f 100644 --- a/internal/proxy/timeouts_test.go +++ b/internal/proxy/timeouts_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "strconv" + "strings" "sync" "sync/atomic" "testing" @@ -248,6 +249,97 @@ func TestClientStopsSendingAfterTheAppHasAnswered(t *testing.T) { wantLimitHits(t, addr, clientRequestTimeout, 1) } +func TestClientStopsSendingWithLittleOfItsBodyLeft(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + // limit is the setting set to shortTimeout, which runs out, and + // clientTimeout what SWWAF_CLIENT_REQUEST_TIMEOUT is set to when it + // is not limit. + limit, clientTimeout string + }{ + { + name: "client request timeout", + limit: clientRequestTimeout, + }, + { + name: "upstream request timeout, client request timeout off", + limit: upstreamRequestTimeout, + clientTimeout: off, + }, + { + name: "upstream request timeout, client request timeout longer", + limit: upstreamRequestTimeout, + clientTimeout: longTimeoutSetting, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // appClosed is closed once smallwebwaf has closed its + // connection to the app, which it does once it has the app's + // answer. + appClosed := make(chan struct{}) + + // The app answers without reading the rest of the body, and + // says it closes the connection, only once smallwebwaf waits + // for the client to send more: it takes the bytes the client + // sent, and then waits half the timeout, far longer than + // smallwebwaf takes to go on to wait. + app := startApp(t, func(w http.ResponseWriter, _ *http.Request) { + conn, buf, err := http.NewResponseController(w).Hijack() + if err != nil { + return + } + + _, _ = io.ReadFull(buf.Reader, make([]byte, len("the first bytes"))) + + time.Sleep(shortTimeout / 2) + + _, _ = buf.WriteString("HTTP/1.1 404 Not Found\r\n" + + "Content-Length: 0\r\nConnection: close\r\n\r\n") + _ = buf.Flush() + _, _ = io.Copy(io.Discard, buf.Reader) + _ = conn.Close() + + close(appClosed) + }) + + env := map[string]string{tc.limit: shortTimeoutSetting, metricsToken: token} + if tc.clientTimeout != "" { + env[clientRequestTimeout] = tc.clientTimeout + } + + addr, out := startProxy(t, app.URL, env) + + // Less than 256 KiB of the body is left after the first bytes, + // little enough for Go's server in smallwebwaf to read the rest + // itself before it sends the app's answer. + start := time.Now() + conn := dial(t, addr) + send(t, conn, "POST /upload HTTP/1.1\r\nHost: app\r\nContent-Length: 200000\r\n\r\n"+ + "the first bytes") + + select { + case <-appClosed: + case <-time.After(waitLimit): + t.Fatal("smallwebwaf did not close its connection to the app") + } + + // More of the body, which can no longer go to the app, and then + // nothing. + send(t, conn, strings.Repeat("x", 8<<10)) + + got := readResponse(t, conn) + wantTimedOut(t, start) + wantStatus(t, got, http.StatusNotFound) + wantLine(t, out.requestLine(t), http.StatusNotFound, requestlog.ActionTimedOut) + wantLimitHits(t, addr, tc.limit, 1) + }) + } +} + func TestAppTooSlowToAnswer(t *testing.T) { t.Parallel()