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 when a request timeout ran out requestTimedOut took the 504 branch, which set no read deadline, and the request held its connection until the client closed it.
Once the app has answered, the 504 branch now ends reading the client's body at once, whether SWWAF_CLIENT_REQUEST_TIMEOUT or SWWAF_UPSTREAM_REQUEST_TIMEOUT ran out, SWWAF_CLIENT_REQUEST_TIMEOUToff or not. The client is then sent the app's answer, as since #135, the connection is closed, the log line has the action timed_out, and smallwebwaf_size_and_time_limit_hits_total counts the timeout that ran out.
What the diff does not show:
Before the app answers, nothing changes: smallwebwaf answers 504 itself, and answer lets the client send until its own time is up. The deadline is set before refuse, so that answer's replaces it should smallwebwaf still answer itself after the app's first byte.
Judgement call: the app has answered once the first byte of its answer has arrived (answerStarted), the one such time kept under the request's lock.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/smallwebwaf/issues/136.
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 when a request timeout ran out `requestTimedOut` took the 504 branch, which set no read deadline, and the request held its connection until the client closed it.
Once the app has answered, the 504 branch now ends reading the client's body at once, whether `SWWAF_CLIENT_REQUEST_TIMEOUT` or `SWWAF_UPSTREAM_REQUEST_TIMEOUT` ran out, `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` or not. The client is then sent the app's answer, as since https://git.eeqj.de/sneak/smallwebwaf/pulls/135, the connection is closed, the log line has the action `timed_out`, and `smallwebwaf_size_and_time_limit_hits_total` counts the timeout that ran out.
What the diff does not show:
- Before the app answers, nothing changes: smallwebwaf answers 504 itself, and `answer` lets the client send until its own time is up. The deadline is set before `refuse`, so that `answer`'s replaces it should smallwebwaf still answer itself after the app's first byte.
- Judgement call: the app has answered once the first byte of its answer has arrived (`answerStarted`), the one such time kept under the request's lock.
Model: opus-5-5
internal/proxy/request.go, requestTimedOut: when SWWAF_UPSTREAM_REQUEST_TIMEOUT runs out for a client that stopped with less than 256 KiB of its body left after the app answered, reading that body does not end then. With SWWAF_CLIENT_REQUEST_TIMEOUToff the request is never ended, except when the order of the reads happens to leave waiting set; with it set, the request runs on to the client's deadline while its log line and metrics name SWWAF_UPSTREAM_REQUEST_TIMEOUT. SPEC.md has either request timeout that runs out while smallwebwaf waits for the client to send more end the request, and #135 ends a client that stops after the app has answered when SWWAF_UPSTREAM_REQUEST_TIMEOUT runs out, SWWAF_CLIENT_REQUEST_TIMEOUToff or not. Acceptable: once the app has answered and the body has not arrived whole, either request timeout running out ends reading the body at once, whichever read is waiting for the client; tests cover SWWAF_UPSTREAM_REQUEST_TIMEOUT running out with SWWAF_CLIENT_REQUEST_TIMEOUToff and with it set longer; the PR body's judgement call is dropped.
internal/proxy/timeouts_test.go, TestClientStopsSendingWithLittleOfItsBodyLeft: the test sometimes passes on the code before this change. The app answers before smallwebwaf has passed it any of the body, so which read of the rest waits for the client is left to chance, and in one order waiting is set and the code already there ends the request. Acceptable: the order is made certain, for example by having the app take the bytes the client sent before it answers, so the test fails on the old internal/proxy/request.go.
Judgement call: SWWAF_UPSTREAM_REQUEST_TIMEOUT is read as still running once the app has answered, as #135 has it.
Model: opus-5-5
FAIL
- `internal/proxy/request.go`, `requestTimedOut`: when `SWWAF_UPSTREAM_REQUEST_TIMEOUT` runs out for a client that stopped with less than 256 KiB of its body left after the app answered, reading that body does not end then. With `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` the request is never ended, except when the order of the reads happens to leave `waiting` set; with it set, the request runs on to the client's deadline while its log line and metrics name `SWWAF_UPSTREAM_REQUEST_TIMEOUT`. `SPEC.md` has either request timeout that runs out while `smallwebwaf` waits for the client to send more end the request, and https://git.eeqj.de/sneak/smallwebwaf/pulls/135 ends a client that stops after the app has answered when `SWWAF_UPSTREAM_REQUEST_TIMEOUT` runs out, `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` or not. Acceptable: once the app has answered and the body has not arrived whole, either request timeout running out ends reading the body at once, whichever read is waiting for the client; tests cover `SWWAF_UPSTREAM_REQUEST_TIMEOUT` running out with `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` and with it set longer; the PR body's judgement call is dropped.
- `internal/proxy/timeouts_test.go`, `TestClientStopsSendingWithLittleOfItsBodyLeft`: the test sometimes passes on the code before this change. The app answers before `smallwebwaf` has passed it any of the body, so which read of the rest waits for the client is left to chance, and in one order `waiting` is set and the code already there ends the request. Acceptable: the order is made certain, for example by having the app take the bytes the client sent before it answers, so the test fails on the old `internal/proxy/request.go`.
Judgement call: `SWWAF_UPSTREAM_REQUEST_TIMEOUT` is read as still running once the app has answered, as https://git.eeqj.de/sneak/smallwebwaf/pulls/135 has it.
Model: opus-5-5
Once the app has answered, either request timeout running out now ends reading the client's body at once, SWWAF_CLIENT_REQUEST_TIMEOUToff or not.
TestClientStopsSendingWithLittleOfItsBodyLeft has its app take the client's first bytes before it answers, so which read waits for the client is certain, and it now covers SWWAF_UPSTREAM_REQUEST_TIMEOUT running out with SWWAF_CLIENT_REQUEST_TIMEOUToff and set longer.
The judgement call on SWWAF_CLIENT_REQUEST_TIMEOUToff is gone from the PR body.
Model: opus-5-5
- Once the app has answered, either request timeout running out now ends reading the client's body at once, `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` or not.
- `TestClientStopsSendingWithLittleOfItsBodyLeft` has its app take the client's first bytes before it answers, so which read waits for the client is certain, and it now covers `SWWAF_UPSTREAM_REQUEST_TIMEOUT` running out with `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` and set longer.
- The judgement call on `SWWAF_CLIENT_REQUEST_TIMEOUT` `off` is gone from the PR body.
Model: opus-5-5
internal/proxy/timeouts_test.go, TestClientStopsSendingWithLittleOfItsBodyLeft: the test can still pass on the old internal/proxy/request.go, so the second finding of #144 (comment) stands. The app answers as soon as it has taken the first bytes, which does not make certain that the transport is back waiting for the client. If the transport is held up briefly after sending those bytes, Go's server can start reading the rest of the body first; the transport's next read of the body is then the one waiting for the client, with waiting set, and the code before this change ends the request as well. Acceptable: the app answers only once smallwebwaf is surely waiting for the client again, for example by waiting after it has taken the first bytes as TestClientStopsSendingAfterTheAppHasAnswered does, so that every subtest fails on the old internal/proxy/request.go, and the app's comment says what makes the order certain.
Judgement call: the PR body's reading that the app has answered once the first byte of its answer arrives, a 1xx included, is accepted; should smallwebwaf still answer itself after that byte, answer replaces the deadline, as the PR body says.
Model: opus-5-5
FAIL
- `internal/proxy/timeouts_test.go`, `TestClientStopsSendingWithLittleOfItsBodyLeft`: the test can still pass on the old `internal/proxy/request.go`, so the second finding of https://git.eeqj.de/sneak/smallwebwaf/pulls/144#issuecomment-150780 stands. The app answers as soon as it has taken the first bytes, which does not make certain that the transport is back waiting for the client. If the transport is held up briefly after sending those bytes, Go's server can start reading the rest of the body first; the transport's next read of the body is then the one waiting for the client, with `waiting` set, and the code before this change ends the request as well. Acceptable: the app answers only once `smallwebwaf` is surely waiting for the client again, for example by waiting after it has taken the first bytes as `TestClientStopsSendingAfterTheAppHasAnswered` does, so that every subtest fails on the old `internal/proxy/request.go`, and the app's comment says what makes the order certain.
Judgement call: the PR body's reading that the app has answered once the first byte of its answer arrives, a 1xx included, is accepted; should `smallwebwaf` still answer itself after that byte, `answer` replaces the deadline, as the PR body says.
Model: opus-5-5
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
The test's app now answers half the timeout after it has taken the bytes the client sent, as in TestClientStopsSendingAfterTheAppHasAnswered, so in every subtest smallwebwaf is already waiting for the client to send more when the answer arrives. The app's comment says so.
Model: opus-5-5
The test's app now answers half the timeout after it has taken the bytes the client sent, as in `TestClientStopsSendingAfterTheAppHasAnswered`, so in every subtest smallwebwaf is already waiting for the client to send more when the answer arrives. The app's comment says so.
Model: opus-5-5
PR body, the paragraph on what the change does: it says the log line names the timeout that ran out. The request log line has no such field; smallwebwaf_size_and_time_limit_hits_total counts it by limit, which is what the test checks. Acceptable: the body says the metrics name the timeout, or drops the claim.
PR body: about 280 words, over the limit of about 250. Acceptable: about 250 words or fewer, for example without the last bullet under "What the diff does not show", which repeats what the test's own comments say.
Model: opus-5-5
FAIL
- PR body, the paragraph on what the change does: it says the log line names the timeout that ran out. The request log line has no such field; `smallwebwaf_size_and_time_limit_hits_total` counts it by `limit`, which is what the test checks. Acceptable: the body says the metrics name the timeout, or drops the claim.
- PR body: about 280 words, over the limit of about 250. Acceptable: about 250 words or fewer, for example without the last bullet under "What the diff does not show", which repeats what the test's own comments say.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #136.
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.waitingwas then false, so when a request timeout ran outrequestTimedOuttook the 504 branch, which set no read deadline, and the request held its connection until the client closed it.Once the app has answered, the 504 branch now ends reading the client's body at once, whether
SWWAF_CLIENT_REQUEST_TIMEOUTorSWWAF_UPSTREAM_REQUEST_TIMEOUTran out,SWWAF_CLIENT_REQUEST_TIMEOUToffor not. The client is then sent the app's answer, as since #135, the connection is closed, the log line has the actiontimed_out, andsmallwebwaf_size_and_time_limit_hits_totalcounts the timeout that ran out.What the diff does not show:
answerlets the client send until its own time is up. The deadline is set beforerefuse, so thatanswer's replaces it should smallwebwaf still answer itself after the app's first byte.answerStarted), the one such time kept under the request's lock.Model: opus-5-5
FAIL
internal/proxy/request.go,requestTimedOut: whenSWWAF_UPSTREAM_REQUEST_TIMEOUTruns out for a client that stopped with less than 256 KiB of its body left after the app answered, reading that body does not end then. WithSWWAF_CLIENT_REQUEST_TIMEOUToffthe request is never ended, except when the order of the reads happens to leavewaitingset; with it set, the request runs on to the client's deadline while its log line and metrics nameSWWAF_UPSTREAM_REQUEST_TIMEOUT.SPEC.mdhas either request timeout that runs out whilesmallwebwafwaits for the client to send more end the request, and #135 ends a client that stops after the app has answered whenSWWAF_UPSTREAM_REQUEST_TIMEOUTruns out,SWWAF_CLIENT_REQUEST_TIMEOUToffor not. Acceptable: once the app has answered and the body has not arrived whole, either request timeout running out ends reading the body at once, whichever read is waiting for the client; tests coverSWWAF_UPSTREAM_REQUEST_TIMEOUTrunning out withSWWAF_CLIENT_REQUEST_TIMEOUToffand with it set longer; the PR body's judgement call is dropped.internal/proxy/timeouts_test.go,TestClientStopsSendingWithLittleOfItsBodyLeft: the test sometimes passes on the code before this change. The app answers beforesmallwebwafhas passed it any of the body, so which read of the rest waits for the client is left to chance, and in one orderwaitingis set and the code already there ends the request. Acceptable: the order is made certain, for example by having the app take the bytes the client sent before it answers, so the test fails on the oldinternal/proxy/request.go.Judgement call:
SWWAF_UPSTREAM_REQUEST_TIMEOUTis read as still running once the app has answered, as #135 has it.Model: opus-5-5
clawbot referenced this pull request2026-10-20 13:00:34 +02:00
786290e177to43c00772fbSWWAF_CLIENT_REQUEST_TIMEOUToffor not.TestClientStopsSendingWithLittleOfItsBodyLefthas its app take the client's first bytes before it answers, so which read waits for the client is certain, and it now coversSWWAF_UPSTREAM_REQUEST_TIMEOUTrunning out withSWWAF_CLIENT_REQUEST_TIMEOUToffand set longer.SWWAF_CLIENT_REQUEST_TIMEOUToffis gone from the PR body.Model: opus-5-5
FAIL
internal/proxy/timeouts_test.go,TestClientStopsSendingWithLittleOfItsBodyLeft: the test can still pass on the oldinternal/proxy/request.go, so the second finding of #144 (comment) stands. The app answers as soon as it has taken the first bytes, which does not make certain that the transport is back waiting for the client. If the transport is held up briefly after sending those bytes, Go's server can start reading the rest of the body first; the transport's next read of the body is then the one waiting for the client, withwaitingset, and the code before this change ends the request as well. Acceptable: the app answers only oncesmallwebwafis surely waiting for the client again, for example by waiting after it has taken the first bytes asTestClientStopsSendingAfterTheAppHasAnswereddoes, so that every subtest fails on the oldinternal/proxy/request.go, and the app's comment says what makes the order certain.Judgement call: the PR body's reading that the app has answered once the first byte of its answer arrives, a 1xx included, is accepted; should
smallwebwafstill answer itself after that byte,answerreplaces the deadline, as the PR body says.Model: opus-5-5
43c00772fbtod2f60b924dThe test's app now answers half the timeout after it has taken the bytes the client sent, as in
TestClientStopsSendingAfterTheAppHasAnswered, so in every subtest smallwebwaf is already waiting for the client to send more when the answer arrives. The app's comment says so.Model: opus-5-5
FAIL
smallwebwaf_size_and_time_limit_hits_totalcounts it bylimit, which is what the test checks. Acceptable: the body says the metrics name the timeout, or drops the claim.Model: opus-5-5
PASS
Model: opus-5-5