From 0435840a0f9f717d010de636a6c5c0792c6572bb Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Fri, 11 Sep 2026 09:17:56 +0200 Subject: [PATCH] fix(upload): revalidate the per-file cap before publishing a chunk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `allowance` is computed before the body arrives, so a chunk that completed while this one was still streaming was not counted in it. Two overlapping 0.75MB chunks under a 1MB cap were therefore both accepted, leaving 1.5MB on disk; enough concurrent streams could go well past the cap before anyone asked to complete the upload. The buffered version got this right for free, because it only ever checked after reading the whole body. The streaming version keeps its pre-read check — that is what makes a too-large request cheap — and asks again with the current aggregate before renaming the staging file into place. Relates to issue 1403 --- .../services/chunkedUploadStreaming.test.js | 18 ++++++++++++++++++ backend/src/services/chunkedUploadService.js | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/backend/__tests__/services/chunkedUploadStreaming.test.js b/backend/__tests__/services/chunkedUploadStreaming.test.js index 8ba4ac24..38fccb51 100644 --- a/backend/__tests__/services/chunkedUploadStreaming.test.js +++ b/backend/__tests__/services/chunkedUploadStreaming.test.js @@ -192,6 +192,24 @@ describe('chunked upload streams the body under a cap (#1403)', () => { .rejects.toMatchObject({ code: 'ENOENT' }); }); + it('counts chunks that landed while another was still streaming', async () => { + const { uploadId } = await init({ totalChunks: 3 }); + + // Start a slow 0.75MB chunk. Its allowance is computed now, when nothing + // else is banked. + const slow = new Readable({ read() {} }); + const slowDone = chunkedUpload.uploadChunk(uploadId, 0, slow); + + // A second 0.75MB chunk completes in the meantime. + await chunkedUpload.uploadChunk(uploadId, 1, Buffer.alloc(0.75 * MB)); + + // Finishing the first must not publish: 1.5MB against a 1MB cap. + slow.push(Buffer.alloc(0.75 * MB)); + slow.push(null); + await expect(slowDone).rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 }); + expect(chunkedUpload.getUploadStatus(uploadId)).toBeNull(); + }); + it('does not destroy the request stream when it trips the cap', async () => { const { uploadId } = await init(); const source = countingSource(8 * MB); diff --git a/backend/src/services/chunkedUploadService.js b/backend/src/services/chunkedUploadService.js index 5e25e606..6b11b4f3 100644 --- a/backend/src/services/chunkedUploadService.js +++ b/backend/src/services/chunkedUploadService.js @@ -266,6 +266,18 @@ async function uploadChunk(uploadId, chunkIndex, source, { declaredBytes } = {}) const partPath = `${chunkPath}.${crypto.randomBytes(6).toString('hex')}.part`; try { chunkLength = await writeChunkStream(source, partPath, allowance); + // Re-check the aggregate before publishing. `allowance` was computed + // before the body arrived, so a chunk that completed while this one was + // still streaming is not counted in it — two overlapping 0.75MB chunks + // under a 1MB cap would otherwise both be accepted. The buffered version + // got this right for free by checking after the read; streaming has to + // ask again. + const bankedNow = totalReceivedBytes(uploadMeta) - (uploadMeta.chunkSizes.get(chunkIndex) || 0); + if (bankedNow + chunkLength > uploadMeta.maxFileSizeBytes) { + await fs.rm(partPath, { force: true }).catch(() => {}); + await abortUpload(uploadId); + throw fileTooLargeError(uploadMeta.maxFileSizeBytes); + } await fs.rename(partPath, chunkPath); } catch (err) { if (err.overAllowance) {