diff --git a/server/libs/expressFileupload/processMultipart.js b/server/libs/expressFileupload/processMultipart.js index 32fb690a7..317194ebf 100644 --- a/server/libs/expressFileupload/processMultipart.js +++ b/server/libs/expressFileupload/processMultipart.js @@ -26,6 +26,18 @@ const waitFlushProperty = Symbol('wait flush property symbol'); */ module.exports = (options, req, res, next) => { req.files = null; + const pendingCleanups = []; + const runPendingCleanups = () => { + while (pendingCleanups.length) { + const cleanupFn = pendingCleanups.pop(); + try { + cleanupFn(); + } catch (err) { + debugLog(options, `Temp file cleanup failed: ${err}`); + } + } + }; + res.once('close', runPendingCleanups); // Build busboy options and init busboy instance. const busboyOptions = buildOptions(options, { headers: req.headers }); @@ -62,6 +74,7 @@ module.exports = (options, req, res, next) => { } = options.useTempFiles ? tempFileHandler(options, field, filename) // Upload into temporary file. : memHandler(options, field, filename); // Upload into RAM. + pendingCleanups.push(cleanup); const writePromise = options.useTempFiles ? getWritePromise().catch(err => { @@ -161,6 +174,7 @@ module.exports = (options, req, res, next) => { busboy.on('error', (err) => { debugLog(options, `Busboy error`); + runPendingCleanups(); next(err); }); diff --git a/server/libs/expressFileupload/tempFileHandler.js b/server/libs/expressFileupload/tempFileHandler.js index bfaca8b4f..473a62179 100644 --- a/server/libs/expressFileupload/tempFileHandler.js +++ b/server/libs/expressFileupload/tempFileHandler.js @@ -28,6 +28,18 @@ module.exports = (options, fieldname, filename) => { reject(err); }); }); + // cleanup() can run from an error path and again on response close. + let cleanedUp = false; + + // Windows cannot unlink while the write stream still holds the file. + const closeWriteStream = () => new Promise((resolve) => { + if (writeStream.destroyed || writeStream.closed) { + return resolve(); + } + writeStream.once('close', resolve); + writeStream.once('error', resolve); + writeStream.end(); + }); return { dataHandler: (data) => { @@ -51,13 +63,21 @@ module.exports = (options, fieldname, filename) => { return Buffer.concat([]); }, cleanup: () => { + if (cleanedUp) return Promise.resolve(); + cleanedUp = true; completed = true; debugLog(options, `Cleaning up temporary file ${tempFilePath}...`); - writeStream.end(); - deleteFile(tempFilePath, err => (err - ? debugLog(options, `Cleaning up temporary file ${tempFilePath} failed: ${err}`) - : debugLog(options, `Cleaning up temporary file ${tempFilePath} done.`) - )); + return closeWriteStream().then(() => new Promise((resolve) => { + // ENOENT: handler already mv()'d the file; that is success. + deleteFile(tempFilePath, err => { + if (err && err.code !== 'ENOENT') { + debugLog(options, `Cleaning up temporary file ${tempFilePath} failed: ${err}`); + } else { + debugLog(options, `Cleaning up temporary file ${tempFilePath} done.`); + } + resolve(); + }); + })); }, getWritePromise: () => writePromise };