diff --git a/lib/make-error.js b/lib/make-error.js index 01d74c78..9f226846 100644 --- a/lib/make-error.js +++ b/lib/make-error.js @@ -1,4 +1,5 @@ var errorMessages = { + 'CLIENT_ABORTED': 'Client aborted', 'LIMIT_PART_COUNT': 'Too many parts', 'LIMIT_FILE_SIZE': 'File too large', 'LIMIT_FILE_COUNT': 'Too many files', diff --git a/lib/make-middleware.js b/lib/make-middleware.js index 1210a6a9..a34fc59c 100644 --- a/lib/make-middleware.js +++ b/lib/make-middleware.js @@ -1,6 +1,5 @@ var is = require('type-is') var Busboy = require('busboy') -var extend = require('xtend') var onFinished = require('on-finished') var appendField = require('append-field') @@ -123,6 +122,7 @@ function makeMiddleware (setup) { var aborting = false pendingWrites.increment() + uploadedFiles.push(file) Object.defineProperty(file, 'stream', { configurable: true, @@ -140,10 +140,9 @@ function makeMiddleware (setup) { abortWithCode('LIMIT_FILE_SIZE', fieldname) }) - storage._handleFile(req, file, function (err, info) { + storage._handleFile(req, file, function (err) { if (aborting) { appender.removePlaceholder(placeholder) - uploadedFiles.push(extend(file, info)) return pendingWrites.decrement() } @@ -153,16 +152,18 @@ function makeMiddleware (setup) { return abortWithError(err) } - var fileInfo = extend(file, info) - - appender.replacePlaceholder(placeholder, fileInfo) - uploadedFiles.push(fileInfo) + appender.replacePlaceholder(placeholder, file) pendingWrites.decrement() indicateDone() }) }) }) + req.on('aborted', function () { + pendingWrites.decrement() + abortWithCode('CLIENT_ABORTED') + }) + busboy.on('error', function (err) { abortWithError(err) }) busboy.on('partsLimit', function () { abortWithCode('LIMIT_PART_COUNT') }) busboy.on('filesLimit', function () { abortWithCode('LIMIT_FILE_COUNT') }) diff --git a/package.json b/package.json index 4d7ce442..329fdab0 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,7 @@ "mkdirp": "^0.5.1", "object-assign": "^3.0.0", "on-finished": "^2.3.0", - "type-is": "^1.6.4", - "xtend": "^4.0.0" + "type-is": "^1.6.4" }, "devDependencies": { "express": "^4.13.1", diff --git a/storage/disk.js b/storage/disk.js index dfbe8893..d1eb1939 100644 --- a/storage/disk.js +++ b/storage/disk.js @@ -31,21 +31,21 @@ DiskStorage.prototype._handleFile = function _handleFile (req, file, cb) { that.getDestination(req, file, function (err, destination) { if (err) return cb(err) + file.destination = destination + that.getFilename(req, file, function (err, filename) { if (err) return cb(err) - var finalPath = path.join(destination, filename) - var outStream = fs.createWriteStream(finalPath) + file.filename = filename + file.path = path.join(destination, filename) + + var outStream = fs.createWriteStream(file.path) file.stream.pipe(outStream) outStream.on('error', cb) outStream.on('finish', function () { - cb(null, { - destination: destination, - filename: filename, - path: finalPath, - size: outStream.bytesWritten - }) + file.size = outStream.bytesWritten + cb(null) }) }) }) diff --git a/storage/memory.js b/storage/memory.js index da41aaa1..1436cf99 100644 --- a/storage/memory.js +++ b/storage/memory.js @@ -4,10 +4,9 @@ function MemoryStorage (opts) {} MemoryStorage.prototype._handleFile = function _handleFile (req, file, cb) { file.stream.pipe(concat(function (data) { - cb(null, { - buffer: data, - size: data.length - }) + file.buffer = data + file.size = data.length + cb(null) })) } diff --git a/test/error-handling.js b/test/error-handling.js index d6595cc2..4d5a04e7 100644 --- a/test/error-handling.js +++ b/test/error-handling.js @@ -7,6 +7,11 @@ var util = require('./_util') var multer = require('../') var stream = require('stream') var FormData = require('form-data') +var crypto = require('crypto') +var fs = require('fs') +var path = require('path') +var onFinished = require('on-finished') +var inherits = require('util').inherits function withLimits (limits, fields) { var storage = multer.memoryStorage() @@ -220,4 +225,71 @@ describe('Error Handling', function () { done() }) }) + + it('should handle clients aborting the request and clean up temporary files', function (done) { + function AbortDuringPassThrough (maxBytes) { + stream.PassThrough.call(this) + + this.bytesPassed = 0 + this.maxBytes = maxBytes + } + + inherits(AbortDuringPassThrough, stream.PassThrough) + + AbortDuringPassThrough.prototype._transform = function (chunk, enc, cb) { + if (this.bytesPassed + chunk.length < this.maxBytes) { + this.bytesPassed += chunk.length + stream.PassThrough.prototype._transform(chunk, enc, cb) + } else { + var len = this.maxBytes - this.bytesPassed + if (len) { + this.bytesPassed += len + stream.PassThrough.prototype._transform(chunk.slice(0, len), enc, cb) + + this.emit('aborted') + } else { + cb() + } + } + } + + var destination = os.tmpdir() + var filename = crypto.pseudoRandomBytes(16).toString('hex') + + var storage = multer.diskStorage({ + destination: destination, + filename: function (req, file, cb) { cb(null, filename) } + }) + var upload = multer({ storage: storage }).single('file') + + var form = new FormData() + form.append('file', util.file('small0.dat')) + + form.getLength(function (err, length) { + assert.equal(err, null) + + var req = new AbortDuringPassThrough(1000) + + req.complete = false + form.once('end', function () { + req.complete = true + }) + + form.pipe(req) + req.headers = { + 'content-type': 'multipart/form-data; boundary=' + form.getBoundary(), + 'content-length': length + } + + upload(req, null, function (err) { + assert.equal(err.code, 'CLIENT_ABORTED') + + onFinished(req, function () { + assert.equal(fs.existsSync(path.join(destination, filename)), false, + 'temporary file must be cleaned up') + done() + }) + }) + }) + }) })