diff --git a/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js b/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js index f6a96bfd..212d1b7e 100644 --- a/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js +++ b/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js @@ -11,6 +11,8 @@ * - a file over the configured cap is rejected with a 400 naming the limit * - the chunked-upload init route honours the same cap (it would otherwise * be a trivial bypass of the multipart route's cap) + * - the chunk route enforces the cap on the bytes actually received, so a + * client can't declare `fileSize: 1` at init and stream past the limit * - a file under the cap still gets past the size gate * - the limit is read per request, so an admin raising it takes effect */ @@ -127,6 +129,20 @@ describe('admin upload per-file size limit (general_max_file_size_mb)', () => { expect(res.body.error).toBe('File too large. Maximum size is 1 MB per file.'); }); + it('rejects chunk bytes over the limit regardless of the declared fileSize', async () => { + await setLimitMb(1); + const initRes = await postChunkedInit(1); + expect(initRes.status).toBe(200); + + const res = await request(app) + .post(`/api/admin/photos/${eventId}/chunked-upload/${initRes.body.uploadId}/chunk/0`) + .set('Authorization', `Bearer ${adminToken}`) + .set('Content-Type', 'application/octet-stream') + .send(Buffer.alloc(2 * 1024 * 1024, 0x41)); + expect(res.status).toBe(413); + expect(res.body.error).toBe('File too large. Maximum size is 1 MB per file.'); + }); + it('lets a file under the limit past the size gate', async () => { await setLimitMb(1); // Junk bytes, so it still fails downstream on the content check — that is diff --git a/backend/__tests__/services/chunkedUploadSizeCap.test.js b/backend/__tests__/services/chunkedUploadSizeCap.test.js new file mode 100644 index 00000000..7b8a32b2 --- /dev/null +++ b/backend/__tests__/services/chunkedUploadSizeCap.test.js @@ -0,0 +1,80 @@ +/** + * The chunked-upload per-file cap has to hold on the bytes actually received, + * not on the client-declared `fileSize` the init route validates. Declaring + * `fileSize: 1` and then streaming 10 GB through the chunk route was a + * complete bypass of general_max_file_size_mb; the merge step only logged a + * size mismatch and processed the file anyway. + */ +const path = require('path'); +const os = require('os'); +const fs = require('fs').promises; + +process.env.STORAGE_PATH = path.join(os.tmpdir(), `picpeak-chunk-cap-test-${process.pid}`); + +const chunkedUpload = require('../../src/services/chunkedUploadService'); + +const MB = 1024 * 1024; + +const init = (overrides = {}) => chunkedUpload.initializeUpload({ + filename: 'clip.mp4', + fileSize: 1, + mimeType: 'video/mp4', + eventId: 1, + totalChunks: 2, + maxFileSizeBytes: 1 * MB, + ...overrides, +}); + +describe('chunkedUploadService per-file size cap', () => { + afterAll(async () => { + await fs.rm(process.env.STORAGE_PATH, { recursive: true, force: true }).catch(() => {}); + }); + + it('rejects a single chunk over the cap even when the declared fileSize is tiny', async () => { + const { uploadId } = await init(); + await expect(chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(2 * MB))) + .rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 }); + // Aborted, not merely rejected: the upload can no longer be completed. + expect(chunkedUpload.getUploadStatus(uploadId)).toBeNull(); + }); + + it('rejects when the running total across chunks crosses the cap', async () => { + const { uploadId } = await init(); + await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(0.75 * MB)); + await expect(chunkedUpload.uploadChunk(uploadId, 1, Buffer.alloc(0.5 * MB))) + .rejects.toMatchObject({ code: 'FILE_TOO_LARGE' }); + }); + + it('counts a re-sent chunk once, not twice', async () => { + const { uploadId } = await init(); + await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(0.6 * MB)); + // Same index again — replaces the earlier bytes, so the total stays 0.6 MB. + await expect(chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(0.6 * MB))).resolves.toBeTruthy(); + await expect(chunkedUpload.uploadChunk(uploadId, 1, Buffer.alloc(0.3 * MB))).resolves.toBeTruthy(); + }); + + it('rejects chunk indices outside the announced range', async () => { + const { uploadId } = await init(); + await expect(chunkedUpload.uploadChunk(uploadId, 2, Buffer.alloc(10))) + .rejects.toMatchObject({ code: 'INVALID_CHUNK', statusCode: 400 }); + await expect(chunkedUpload.uploadChunk(uploadId, -1, Buffer.alloc(10))) + .rejects.toMatchObject({ code: 'INVALID_CHUNK' }); + await expect(chunkedUpload.uploadChunk(uploadId, NaN, Buffer.alloc(10))) + .rejects.toMatchObject({ code: 'INVALID_CHUNK' }); + }); + + it('merges an upload under the cap and reports the real size', async () => { + const { uploadId } = await init(); + await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(400 * 1024, 0x41)); + await chunkedUpload.uploadChunk(uploadId, 1, Buffer.alloc(400 * 1024, 0x42)); + const merged = await chunkedUpload.completeUpload(uploadId); + expect(merged.size).toBe(800 * 1024); + await fs.rm(merged.tempDir, { recursive: true, force: true }); + }); + + it('applies no cap when none is given', async () => { + const { uploadId } = await init({ maxFileSizeBytes: undefined, totalChunks: 1 }); + await expect(chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(3 * MB))).resolves.toBeTruthy(); + await chunkedUpload.abortUpload(uploadId); + }); +}); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 11d62298..e27451b4 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -1623,7 +1623,10 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo fileSize, mimeType, eventId: parseInt(eventId), - totalChunks + totalChunks, + // The declared fileSize check above is client-controlled; the service + // enforces this cap on the bytes it actually receives and merges. + maxFileSizeBytes: maxSize }); res.json(result); @@ -1648,6 +1651,9 @@ router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, r res.json(result); } catch (error) { + if (error.statusCode === 413 || error.statusCode === 400) { + return res.status(error.statusCode).json({ error: error.message }); + } logger.error('Error uploading chunk:', error); res.status(500).json({ error: error.message || 'Failed to upload chunk' }); } @@ -1690,6 +1696,9 @@ router.post('/:eventId/chunked-upload/:uploadId/complete', adminAuth, requirePer photos: uploadedPhotos }); } catch (error) { + if (error.statusCode === 413) { + return res.status(413).json({ error: error.message }); + } logger.error('Error completing chunked upload:', error); res.status(500).json({ error: error.message || 'Failed to complete upload' }); } diff --git a/backend/src/services/chunkedUploadService.js b/backend/src/services/chunkedUploadService.js index 20959f43..d0fc4711 100644 --- a/backend/src/services/chunkedUploadService.js +++ b/backend/src/services/chunkedUploadService.js @@ -16,6 +16,27 @@ const CHUNK_SIZE = 10 * 1024 * 1024; // Upload expiration: 24 hours const UPLOAD_EXPIRATION_MS = 24 * 60 * 60 * 1000; +function totalReceivedBytes(uploadMeta) { + let total = 0; + for (const size of uploadMeta.chunkSizes.values()) total += size; + return total; +} + +// Tagged errors so the routes can answer 413/400 instead of a blanket 500. +function fileTooLargeError(maxFileSizeBytes) { + const err = new Error(`File too large. Maximum size is ${Math.floor(maxFileSizeBytes / (1024 * 1024))} MB per file.`); + err.code = 'FILE_TOO_LARGE'; + err.statusCode = 413; + return err; +} + +function invalidChunkError(message) { + const err = new Error(message); + err.code = 'INVALID_CHUNK'; + err.statusCode = 400; + return err; +} + /** * Initialize a new chunked upload * @param {Object} options - Upload options @@ -27,7 +48,8 @@ async function initializeUpload(options) { fileSize, mimeType, eventId, - totalChunks + totalChunks, + maxFileSizeBytes } = options; // Strip any directory components from the client-supplied filename. It is @@ -50,6 +72,13 @@ async function initializeUpload(options) { // Calculate expected chunks const expectedChunks = totalChunks || Math.ceil(fileSize / CHUNK_SIZE); + // The per-file cap is enforced on the BYTES ACTUALLY RECEIVED, not on the + // client-declared fileSize the init route checks: a client can declare + // `fileSize: 1` and then stream whatever it likes through the chunk route. + // Missing/invalid cap means "no cap" (callers outside the admin routes). + const cap = Number(maxFileSizeBytes); + const sizeCap = Number.isFinite(cap) && cap > 0 ? cap : Infinity; + // Store upload metadata const uploadMeta = { uploadId, @@ -59,6 +88,9 @@ async function initializeUpload(options) { eventId, expectedChunks, receivedChunks: new Set(), + // Bytes per chunk index, so a re-sent chunk replaces rather than adds. + chunkSizes: new Map(), + maxFileSizeBytes: sizeCap, uploadDir, createdAt: Date.now(), expiresAt: Date.now() + UPLOAD_EXPIRATION_MS, @@ -107,12 +139,28 @@ async function uploadChunk(uploadId, chunkIndex, chunkData) { throw new Error('Upload expired'); } + // Only the announced chunk indices are valid — anything else would merge + // into nothing (a gap) or let more chunks in than the declared file has. + if (!Number.isInteger(chunkIndex) || chunkIndex < 0 || chunkIndex >= uploadMeta.expectedChunks) { + throw invalidChunkError(`Invalid chunk index ${chunkIndex}: expected 0-${uploadMeta.expectedChunks - 1}`); + } + + // Enforce the per-file cap on the running byte total. The upload is + // aborted, not just rejected: the chunks on disk are already over the + // limit and the client can't complete the file any more. + const receivedBytes = totalReceivedBytes(uploadMeta) - (uploadMeta.chunkSizes.get(chunkIndex) || 0) + chunkData.length; + if (receivedBytes > uploadMeta.maxFileSizeBytes) { + await abortUpload(uploadId); + throw fileTooLargeError(uploadMeta.maxFileSizeBytes); + } + // Write chunk to disk const chunkPath = path.join(uploadMeta.uploadDir, `chunk_${String(chunkIndex).padStart(6, '0')}`); await fs.writeFile(chunkPath, chunkData); // Mark chunk as received uploadMeta.receivedChunks.add(chunkIndex); + uploadMeta.chunkSizes.set(chunkIndex, chunkData.length); const progress = (uploadMeta.receivedChunks.size / uploadMeta.expectedChunks) * 100; @@ -184,6 +232,15 @@ async function completeUpload(uploadId) { }); } + // Backstop for the per-chunk running total above: the merged file is + // the number that matters, so it is the number that is checked last. + if (stats.size > uploadMeta.maxFileSizeBytes) { + await fs.rm(tempDir, { recursive: true, force: true }).catch(() => {}); + await fs.rm(uploadMeta.uploadDir, { recursive: true, force: true }).catch(() => {}); + activeUploads.delete(uploadId); + throw fileTooLargeError(uploadMeta.maxFileSizeBytes); + } + // Clean up chunks await fs.rm(uploadMeta.uploadDir, { recursive: true, force: true });