From 1e80eb4ef4d1b9fa69d00d26fa0560c652fc418e Mon Sep 17 00:00:00 2001 From: Swift Date: Fri, 16 Oct 2026 10:17:43 -0400 Subject: [PATCH] fix: let a refused upload read the bird server's 413 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. --- challenges/birdsbirdbirds/server.mjs | 20 +++++++++-- test/birdsbirdbirds.test.mjs | 51 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/challenges/birdsbirdbirds/server.mjs b/challenges/birdsbirdbirds/server.mjs index 1f8a4bf..d046b5d 100644 --- a/challenges/birdsbirdbirds/server.mjs +++ b/challenges/birdsbirdbirds/server.mjs @@ -20,6 +20,9 @@ export const DEFAULT_PORT = 4322 export const MAX_PNG_BYTES = 2 * 1024 * 1024 export const PORT_BUSY_EXIT_CODE = 3 +/** How much of a refused upload the server reads and discards before it drops the connection. */ +const MAX_DISCARDED_BYTES = 4 * MAX_PNG_BYTES + const LISTEN_HOST = '127.0.0.1' const PAGE_DIRECTORY = path.join(path.dirname(fileURLToPath(import.meta.url)), 'page') const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) @@ -157,10 +160,21 @@ async function saveBird({ request, response, workspace, port }) { send(response, 204) } -/** Answers 413 and drops the connection once the answer is out, so the rest of the body is never read. */ +/** + * Answers 413, then reads and discards the rest of the body, so the client + * can finish sending and read the answer. Closing on a client that is still + * sending can make it see a reset instead, unless it reads answers that + * arrive mid-upload as Chromium does. Drops the connection once more than + * MAX_DISCARDED_BYTES arrive, so an endless upload can't hold it open. + */ function refuseOversizedBody(request, response) { - response.setHeader('Connection', 'close') - response.on('finish', () => request.destroy()) + let discardedBytes = 0 + request.on('data', (chunk) => { + discardedBytes += chunk.length + if (discardedBytes > MAX_DISCARDED_BYTES) request.destroy() + }) + // readBodyWithin pauses the body when it gives up; a paused stream stays paused when a listener is added. + request.resume() sendText(response, 413, `the image is over ${MAX_PNG_BYTES} bytes`) } diff --git a/test/birdsbirdbirds.test.mjs b/test/birdsbirdbirds.test.mjs index 2e5754d..21fc1f1 100644 --- a/test/birdsbirdbirds.test.mjs +++ b/test/birdsbirdbirds.test.mjs @@ -465,6 +465,37 @@ const sendRequest = ({ port, method = 'GET', requestPath = '/', headers = {}, bo request.end(body) }) +/** + * Streams a chunked PNG upload over a raw socket until the server closes the + * connection or `maxBytes` have gone out, and resolves `{ status, dropped }`. + * `dropped` is true when the connection closed before the upload finished. + * A raw socket, since Node's HTTP client stops sending once it has a whole + * answer, and a browser doesn't. + */ +const streamUntilDropped = ({ port, maxBytes }) => new Promise((resolve) => { + const chunk = Buffer.alloc(64 * 1024) + const chunkFrame = Buffer.concat([Buffer.from(`${chunk.length.toString(16)}\r\n`), chunk, Buffer.from('\r\n')]) + const socket = net.connect(port, '127.0.0.1') + let received = '' + let sentBytes = 0 + + /** Writes chunks until the socket pushes back or the cap is reached, then ends the body. */ + const writeMore = () => { + while (sentBytes < maxBytes) { + sentBytes += chunk.length + if (!socket.write(chunkFrame)) return socket.once('drain', writeMore) + } + socket.end('0\r\n\r\n') + } + + socket.setEncoding('latin1') + socket.on('data', (text) => { received += text }) + socket.on('error', () => undefined) + socket.on('close', () => resolve({ status: Number(/^HTTP\/1\.1 (\d{3})/.exec(received)?.[1]), dropped: !socket.writableFinished })) + socket.write(`POST /bird.png HTTP/1.1\r\nHost: 127.0.0.1:${port}\r\nContent-Type: image/png\r\nTransfer-Encoding: chunked\r\n\r\n`) + writeMore() +}) + /** POSTs a PNG body to /bird.png; `headers` replace only the defaults a test is about. */ const postBird = ({ port, body = TINY_PNG, headers = {}, requestPath = '/bird.png' }) => sendRequest({ port, @@ -512,6 +543,26 @@ test('the server refuses a streamed body once it passes the cap, with no Content assert.deepEqual(await readdir(workspace), []) })) +test('the server lets an oversized upload finish sending and answers 413', (testContext) => withWorkspace(async ({ workspace }) => { + const port = await startBirdServer(testContext, workspace) + const oversized = Buffer.concat([TINY_PNG, Buffer.alloc(3 * MAX_PNG_BYTES)]) + + const response = await postBird({ port, body: oversized }) + + assert.equal(response.status, 413) + assert.deepEqual(await readdir(workspace), []) +})) + +test('the server drops an oversized upload that keeps sending past the discard limit', (testContext) => withWorkspace(async ({ workspace }) => { + const port = await startBirdServer(testContext, workspace) + + const upload = await streamUntilDropped({ port, maxBytes: 64 * MAX_PNG_BYTES }) + + assert.equal(upload.status, 413) + assert.equal(upload.dropped, true) + assert.deepEqual(await readdir(workspace), []) +})) + const REFUSED_UPLOADS = [ { behavior: 'a body that is not a PNG with 415', upload: { body: Buffer.from('GIF89a not a png at all') }, status: 415 }, { behavior: 'a PNG signature without an IHDR chunk with 415', upload: { body: Buffer.concat([PNG_SIGNATURE, pngChunk('IEND', Buffer.alloc(0))]) }, status: 415 },