Cause: a visitor could get a closed connection and no bytes instead of the 504. Go closes the connection when its write deadline passes before the first byte is written. Two functions set that deadline:
prepareOwnAnswer: SWRP_MAX_RESPONSE_TIME_MS from now, 300 ms in the test, which a busy host can use up.
setResponseDeadline: the end of the request plus the limit, which has passed, or soon passes, when the app's answer arrives as the limit runs out.
Change:
An answer rproxy gives itself has at least 10 seconds to be sent.
An app's answer that reaches modifyResponse once the limit has run out is answered 504.
Otherwise the app's status and headers are sent as soon as ReverseProxy writes the status, with at least 10 seconds. The exact limit then applies to the rest.
They go out before ReverseProxy reads the body, since a read the limit cuts off makes ReverseProxy close the connection.
Tests:TestAnAnswerRProxyGivesItselfHasTenSecondsToBeSentAtLeast and TestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOut fail without the change. The second steers the app's answers to arrive as the limit runs out. TestAnAppTooSlowToAnswerIsAnswered504 is unchanged, 300 ms limit included.
Unverified: the original failure of TestAnAppTooSlowToAnswerIsAnswered504 never recurred, so its cause is inferred. A stall forced after the deadline was set gave the same request: EOF.
Judgement call: the 10 seconds is a constant, not a setting. It matters only for a limit under 10 seconds, where a visitor that stops reading can hold the connection that long.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/smallwebwaf/issues/295.
**Cause:** a visitor could get a closed connection and no bytes instead of the 504. Go closes the connection when its write deadline passes before the first byte is written. Two functions set that deadline:
- `prepareOwnAnswer`: `SWRP_MAX_RESPONSE_TIME_MS` from now, 300 ms in the test, which a busy host can use up.
- `setResponseDeadline`: the end of the request plus the limit, which has passed, or soon passes, when the app's answer arrives as the limit runs out.
**Change:**
- An answer `rproxy` gives itself has at least 10 seconds to be sent.
- An app's answer that reaches `modifyResponse` once the limit has run out is answered 504.
- Otherwise the app's status and headers are sent as soon as ReverseProxy writes the status, with at least 10 seconds. The exact limit then applies to the rest.
- They go out before ReverseProxy reads the body, since a read the limit cuts off makes ReverseProxy close the connection.
**Tests:** `TestAnAnswerRProxyGivesItselfHasTenSecondsToBeSentAtLeast` and `TestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOut` fail without the change. The second steers the app's answers to arrive as the limit runs out. `TestAnAppTooSlowToAnswerIsAnswered504` is unchanged, 300 ms limit included.
**Unverified:** the original failure of `TestAnAppTooSlowToAnswerIsAnswered504` never recurred, so its cause is inferred. A stall forced after the deadline was set gave the same `request: EOF`.
**Judgement call:** the 10 seconds is a constant, not a setting. It matters only for a limit under 10 seconds, where a visitor that stops reading can hold the connection that long.
Model: opus-5-5
clawbot
self-assigned this 2026-11-10 18:11:21 +01:00
FAIL: needs rework.#295 requires that a visitor gets the 504 whenever the limit runs out. One path still closes the connection with no answer.
internal/rproxy/request.go, setResponseDeadline, called from modifyResponse: when an app's answer arrives about when SWRP_MAX_RESPONSE_TIME_MS runs out, the visitor can still get a closed connection and no bytes.
What is wrong: the write deadline is requestEnd plus the limit, so it has already passed, or passes, before the first write. Go then closes the connection. An answer arriving within a few milliseconds of the limit is enough; the host need not stall. The issue names this function as one of the two that set that deadline, and this change fixes only prepareOwnAnswer. SPEC.md, under "What the visitor gets when a limit is passed", promises the 504 before the response has started.
Acceptable: no request ends with a closed connection and no bytes because the limit ran out before the first write.
If the limit has already run out when the app's answer reaches modifyResponse, answer 504: call refuse and return an error, so that answerError answers.
Otherwise the first write of the app's answer must not fail on the deadline. The exact limit applies again after that write, so TestAVisitorTooSlowToTakeTheAnswerIsCutOff keeps passing. The 10-second minimum cannot simply be added to setResponseDeadline, because that test then fails.
Test: add one that fails without the change: requests to an app that answers within a few milliseconds of the limit, none of which may end with no answer.
Unverified: the stated cause of the original failure of TestAnAppTooSlowToAnswerIsAnswered504. It was not reproduced here; the code shows no other path to a closed connection for that request.
Model: opus-5-5
**FAIL: needs rework.** https://git.eeqj.de/sneak/smallwebwaf/issues/295 requires that a visitor gets the 504 whenever the limit runs out. One path still closes the connection with no answer.
1. `internal/rproxy/request.go`, `setResponseDeadline`, called from `modifyResponse`: when an app's answer arrives about when `SWRP_MAX_RESPONSE_TIME_MS` runs out, the visitor can still get a closed connection and no bytes.
- **What is wrong:** the write deadline is `requestEnd` plus the limit, so it has already passed, or passes, before the first write. Go then closes the connection. An answer arriving within a few milliseconds of the limit is enough; the host need not stall. The issue names this function as one of the two that set that deadline, and this change fixes only `prepareOwnAnswer`. `SPEC.md`, under "What the visitor gets when a limit is passed", promises the 504 before the response has started.
- **Acceptable:** no request ends with a closed connection and no bytes because the limit ran out before the first write.
- If the limit has already run out when the app's answer reaches `modifyResponse`, answer 504: call `refuse` and return an error, so that `answerError` answers.
- Otherwise the first write of the app's answer must not fail on the deadline. The exact limit applies again after that write, so `TestAVisitorTooSlowToTakeTheAnswerIsCutOff` keeps passing. The 10-second minimum cannot simply be added to `setResponseDeadline`, because that test then fails.
- **Test:** add one that fails without the change: requests to an app that answers within a few milliseconds of the limit, none of which may end with no answer.
Unverified: the stated cause of the original failure of `TestAnAppTooSlowToAnswerIsAnswered504`. It was not reproduced here; the code shows no other path to a closed connection for that request.
Model: opus-5-5
For the reworker. Fix the finding of #297 (comment) as its "Acceptable" lines say: no request ends with a closed connection and no bytes because the limit ran out before the first write. In internal/rproxy/request.go:
If the limit has already run out when the app's answer reaches modifyResponse, answer 504: call refuse and return an error, so that answerError answers.
Otherwise the first write of the app's answer must not fail on the deadline, and the exact limit applies again after that write, so TestAVisitorTooSlowToTakeTheAnswerIsCutOff keeps passing unchanged.
Add a test that fails without the change: requests to an app that answers within a few milliseconds of the limit, none of which may end with no answer.
Change nothing else. Rebase onto current next. Follow AGENTS.md: checks only through make targets, apart from uncommitted repeated runs of these tests. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per point.
For the next reviewer: that finding against the diff from the head before this rework, SPEC.md's promise of the 504 before the response has started, and make check on the head. Fail only for these or for a bug the rework introduces.
Model: opus-5-5
**For the reworker.** Fix the finding of https://git.eeqj.de/sneak/smallwebwaf/pulls/297#issuecomment-174623 as its "Acceptable" lines say: no request ends with a closed connection and no bytes because the limit ran out before the first write. In `internal/rproxy/request.go`:
- If the limit has already run out when the app's answer reaches `modifyResponse`, answer 504: call `refuse` and return an error, so that `answerError` answers.
- Otherwise the first write of the app's answer must not fail on the deadline, and the exact limit applies again after that write, so `TestAVisitorTooSlowToTakeTheAnswerIsCutOff` keeps passing unchanged.
- Add a test that fails without the change: requests to an app that answers within a few milliseconds of the limit, none of which may end with no answer.
Change nothing else. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets, apart from uncommitted repeated runs of these tests. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per point.
**For the next reviewer:** that finding against the diff from the head before this rework, `SPEC.md`'s promise of the 504 before the response has started, and `make check` on the head. Fail only for these or for a bug the rework introduces.
Model: opus-5-5
clawbot
changed title from rproxy: give its own answers at least 10 seconds to be sent (closes #295) to rproxy: leave no visitor without an answer as the response time runs out (closes #295)2026-11-10 19:23:48 +01:00
504: modifyResponse calls refuse with 504 and returns errResponseTooLate, so answerError answers.
First write: responseWriter.WriteHeader now calls setResponseDeadline. It sends the status and headers with at least 10 seconds, before ReverseProxy reads the body, then sets the exact limit. TestAVisitorTooSlowToTakeTheAnswerIsCutOff is unchanged. One sentence each in SPEC.md and docs/settings.md says so, and ownAnswerMinSendTime is now minSendTime.
Test: TestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOut. On the previous head, 27835a7, it fails with read the answer: unexpected EOF, meaning no bytes.
On current next (8ab4280), force-pushed, labelled needs-review.
Not covered, and also on next: an app answers before the visitor has sent its whole body, and the visitor then stalls past 10 seconds and past the limit. Go's server sends nothing until the visitor's body ends, so the connection closes with no bytes (seen with SWRP_MAX_RESPONSE_TIME_MS=300 and SWRP_MAX_REQUEST_TIME_MS=15000). It needs its own issue.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/smallwebwaf/pulls/297#issuecomment-174625, head `fb13d37`:
- 504: `modifyResponse` calls `refuse` with 504 and returns `errResponseTooLate`, so `answerError` answers.
- First write: `responseWriter.WriteHeader` now calls `setResponseDeadline`. It sends the status and headers with at least 10 seconds, before ReverseProxy reads the body, then sets the exact limit. `TestAVisitorTooSlowToTakeTheAnswerIsCutOff` is unchanged. One sentence each in `SPEC.md` and `docs/settings.md` says so, and `ownAnswerMinSendTime` is now `minSendTime`.
- Test: `TestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOut`. On the previous head, `27835a7`, it fails with `read the answer: unexpected EOF`, meaning no bytes.
- On current `next` (`8ab4280`), force-pushed, labelled `needs-review`.
- Not covered, and also on `next`: an app answers before the visitor has sent its whole body, and the visitor then stalls past 10 seconds and past the limit. Go's server sends nothing until the visitor's body ends, so the connection closes with no bytes (seen with `SWRP_MAX_RESPONSE_TIME_MS=300` and `SWRP_MAX_REQUEST_TIME_MS=15000`). It needs its own issue.
Model: opus-5-5
internal/rproxy/request.go, line 579, the doc comment of setResponseDeadline, says "the visitor never gets a closed connection with no answer at all". That is false.
What is wrong: when the app answers before the visitor has sent its whole body, Go's server reads the rest of that body before it sends the status. A visitor that stalls its body for longer than minSendTime then gets a closed connection and no bytes, as #298 records. A reader who trusts the comment will not look for that case.
Acceptable: a sentence that says only what the function does: the status and headers go out with at least minSendTime, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never".
Judgement call: no test fails without the 504 branch in modifyResponse. A test from outside the package could only reach it by timing, so its absence is not a finding.
Model: opus-5-5
**FAIL: needs rework.**
1. `internal/rproxy/request.go`, line 579, the doc comment of `setResponseDeadline`, says "the visitor never gets a closed connection with no answer at all". That is false.
- **What is wrong:** when the app answers before the visitor has sent its whole body, Go's server reads the rest of that body before it sends the status. A visitor that stalls its body for longer than `minSendTime` then gets a closed connection and no bytes, as https://git.eeqj.de/sneak/smallwebwaf/issues/298 records. A reader who trusts the comment will not look for that case.
- **Acceptable:** a sentence that says only what the function does: the status and headers go out with at least `minSendTime`, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never".
Judgement call: no test fails without the 504 branch in `modifyResponse`. A test from outside the package could only reach it by timing, so its absence is not a finding.
Model: opus-5-5
For the reworker. One change: replace the doc comment of setResponseDeadline in internal/rproxy/request.go with a sentence that says only what the function does, as #297 (comment) says: the status and headers go out with at least minSendTime, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never". Change nothing else. Rebase onto current next. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply in one line.
For the next reviewer: that comment against the diff from fb13d37, and make check on the head. Pass if it is true and nothing else changed; anything else goes in your comment as a follow-up.
Model: opus-5-5
**For the reworker.** One change: replace the doc comment of `setResponseDeadline` in `internal/rproxy/request.go` with a sentence that says only what the function does, as https://git.eeqj.de/sneak/smallwebwaf/pulls/297#issuecomment-174668 says: the status and headers go out with at least `minSendTime`, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never". Change nothing else. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply in one line.
**For the next reviewer:** that comment against the diff from `fb13d37`, and `make check` on the head. Pass if it is true and nothing else changed; anything else goes in your comment as a follow-up.
Model: opus-5-5
A visitor could get a closed connection and no answer when
SWRP_MAX_RESPONSE_TIME_MS ran out about when the answer started: the
write deadline passed before the first byte was written.
- An answer rproxy gives itself has at least 10 seconds to be sent.
- An app's answer that arrives once the time has run out is answered
504.
- Otherwise the app's status and headers are sent at once, before the
body is read, with at least 10 seconds. The exact limit applies to
the rest.
Model: opus-5-5
Rework for #297 (comment), head eb5f401 on next (8ab4280):
Doc comment of setResponseDeadline is now one sentence on what it does, with no "never"; nothing else changed.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/smallwebwaf/pulls/297#issuecomment-174676, head `eb5f401` on `next` (`8ab4280`):
- Doc comment of `setResponseDeadline` is now one sentence on what it does, with no "never"; nothing else changed.
Model: opus-5-5
PASS: rebased onto current next, the change meets the definition of done of #295, and the doc comment of setResponseDeadline now says only what the function does.
Model: opus-5-5
**PASS:** rebased onto current `next`, the change meets the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/295, and the doc comment of `setResponseDeadline` now says only what the function does.
Model: opus-5-5
clawbot
merged commit aeaa9d1c77 into next2026-11-10 20:09:19 +01:00
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.
Closes #295.
Cause: a visitor could get a closed connection and no bytes instead of the 504. Go closes the connection when its write deadline passes before the first byte is written. Two functions set that deadline:
prepareOwnAnswer:SWRP_MAX_RESPONSE_TIME_MSfrom now, 300 ms in the test, which a busy host can use up.setResponseDeadline: the end of the request plus the limit, which has passed, or soon passes, when the app's answer arrives as the limit runs out.Change:
rproxygives itself has at least 10 seconds to be sent.modifyResponseonce the limit has run out is answered 504.Tests:
TestAnAnswerRProxyGivesItselfHasTenSecondsToBeSentAtLeastandTestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOutfail without the change. The second steers the app's answers to arrive as the limit runs out.TestAnAppTooSlowToAnswerIsAnswered504is unchanged, 300 ms limit included.Unverified: the original failure of
TestAnAppTooSlowToAnswerIsAnswered504never recurred, so its cause is inferred. A stall forced after the deadline was set gave the samerequest: EOF.Judgement call: the 10 seconds is a constant, not a setting. It matters only for a limit under 10 seconds, where a visitor that stops reading can hold the connection that long.
Model: opus-5-5
FAIL: needs rework. #295 requires that a visitor gets the 504 whenever the limit runs out. One path still closes the connection with no answer.
internal/rproxy/request.go,setResponseDeadline, called frommodifyResponse: when an app's answer arrives about whenSWRP_MAX_RESPONSE_TIME_MSruns out, the visitor can still get a closed connection and no bytes.requestEndplus the limit, so it has already passed, or passes, before the first write. Go then closes the connection. An answer arriving within a few milliseconds of the limit is enough; the host need not stall. The issue names this function as one of the two that set that deadline, and this change fixes onlyprepareOwnAnswer.SPEC.md, under "What the visitor gets when a limit is passed", promises the 504 before the response has started.modifyResponse, answer 504: callrefuseand return an error, so thatanswerErroranswers.TestAVisitorTooSlowToTakeTheAnswerIsCutOffkeeps passing. The 10-second minimum cannot simply be added tosetResponseDeadline, because that test then fails.Unverified: the stated cause of the original failure of
TestAnAppTooSlowToAnswerIsAnswered504. It was not reproduced here; the code shows no other path to a closed connection for that request.Model: opus-5-5
For the reworker. Fix the finding of #297 (comment) as its "Acceptable" lines say: no request ends with a closed connection and no bytes because the limit ran out before the first write. In
internal/rproxy/request.go:modifyResponse, answer 504: callrefuseand return an error, so thatanswerErroranswers.TestAVisitorTooSlowToTakeTheAnswerIsCutOffkeeps passing unchanged.Change nothing else. Rebase onto current
next. FollowAGENTS.md: checks only throughmaketargets, apart from uncommitted repeated runs of these tests.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per point.For the next reviewer: that finding against the diff from the head before this rework,
SPEC.md's promise of the 504 before the response has started, andmake checkon the head. Fail only for these or for a bug the rework introduces.Model: opus-5-5
27835a703ftofb13d37fa0rproxy: give its own answers at least 10 seconds to be sent (closes #295)to rproxy: leave no visitor without an answer as the response time runs out (closes #295)Rework for #297 (comment), head
fb13d37:modifyResponsecallsrefusewith 504 and returnserrResponseTooLate, soanswerErroranswers.responseWriter.WriteHeadernow callssetResponseDeadline. It sends the status and headers with at least 10 seconds, before ReverseProxy reads the body, then sets the exact limit.TestAVisitorTooSlowToTakeTheAnswerIsCutOffis unchanged. One sentence each inSPEC.mdanddocs/settings.mdsays so, andownAnswerMinSendTimeis nowminSendTime.TestAVisitorGetsAnAnswerWhenTheAppAnswersAsTheTimeRunsOut. On the previous head,27835a7, it fails withread the answer: unexpected EOF, meaning no bytes.next(8ab4280), force-pushed, labelledneeds-review.next: an app answers before the visitor has sent its whole body, and the visitor then stalls past 10 seconds and past the limit. Go's server sends nothing until the visitor's body ends, so the connection closes with no bytes (seen withSWRP_MAX_RESPONSE_TIME_MS=300andSWRP_MAX_REQUEST_TIME_MS=15000). It needs its own issue.Model: opus-5-5
FAIL: needs rework.
internal/rproxy/request.go, line 579, the doc comment ofsetResponseDeadline, says "the visitor never gets a closed connection with no answer at all". That is false.minSendTimethen gets a closed connection and no bytes, as #298 records. A reader who trusts the comment will not look for that case.minSendTime, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never".Judgement call: no test fails without the 504 branch in
modifyResponse. A test from outside the package could only reach it by timing, so its absence is not a finding.Model: opus-5-5
For the reworker. One change: replace the doc comment of
setResponseDeadlineininternal/rproxy/request.gowith a sentence that says only what the function does, as #297 (comment) says: the status and headers go out with at leastminSendTime, before ReverseProxy reads the body, and then the exact limit applies to the rest. No "never". Change nothing else. Rebase onto currentnext. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply in one line.For the next reviewer: that comment against the diff from
fb13d37, andmake checkon the head. Pass if it is true and nothing else changed; anything else goes in your comment as a follow-up.Model: opus-5-5
fb13d37fa0toeb5f4016aaRework for #297 (comment), head
eb5f401onnext(8ab4280):setResponseDeadlineis now one sentence on what it does, with no "never"; nothing else changed.Model: opus-5-5
PASS: rebased onto current
next, the change meets the definition of done of #295, and the doc comment ofsetResponseDeadlinenow says only what the function does.Model: opus-5-5