From 6842d599e9822975d1b7258aa01eb0a53fdf4466 Mon Sep 17 00:00:00 2001
From: Daniel Lockyer <hi@daniellockyer.com>
Date: Mon, 11 Mar 2024 21:50:55 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20handling=20of=20image=20?=
 =?UTF-8?q?uploads=20with=20overly=20long=20filenames?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fixes ENG-733
ref https://linear.app/tryghost/issue/ENG-733/handle-image-uploads-where-name-is-too-long

- filesystems usually have a filename length limit; ie. on macOS it is
  255 characters
- if a file is uploaded with a longer filename, we'll return a HTTP 500
- we shouldn't do this as it is user error, so we can just catch the
  error code and return BadRequest
- this implements that, and adds a breaking test
---
 .../adapters/storage/LocalStorageBase.js       | 10 +++++++++-
 .../admin/__snapshots__/images.test.js.snap    | 18 ++++++++++++++++++
 ghost/core/test/e2e-api/admin/images.test.js   | 14 ++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/ghost/core/core/server/adapters/storage/LocalStorageBase.js b/ghost/core/core/server/adapters/storage/LocalStorageBase.js
index 274247fbfa..5fa076b190 100644
--- a/ghost/core/core/server/adapters/storage/LocalStorageBase.js
+++ b/ghost/core/core/server/adapters/storage/LocalStorageBase.js
@@ -58,7 +58,15 @@ class LocalStorageBase extends StorageBase {
         targetFilename = filename;
         await fs.mkdirs(targetDir);
 
-        await fs.copy(file.path, targetFilename);
+        try {
+            await fs.copy(file.path, targetFilename);
+        } catch (err) {
+            if (err.code === 'ENAMETOOLONG') {
+                throw new errors.BadRequestError({err});
+            }
+
+            throw err;
+        }
 
         // The src for the image must be in URI format, not a file system path, which in Windows uses \
         // For local file system storage can use relative path so add a slash
diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap
index 859e570433..2bc56cbaf0 100644
--- a/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap
+++ b/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap
@@ -71,3 +71,21 @@ Object {
   ],
 }
 `;
+
+exports[`Images API Will error when filename is too long 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": "ENAMETOOLONG",
+      "context": "The request could not be understood.",
+      "details": null,
+      "ghostErrorCode": null,
+      "help": null,
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "Request not understood error, cannot upload image.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;
diff --git a/ghost/core/test/e2e-api/admin/images.test.js b/ghost/core/test/e2e-api/admin/images.test.js
index 1df51c5e7e..8d6467e7aa 100644
--- a/ghost/core/test/e2e-api/admin/images.test.js
+++ b/ghost/core/test/e2e-api/admin/images.test.js
@@ -185,6 +185,20 @@ describe('Images API', function () {
         await uploadImageCheck({path: originalFilePath, filename: 'loadingcat_square.gif', contentType: 'image/gif'});
     });
 
+    it('Will error when filename is too long', async function () {
+        const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png');
+        const fileContents = await fs.readFile(originalFilePath);
+        const loggingStub = sinon.stub(logging, 'error');
+        await uploadImageRequest({fileContents, filename: `${'a'.repeat(300)}.png`, contentType: 'image/png'})
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId
+                }]
+            });
+        sinon.assert.calledOnce(loggingStub);
+    });
+
     it('Can not upload a json file', async function () {
         const originalFilePath = p.join(__dirname, '/../../utils/fixtures/data/redirects.json');
         const fileContents = await fs.readFile(originalFilePath);