A client that stops sending with little of its body left after the app has answered is timed out (closes #136)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
When less than 256 KiB of a client's body is left after the app has answered, the rest is read by closing the body, which the transport, ReverseProxy and Go's server each do, not through requestBody.Read. waiting was then false, so a request timeout that ran out took the 504 branch, which set no read deadline, and the request held its connection for as long as the client kept it open. Once the app has answered, that branch now ends reading the client's body at once, whichever of SWWAF_CLIENT_REQUEST_TIMEOUT and SWWAF_UPSTREAM_REQUEST_TIMEOUT ran out. Model: opus-5-5
This commit was merged in pull request #144.
This commit is contained in:
@@ -710,6 +710,16 @@ func (rq *request) requestTimedOut(limit string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if rq.body == nil || !rq.body.waiting.Load() {
|
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{
|
rq.refuse(refusal{
|
||||||
status: http.StatusGatewayTimeout,
|
status: http.StatusGatewayTimeout,
|
||||||
action: requestlog.ActionTimedOut,
|
action: requestlog.ActionTimedOut,
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -248,6 +249,97 @@ func TestClientStopsSendingAfterTheAppHasAnswered(t *testing.T) {
|
|||||||
wantLimitHits(t, addr, clientRequestTimeout, 1)
|
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) {
|
func TestAppTooSlowToAnswer(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user