Fixed handling cutoff boundary data in image + media upload

fix https://linear.app/tryghost/issue/SLO-95/unexpected-end-of-multipart-data-for-broken-image-upload-request

- in the event the client sends an invalid body to the image or media
  upload endpoints, Dicer will throw an error if the boundary data is
  malformed
- previously, we've just been bubbling that up as an InternalServerError
  and that results in an HTTP 500
- we can capture errors produced by dicer and return a handled
  BadRequestError, as it's the client's fault
- also includes breaking tests

ghost/core/core/server/web/api/middleware/upload.js

@@ -58,7 +58,8 @@ const single = name => function singleUploadFunction(req, res, next) {
 
     singleUpload(req, res, (err) => {
         if (err) {
-            if (err instanceof multer.MulterError) {
+            // Multer or Dicer errors are usually caused by invalid file uploads
+            if (err instanceof multer.MulterError || err.stack?.includes('dicer')) {
                 return next(new errors.BadRequestError({
                     err
                 }));
@@ -100,7 +101,8 @@ const media = (fileName, thumbName) => function mediaUploadFunction(req, res, ne
 
     mediaUpload(req, res, (err) => {
         if (err) {
-            if (err instanceof multer.MulterError) {
+            // Multer or Dicer errors are usually caused by invalid file uploads
+            if (err instanceof multer.MulterError || err.stack?.includes('dicer')) {
                 return next(new errors.BadRequestError({
                     err
                 }));

ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap

@@ -108,6 +108,24 @@ Object {
 }
 `;
 
+exports[`Images API Errors when image request body is broken #2 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": null,
+      "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": "The request could not be understood.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;
+
 exports[`Images API Errors when image request body is broken 1: [body] 1`] = `
 Object {
   "errors": Array [

ghost/core/test/e2e-api/admin/images.test.js

@@ -344,6 +344,31 @@ describe('Images API', function () {
             });
     });
 
+    it('Errors when image request body is broken #2', async function () {
+        // Manually construct a broken request body
+        const blob = await fetch('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==').then(res => res.blob());
+
+        // Note: this differs from above test by not including the boundary at the end of the payload
+        const brokenPayload = '--boundary\r\nContent-Disposition: form-data; name=\"image\"; filename=\"example.png\"\r\nContent-Type: image/png\r\n';
+
+        // eslint-disable-next-line no-undef
+        const brokenDataBlob = await (new Blob([brokenPayload, blob.slice(0, blob.size / 2)], {
+            type: 'multipart/form-data; boundary=boundary'
+        })).text();
+
+        sinon.stub(logging, 'error');
+        await agent
+            .post('/images/upload/')
+            .header('Content-Type', 'multipart/form-data; boundary=boundary')
+            .body(brokenDataBlob)
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId
+                }]
+            });
+    });
+
     it('Does not return HTTP 500 when image processing fails', async function () {
         sinon.stub(imageTransform, 'resizeFromPath').rejects(new Error('Image processing failed'));
 

ghost/core/test/e2e-api/admin/media.test.js

@@ -152,6 +152,30 @@ describe('Media API', function () {
 
             res.body.errors[0].message.should.match(/The request could not be understood./gi);
         });
+
+        it('Errors when media request body is broken #2', async function () {
+            // Manually construct a broken request body
+
+            // Note: still using png mime type here but it doesn't matter because we're sending an invalid
+            // request body anyway
+            const blob = await fetch('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==').then(res => res.blob());
+
+            // Note: this differs from above test by not including the boundary at the end of the payload
+            const brokenPayload = '--boundary\r\nContent-Disposition: form-data; name=\"image\"; filename=\"example.png\"\r\nContent-Type: image/png\r\n';
+
+            // eslint-disable-next-line no-undef
+            const brokenDataBlob = await (new Blob([brokenPayload, blob.slice(0, blob.size / 2)], {
+                type: 'multipart/form-data; boundary=boundary'
+            })).text();
+
+            sinon.stub(logging, 'error');
+            const res = await request.post(localUtils.API.getApiQuery('media/upload'))
+                .set('Content-Type', 'multipart/form-data; boundary=boundary')
+                .send(brokenDataBlob)
+                .expect(400);
+
+            res.body.errors[0].message.should.match(/The request could not be understood./gi);
+        });
     });
 
     describe('media/thumbnail/upload', function () {