Repository navigation
fix: let a refused upload read the bird server's 413 - #22
Merged
Merged
Conversation
The server dropped the connection as soon as its 413 was out, so a client still sending the body could get a reset instead of the answer. It now reads and discards the rest of the body, and drops the connection only past a discard limit.
theycallmeswift
marked this pull request as ready for review
October 16, 2026 14:18
…3-linger # Conflicts: # test/birdsbirdbirds.test.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bird server now answers an oversized upload with 413 and then reads and discards the rest of the body, instead of dropping the connection right away. A client still sending could get a reset instead of the 413. Past four times the PNG cap, the server still drops the connection, so an endless upload can't hold it open.
Decide
fetchread the 413 from the old server 15 of 15 times at 3x and 32x the cap, andcurldid too. The reset shows up with Node's HTTP client, which got it in 13 of 20 tries. Safari is untested. Recommend: merge, because the change is small, covered by tests, and makes the answer reliable for any client, not just Chromium.Ready gate
npm run verify: ok for all three packages.npm run lint: 0 issues in 15 files.npm teston macOS: 194 of 194. BirdsBirdBirds on Linux innode:26: 93 of 93.node:26, with the old server, the finish-sending and discard-limit tests fail in 5 of 5 runs; with the change, the upload tests pass 5 of 5. With the drain but no discard limit, the discard-limit test fails. Both new tests stream over a raw socket, since Node's HTTP client stops sending once it has an answer and hid the race on Linux.refuseOversizedBodyis the only place the server drops a connection; both the Content-Length and streamed paths go through it.server.mjsand posted oversized bodies from the drawing page's origin in the in-app browser: 413 every time at 3x and 32x the cap. No Pi playthrough, since only the upload refusal changed.docs/style/; the comment says which clients the drain is for.