Protected against deleting non-existent image during upload

fix https://linear.app/tryghost/issue/SLO-93/undefined-path-error-with-bad-image-upload

- in the event we receive a request to upload an image, that doesn't
  contain an image, we still try and unlink the files
- this is a dangling promise, so it doesn't cause an explicit HTTP
  error, but it does show up as a console error
- fixed it by checking for the path, and early returning if it doesn't
  exist
- also added a test that would fail without this
This commit is contained in:
Daniel Lockyer 2024-05-02 17:14:11 +02:00 committed by Daniel Lockyer
parent e996213122
commit 7950122ffe
3 changed files with 44 additions and 1 deletions

View File

@ -45,7 +45,13 @@ const messages = {
const enabledClear = config.get('uploadClear') || true;
const upload = multer({dest: os.tmpdir()});
const deleteSingleFile = file => fs.unlink(file.path).catch(err => logging.error(err));
const deleteSingleFile = (file) => {
if (!file.path) {
return;
}
fs.unlink(file.path).catch(err => logging.error(err));
};
const single = name => function singleUploadFunction(req, res, next) {
const singleUpload = upload.single(name);

View File

@ -90,6 +90,24 @@ Object {
}
`;
exports[`Images API Does not try to delete non-existing file upon invalid request 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": "Please select an image.",
"property": null,
"type": "ValidationError",
},
],
}
`;
exports[`Images API Will error when filename is too long 1: [body] 1`] = `
Object {
"errors": Array [

View File

@ -1,6 +1,7 @@
const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework');
const FormData = require('form-data');
const p = require('path');
const fsExtra = require('fs-extra');
const {promises: fs} = require('fs');
const assert = require('assert/strict');
const config = require('../../../core/shared/config');
@ -302,6 +303,24 @@ describe('Images API', function () {
clock.restore();
});
it('Does not try to delete non-existing file upon invalid request', async function () {
sinon.stub(logging, 'error');
const unlinkStub = sinon.stub(fsExtra, 'unlink');
await agent
.post('/images/upload/')
.body(JSON.stringify({}))
.expectStatus(422)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
});
// We didn't upload an image, so we shouldn't have called unlink
sinon.assert.notCalled(unlinkStub);
});
it('Does not return HTTP 500 when image processing fails', async function () {
sinon.stub(imageTransform, 'resizeFromPath').rejects(new Error('Image processing failed'));