From 18bc1b77cb8be3fbb7e9a35ff2cdf1652db187d9 Mon Sep 17 00:00:00 2001
From: Paul Nothaft <paul@MacStudio-von-Paul.local>
Date: Fri, 11 Sep 2026 10:22:22 +0200
Subject: [PATCH] fix(upload): retire the connection after an early refusal,
 clean up a failed publish
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two more from review.

Refusing a body before reading it is the point of the streaming cap, but the
unread bytes are still in flight on a connection the response advertises as
keep-alive. Node does not drain them, so the NEXT request on that socket hangs
until it times out — reproducible with an 8MB body against a 1MB cap. The
error response now sets Connection: close whenever the request was not read to
the end.

A failed rename — ENOSPC, a vanished directory — left the fully written
staging file behind. Staging names are per-attempt, so a client that retries
instead of aborting accumulates one per try until the upload expires. The
partial is removed on that path too.

Relates to issue 1403
---
 .../services/chunkedUploadStreaming.test.js       | 15 +++++++++++++++
 backend/src/routes/adminPhotos.js                 |  7 +++++++
 backend/src/services/chunkedUploadService.js      |  9 ++++++++-
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/backend/__tests__/services/chunkedUploadStreaming.test.js b/backend/__tests__/services/chunkedUploadStreaming.test.js
index ad3fb40d..6bb3f130 100644
--- a/backend/__tests__/services/chunkedUploadStreaming.test.js
+++ b/backend/__tests__/services/chunkedUploadStreaming.test.js
@@ -210,6 +210,21 @@ describe('chunked upload streams the body under a cap (#1403)', () => {
       expect(chunkedUpload.getUploadStatus(uploadId)).toBeNull();
     });
 
+    it('removes the staging file when publishing it fails', async () => {
+      const { uploadId } = await init();
+      const dir = path.join(process.env.STORAGE_PATH, 'chunks', uploadId);
+      // Make the rename fail by putting a directory where the chunk goes.
+      await fs.mkdir(path.join(dir, 'chunk_000000'), { recursive: true });
+
+      await expect(chunkedUpload.uploadChunk(uploadId, 0, countingSource(1024)))
+        .rejects.toThrow();
+
+      // The fully written .part must not survive a failed publish — its name is
+      // per-attempt, so retries would otherwise pile them up until expiry.
+      const leftovers = (await fs.readdir(dir)).filter((f) => f.endsWith('.part'));
+      expect(leftovers).toEqual([]);
+    });
+
     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/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js
index a9557b5d..735a6bbe 100644
--- a/backend/src/routes/adminPhotos.js
+++ b/backend/src/routes/adminPhotos.js
@@ -1718,6 +1718,13 @@ router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, r
     // large) carry their own status. Only a genuinely unexpected error should
     // reach the 500 below and the error log with it.
     if (error.statusCode) {
+      // Refusing the body early is the point — but it leaves unread bytes in
+      // flight on a connection this response still advertises as keep-alive.
+      // Node does not drain them, so the NEXT request on that socket hangs
+      // until it times out. Retire the connection instead.
+      if (!req.readableEnded) {
+        res.set('Connection', 'close');
+      }
       return res.status(error.statusCode).json({ error: error.message });
     }
     logger.error('Error uploading chunk:', error);
diff --git a/backend/src/services/chunkedUploadService.js b/backend/src/services/chunkedUploadService.js
index 3ca14df1..ab5f59f9 100644
--- a/backend/src/services/chunkedUploadService.js
+++ b/backend/src/services/chunkedUploadService.js
@@ -289,7 +289,14 @@ async function uploadChunk(uploadId, chunkIndex, source, { declaredBytes } = {})
         await abortUpload(uploadId);
         throw fileTooLargeError(uploadMeta.maxFileSizeBytes);
       }
-      await fs.rename(partPath, chunkPath);
+      await fs.rename(partPath, chunkPath).catch(async (renameErr) => {
+        // A failed publish (ENOSPC, a vanished directory) left the fully
+        // written staging file behind. Its name is per-attempt, so a client
+        // that retries instead of aborting just accumulates more of them until
+        // the upload expires.
+        await fs.rm(partPath, { force: true }).catch(() => {});
+        throw renameErr;
+      });
     } catch (err) {
       if (err.overAllowance) {
         await abortUpload(uploadId);