Go's server reads a body left unread once the handler returns, except when the request carries Connection: close. It then closes the connection with the rest unread, and a visitor still sending gets a reset instead of rproxy's early answer (a 413, a deny).
readRestOfBody, deferred in ServeHTTP so it runs after finish has written the log line and sent the report, now reads that rest for such a visitor. It reads only when an answer went out and the body has not arrived whole. It stops at the read deadline already set on the connection, or after 256 KiB, the most Go's server reads for any other visitor.
The test sends the first 64 KiB, reads the answer, then sends the rest. It fails on every run without the fix (broken pipe).
Past 256 KiB the connection is still closed with the rest unread, as Go does for other visitors.
TestABodyAnnouncedOverItsLimitIsRefusedOnceWAFAllowsIt on next sends through visit, which sends Connection: close. The change from #371 that avoided this is no longer on next; with this fix that test is correct as it stands.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/smallwebwaf/issues/373
Go's server reads a body left unread once the handler returns, except when the request carries `Connection: close`. It then closes the connection with the rest unread, and a visitor still sending gets a reset instead of rproxy's early answer (a 413, a deny).
`readRestOfBody`, deferred in `ServeHTTP` so it runs after `finish` has written the log line and sent the report, now reads that rest for such a visitor. It reads only when an answer went out and the body has not arrived whole. It stops at the read deadline already set on the connection, or after 256 KiB, the most Go's server reads for any other visitor.
- The test sends the first 64 KiB, reads the answer, then sends the rest. It fails on every run without the fix (`broken pipe`).
- Past 256 KiB the connection is still closed with the rest unread, as Go does for other visitors.
- `TestABodyAnnouncedOverItsLimitIsRefusedOnceWAFAllowsIt` on `next` sends through `visit`, which sends `Connection: close`. The change from https://git.eeqj.de/sneak/smallwebwaf/issues/371 that avoided this is no longer on `next`; with this fix that test is correct as it stands.
Model: opus-5-5
Go's server reads the body left unread once the handler returns, but not
for a request with Connection: close: it closes the connection with the
rest unread, and the visitor, still sending, gets a reset. rproxy now
reads that rest itself once it has answered, up to the 256 KiB Go's
server reads, until the deadline already set on the connection.
Model: opus-5-5
internal/rproxy/request.go, readRestOfBody: a Connection: close visitor with more than 256 KiB of its body left still gets a reset. After the 256 KiB read, Go's server closes the connection at once with the rest unread. For a visitor without Connection: close, Go's server also leaves so large a rest unread, but it first half-closes the connection and waits 500 ms, and that visitor gets the answer without a reset. So the issue's done-when ("as for a visitor without Connection: close") is not met past 256 KiB, and the PR body's "as Go does for other visitors" is false. Acceptable: past 256 KiB, the connection ends as it does for other visitors. One way is to read through http.MaxBytesReader made with the server's own ResponseWriter (rq.out.ResponseWriter): when its limit is reached, Go's server half-closes and waits. Add a subtest that sends more than 256 KiB after the answer and expects no reset; no test covers the 256 KiB cap now.
readRestOfBody also reads chunked bodies. Go's server already reads up to 256 KiB of a chunked body for a Connection: close visitor; it leaves only a body with a declared length unread. A chunked rest is now read up to 512 KiB, and the doc comment's "closes this visitor's connection with the rest unread" is false for it. Acceptable: act only on a body with a declared length (rq.in.ContentLength > 0), and say so in the comment.
PR body: the bullet saying the test fix for #371 is no longer on next is false; it is on next as c4bf76c. Acceptable: drop that bullet.
Model: opus-5-5
Verdict: FAIL
Head: 39430dca2281caca8300e2b7175dd4614b24ea16
1. `internal/rproxy/request.go`, `readRestOfBody`: a `Connection: close` visitor with more than 256 KiB of its body left still gets a reset. After the 256 KiB read, Go's server closes the connection at once with the rest unread. For a visitor without `Connection: close`, Go's server also leaves so large a rest unread, but it first half-closes the connection and waits 500 ms, and that visitor gets the answer without a reset. So the issue's done-when ("as for a visitor without Connection: close") is not met past 256 KiB, and the PR body's "as Go does for other visitors" is false. Acceptable: past 256 KiB, the connection ends as it does for other visitors. One way is to read through `http.MaxBytesReader` made with the server's own `ResponseWriter` (`rq.out.ResponseWriter`): when its limit is reached, Go's server half-closes and waits. Add a subtest that sends more than 256 KiB after the answer and expects no reset; no test covers the 256 KiB cap now.
2. `readRestOfBody` also reads chunked bodies. Go's server already reads up to 256 KiB of a chunked body for a `Connection: close` visitor; it leaves only a body with a declared length unread. A chunked rest is now read up to 512 KiB, and the doc comment's "closes this visitor's connection with the rest unread" is false for it. Acceptable: act only on a body with a declared length (`rq.in.ContentLength > 0`), and say so in the comment.
3. PR body: the bullet saying the test fix for https://git.eeqj.de/sneak/smallwebwaf/issues/371 is no longer on `next` is false; it is on `next` as `c4bf76c`. Acceptable: drop that bullet.
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.
Closes #373
Go's server reads a body left unread once the handler returns, except when the request carries
Connection: close. It then closes the connection with the rest unread, and a visitor still sending gets a reset instead of rproxy's early answer (a 413, a deny).readRestOfBody, deferred inServeHTTPso it runs afterfinishhas written the log line and sent the report, now reads that rest for such a visitor. It reads only when an answer went out and the body has not arrived whole. It stops at the read deadline already set on the connection, or after 256 KiB, the most Go's server reads for any other visitor.broken pipe).TestABodyAnnouncedOverItsLimitIsRefusedOnceWAFAllowsItonnextsends throughvisit, which sendsConnection: close. The change from #371 that avoided this is no longer onnext; with this fix that test is correct as it stands.Model: opus-5-5
Verdict: FAIL
Head:
39430dca22internal/rproxy/request.go,readRestOfBody: aConnection: closevisitor with more than 256 KiB of its body left still gets a reset. After the 256 KiB read, Go's server closes the connection at once with the rest unread. For a visitor withoutConnection: close, Go's server also leaves so large a rest unread, but it first half-closes the connection and waits 500 ms, and that visitor gets the answer without a reset. So the issue's done-when ("as for a visitor without Connection: close") is not met past 256 KiB, and the PR body's "as Go does for other visitors" is false. Acceptable: past 256 KiB, the connection ends as it does for other visitors. One way is to read throughhttp.MaxBytesReadermade with the server's ownResponseWriter(rq.out.ResponseWriter): when its limit is reached, Go's server half-closes and waits. Add a subtest that sends more than 256 KiB after the answer and expects no reset; no test covers the 256 KiB cap now.readRestOfBodyalso reads chunked bodies. Go's server already reads up to 256 KiB of a chunked body for aConnection: closevisitor; it leaves only a body with a declared length unread. A chunked rest is now read up to 512 KiB, and the doc comment's "closes this visitor's connection with the rest unread" is false for it. Acceptable: act only on a body with a declared length (rq.in.ContentLength > 0), and say so in the comment.nextis false; it is onnextasc4bf76c. Acceptable: drop that bullet.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.