From 71022a715b50f6d9aa0e4cea012588b3de106840 Mon Sep 17 00:00:00 2001 From: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com> Date: Tue, 12 May 2026 08:11:02 +0530 Subject: [PATCH 1/2] fix: remove partial disk file on aborted uploads --- lib/make-middleware.js | 23 +++++++++++- storage/disk.js | 3 ++ test/error-handling.js | 85 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) diff --git a/lib/make-middleware.js b/lib/make-middleware.js index ee509886..483a77e1 100644 --- a/lib/make-middleware.js +++ b/lib/make-middleware.js @@ -35,6 +35,7 @@ function makeMiddleware (setup) { var errorOccured = false var pendingWrites = new Counter() var uploadedFiles = [] + var filesInProgress = [] function done (err) { var called = false @@ -73,16 +74,27 @@ function makeMiddleware (setup) { if (readFinished && pendingWrites.isZero() && !errorOccured) done() } + function removePendingFile (file) { + var fileIndex = filesInProgress.indexOf(file) + if (~fileIndex) { + filesInProgress.splice(fileIndex, 1) + } + } + function abortWithError (uploadError, skipPendingWait) { if (errorOccured) return errorOccured = true function finishAbort () { + var filesToCleanup = uploadedFiles.concat(filesInProgress) + uploadedFiles = [] + filesInProgress = [] + function remove (file, cb) { storage._removeFile(req, file, cb) } - removeUploadedFiles(uploadedFiles, remove, function (err, storageErrors) { + removeUploadedFiles(filesToCleanup, remove, function (err, storageErrors) { if (err) return done(err) uploadError.storageErrors = storageErrors @@ -199,6 +211,7 @@ function makeMiddleware (setup) { var aborting = false pendingWritesIncremented = true pendingWrites.increment() + filesInProgress.push(file) Object.defineProperty(file, 'stream', { configurable: true, @@ -212,6 +225,13 @@ function makeMiddleware (setup) { }) storage._handleFile(req, file, function (err, info) { + removePendingFile(file) + + if (errorOccured) { + appender.removePlaceholder(placeholder) + return pendingWrites.decrement() + } + if (aborting) { appender.removePlaceholder(placeholder) uploadedFiles.push({ ...file, ...info }) @@ -220,6 +240,7 @@ function makeMiddleware (setup) { if (err) { appender.removePlaceholder(placeholder) + uploadedFiles.push(file) pendingWrites.decrement() return abortWithError(err) } diff --git a/storage/disk.js b/storage/disk.js index ae1f7bb9..f5149161 100644 --- a/storage/disk.js +++ b/storage/disk.js @@ -34,6 +34,9 @@ DiskStorage.prototype._handleFile = function _handleFile (req, file, cb) { if (err) return cb(err) var finalPath = path.join(destination, filename) + file.destination = destination + file.filename = filename + file.path = finalPath var outStream = fs.createWriteStream(finalPath) file.stream.pipe(outStream) diff --git a/test/error-handling.js b/test/error-handling.js index 597e8039..8bdc9eb3 100644 --- a/test/error-handling.js +++ b/test/error-handling.js @@ -2,7 +2,10 @@ var assert = require('assert') +var fs = require('fs') var os = require('os') +var temp = require('fs-temp') +var rimraf = require('rimraf') var util = require('./_util') var multer = require('../') var removeUploadedFiles = require('../lib/remove-uploaded-files') @@ -460,3 +463,85 @@ describe('Error Handling', function () { }) }) }) + +it('should remove uploaded files if request is aborted before completion', function (done) { + this.timeout(5000) + + temp.mkdir(function (err, uploadDir) { + if (err) return done(err) + + var upload = multer({ storage: multer.diskStorage({ destination: uploadDir }) }).single('file') + var server = http.createServer(function (req, res) { + upload(req, res, function (err) { + assert(err) + var files = fs.readdirSync(uploadDir) + assert.strictEqual(files.length, 0) + clearTimeout(timer) + server.close(function () { + rimraf(uploadDir, function (cleanupErr) { + done(cleanupErr) + }) + }) + }) + }) + + var timer = setTimeout(function () { + server.close(function () { + rimraf(uploadDir, function (cleanupErr) { + done(cleanupErr || new Error('middleware did not complete after client abort')) + }) + }) + }, 2000) + + server.listen(0, function () { + var port = server.address().port + var boundary = 'Abort' + Date.now() + var preamble = [ + '--' + boundary, + 'Content-Disposition: form-data; name="file"; filename="test.bin"', + 'Content-Type: application/octet-stream', + '', + '' + ].join('\r\n') + var footer = '\r\n--' + boundary + '--\r\n' + var chunk = Buffer.alloc(32 * 1024, 97) + var totalChunks = 5 + var contentLength = Buffer.byteLength(preamble) + + (chunk.length * totalChunks) + + Buffer.byteLength(footer) + + var sock = new net.Socket() + var sentChunks = 0 + function writeChunk () { + if (sentChunks >= 3) { + sock.destroy() + return + } + + sentChunks += 1 + var canContinue = sock.write(chunk) + if (canContinue) { + setTimeout(writeChunk, 2) + } else { + sock.once('drain', function () { + setTimeout(writeChunk, 2) + }) + } + } + + sock.connect(port, '127.0.0.1', function () { + sock.write( + 'POST / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + 'Connection: close\r\n' + + 'Content-Type: multipart/form-data; boundary=' + boundary + '\r\n' + + 'Content-Length: ' + contentLength + '\r\n\r\n' + ) + sock.write(preamble) + writeChunk() + }) + + sock.on('error', function () {}) + }) + }) +}) From fc39d3b49f0bec8796c2e772fcb9b1e3f8d68acc Mon Sep 17 00:00:00 2001 From: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com> Date: Sat, 30 May 2026 14:34:57 +0530 Subject: [PATCH 2/2] Fix error cleanup for files started before abort --- lib/make-middleware.js | 6 ++++-- test/_util.js | 10 +++++++++- test/disk-storage.js | 29 +++++++++++++++++------------ test/functionality.js | 9 ++++++--- test/memory-storage.js | 29 +++++++++++++++++------------ test/reuse-middleware.js | 5 +++-- 6 files changed, 56 insertions(+), 32 deletions(-) diff --git a/lib/make-middleware.js b/lib/make-middleware.js index 483a77e1..8fbf252b 100644 --- a/lib/make-middleware.js +++ b/lib/make-middleware.js @@ -36,6 +36,7 @@ function makeMiddleware (setup) { var pendingWrites = new Counter() var uploadedFiles = [] var filesInProgress = [] + var filesToCleanup = [] function done (err) { var called = false @@ -84,9 +85,10 @@ function makeMiddleware (setup) { function abortWithError (uploadError, skipPendingWait) { if (errorOccured) return errorOccured = true + filesToCleanup = uploadedFiles.concat(filesInProgress) function finishAbort () { - var filesToCleanup = uploadedFiles.concat(filesInProgress) + var files = filesToCleanup.concat() uploadedFiles = [] filesInProgress = [] @@ -94,7 +96,7 @@ function makeMiddleware (setup) { storage._removeFile(req, file, cb) } - removeUploadedFiles(filesToCleanup, remove, function (err, storageErrors) { + removeUploadedFiles(files, remove, function (err, storageErrors) { if (err) return done(err) uploadError.storageErrors = storageErrors diff --git a/test/_util.js b/test/_util.js index a6782ad8..52694bb0 100644 --- a/test/_util.js +++ b/test/_util.js @@ -2,14 +2,22 @@ var fs = require('fs') var path = require('path') var stream = require('stream') +function filePath (name) { + return path.join(__dirname, 'files', name) +} + exports.file = function file (name) { - return fs.createReadStream(path.join(__dirname, 'files', name)) + return fs.createReadStream(filePath(name)) } exports.fileSize = function fileSize (path) { return fs.statSync(path).size } +exports.fixtureSize = function fixtureSize (name) { + return exports.fileSize(filePath(name)) +} + exports.submitForm = function submitForm (multer, form, cb) { form.getLength(function (err, length) { if (err) return cb(err) diff --git a/test/disk-storage.js b/test/disk-storage.js index bc923ab5..4867364f 100644 --- a/test/disk-storage.js +++ b/test/disk-storage.js @@ -10,6 +10,11 @@ var multer = require('../') var temp = require('fs-temp') var rimraf = require('rimraf') var FormData = require('form-data') +var small0Size = util.fixtureSize('small0.dat') +var small1Size = util.fixtureSize('small1.dat') +var tiny0Size = util.fixtureSize('tiny0.dat') +var mediumSize = util.fixtureSize('medium.dat') +var largeSize = util.fixtureSize('large.jpg') describe('Disk Storage', function () { var uploadDir, upload @@ -42,8 +47,8 @@ describe('Disk Storage', function () { assert.strictEqual(req.file.fieldname, 'small0') assert.strictEqual(req.file.originalname, 'small0.dat') - assert.strictEqual(req.file.size, 1778) - assert.strictEqual(util.fileSize(req.file.path), 1778) + assert.strictEqual(req.file.size, small0Size) + assert.strictEqual(util.fileSize(req.file.path), small0Size) done() }) @@ -116,8 +121,8 @@ describe('Disk Storage', function () { assert.strictEqual(req.files.tiny0[0].fieldname, 'tiny0') assert.strictEqual(req.files.tiny0[0].originalname, 'tiny0.dat') - assert.strictEqual(req.files.tiny0[0].size, 122) - assert.strictEqual(util.fileSize(req.files.tiny0[0].path), 122) + assert.strictEqual(req.files.tiny0[0].size, tiny0Size) + assert.strictEqual(util.fileSize(req.files.tiny0[0].path), tiny0Size) assert.strictEqual(req.files.tiny1[0].fieldname, 'tiny1') assert.strictEqual(req.files.tiny1[0].originalname, 'tiny1.dat') @@ -126,23 +131,23 @@ describe('Disk Storage', function () { assert.strictEqual(req.files.small0[0].fieldname, 'small0') assert.strictEqual(req.files.small0[0].originalname, 'small0.dat') - assert.strictEqual(req.files.small0[0].size, 1778) - assert.strictEqual(util.fileSize(req.files.small0[0].path), 1778) + assert.strictEqual(req.files.small0[0].size, small0Size) + assert.strictEqual(util.fileSize(req.files.small0[0].path), small0Size) assert.strictEqual(req.files.small1[0].fieldname, 'small1') assert.strictEqual(req.files.small1[0].originalname, 'small1.dat') - assert.strictEqual(req.files.small1[0].size, 315) - assert.strictEqual(util.fileSize(req.files.small1[0].path), 315) + assert.strictEqual(req.files.small1[0].size, small1Size) + assert.strictEqual(util.fileSize(req.files.small1[0].path), small1Size) assert.strictEqual(req.files.medium[0].fieldname, 'medium') assert.strictEqual(req.files.medium[0].originalname, 'medium.dat') - assert.strictEqual(req.files.medium[0].size, 13196) - assert.strictEqual(util.fileSize(req.files.medium[0].path), 13196) + assert.strictEqual(req.files.medium[0].size, mediumSize) + assert.strictEqual(util.fileSize(req.files.medium[0].path), mediumSize) assert.strictEqual(req.files.large[0].fieldname, 'large') assert.strictEqual(req.files.large[0].originalname, 'large.jpg') - assert.strictEqual(req.files.large[0].size, 2413677) - assert.strictEqual(util.fileSize(req.files.large[0].path), 2413677) + assert.strictEqual(req.files.large[0].size, largeSize) + assert.strictEqual(util.fileSize(req.files.large[0].path), largeSize) done() }) diff --git a/test/functionality.js b/test/functionality.js index a4db245a..8c596a3f 100644 --- a/test/functionality.js +++ b/test/functionality.js @@ -1,6 +1,7 @@ /* eslint-env mocha */ var assert = require('assert') +var path = require('path') var util = require('./_util') var multer = require('../') @@ -16,6 +17,8 @@ function startsWith (str, start) { return (str.substring(0, start.length) === start) } +var small0Size = util.fixtureSize('small0.dat') + describe('Functionality', function () { var cleanup = [] @@ -52,7 +55,7 @@ describe('Functionality', function () { util.submitForm(parser, env.form, function (err, req) { assert.ifError(err) assert.ok(startsWith(req.file.path, env.uploadDir)) - assert.strictEqual(util.fileSize(req.file.path), 1778) + assert.strictEqual(util.fileSize(req.file.path), small0Size) done() }) }) @@ -129,8 +132,8 @@ describe('Functionality', function () { util.submitForm(parser, form, function (err, req) { assert.ifError(err) assert.strictEqual(req.files.length, 2) - assert.ok(req.files[0].path.indexOf('/testforme-') >= 0) - assert.ok(req.files[1].path.indexOf('/testforme-') >= 0) + assert.strictEqual(path.basename(path.dirname(req.files[0].path)).startsWith('testforme-'), true) + assert.strictEqual(path.basename(path.dirname(req.files[1].path)).startsWith('testforme-'), true) done() }) }) diff --git a/test/memory-storage.js b/test/memory-storage.js index cbc3e2e5..fcedfbab 100644 --- a/test/memory-storage.js +++ b/test/memory-storage.js @@ -6,6 +6,11 @@ var deepEqual = require('deep-equal') var util = require('./_util') var multer = require('../') var FormData = require('form-data') +var tiny0Size = util.fixtureSize('tiny0.dat') +var small0Size = util.fixtureSize('small0.dat') +var small1Size = util.fixtureSize('small1.dat') +var mediumSize = util.fixtureSize('medium.dat') +var largeSize = util.fixtureSize('large.jpg') describe('Memory Storage', function () { var upload @@ -29,8 +34,8 @@ describe('Memory Storage', function () { assert.strictEqual(req.file.fieldname, 'small0') assert.strictEqual(req.file.originalname, 'small0.dat') - assert.strictEqual(req.file.size, 1778) - assert.strictEqual(req.file.buffer.length, 1778) + assert.strictEqual(req.file.size, small0Size) + assert.strictEqual(req.file.buffer.length, small0Size) done() }) @@ -104,8 +109,8 @@ describe('Memory Storage', function () { assert.strictEqual(req.files.tiny0[0].fieldname, 'tiny0') assert.strictEqual(req.files.tiny0[0].originalname, 'tiny0.dat') - assert.strictEqual(req.files.tiny0[0].size, 122) - assert.strictEqual(req.files.tiny0[0].buffer.length, 122) + assert.strictEqual(req.files.tiny0[0].size, tiny0Size) + assert.strictEqual(req.files.tiny0[0].buffer.length, tiny0Size) assert.strictEqual(req.files.tiny1[0].fieldname, 'tiny1') assert.strictEqual(req.files.tiny1[0].originalname, 'tiny1.dat') @@ -114,23 +119,23 @@ describe('Memory Storage', function () { assert.strictEqual(req.files.small0[0].fieldname, 'small0') assert.strictEqual(req.files.small0[0].originalname, 'small0.dat') - assert.strictEqual(req.files.small0[0].size, 1778) - assert.strictEqual(req.files.small0[0].buffer.length, 1778) + assert.strictEqual(req.files.small0[0].size, small0Size) + assert.strictEqual(req.files.small0[0].buffer.length, small0Size) assert.strictEqual(req.files.small1[0].fieldname, 'small1') assert.strictEqual(req.files.small1[0].originalname, 'small1.dat') - assert.strictEqual(req.files.small1[0].size, 315) - assert.strictEqual(req.files.small1[0].buffer.length, 315) + assert.strictEqual(req.files.small1[0].size, small1Size) + assert.strictEqual(req.files.small1[0].buffer.length, small1Size) assert.strictEqual(req.files.medium[0].fieldname, 'medium') assert.strictEqual(req.files.medium[0].originalname, 'medium.dat') - assert.strictEqual(req.files.medium[0].size, 13196) - assert.strictEqual(req.files.medium[0].buffer.length, 13196) + assert.strictEqual(req.files.medium[0].size, mediumSize) + assert.strictEqual(req.files.medium[0].buffer.length, mediumSize) assert.strictEqual(req.files.large[0].fieldname, 'large') assert.strictEqual(req.files.large[0].originalname, 'large.jpg') - assert.strictEqual(req.files.large[0].size, 2413677) - assert.strictEqual(req.files.large[0].buffer.length, 2413677) + assert.strictEqual(req.files.large[0].size, largeSize) + assert.strictEqual(req.files.large[0].buffer.length, largeSize) done() }) diff --git a/test/reuse-middleware.js b/test/reuse-middleware.js index ab287041..3c6cc25f 100644 --- a/test/reuse-middleware.js +++ b/test/reuse-middleware.js @@ -5,6 +5,7 @@ var assert = require('assert') var util = require('./_util') var multer = require('../') var FormData = require('form-data') +var small0Size = util.fixtureSize('small0.dat') describe('Reuse Middleware', function () { var parser @@ -37,8 +38,8 @@ describe('Reuse Middleware', function () { req.files.forEach(function (file) { assert.strictEqual(file.fieldname, 'them-files') assert.strictEqual(file.originalname, 'small0.dat') - assert.strictEqual(file.size, 1778) - assert.strictEqual(file.buffer.length, 1778) + assert.strictEqual(file.size, small0Size) + assert.strictEqual(file.buffer.length, small0Size) }) if (--pending === 0) done()