From e8bce4da5e149f6a03aa0e0c6d067cd38f00f19e Mon Sep 17 00:00:00 2001 From: devconcept Date: Mon, 19 Jun 2017 12:07:22 -0400 Subject: [PATCH 1/4] First draft of new api --- .gitignore | 2 + README.md | 71 ++++++++++- index.js | 31 +++-- lib/middleware.js | 10 +- lib/read-body.js | 103 +++++++++++----- lib/stream-handler.js | 54 ++++++++ test/body.js | 2 +- test/express-integration.js | 2 +- test/functionality.js | 56 +++++++++ test/handlers.js | 238 ++++++++++++++++++++++++++++++++++++ test/limits.js | 2 +- test/misc.js | 4 - 12 files changed, 513 insertions(+), 62 deletions(-) create mode 100644 lib/stream-handler.js create mode 100644 test/functionality.js create mode 100644 test/handlers.js diff --git a/.gitignore b/.gitignore index 38602882..e8bfc982 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +.idea/* + # OS X .DS_Store* Icon? diff --git a/README.md b/README.md index 508a4760..ae5ea7f3 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,9 @@ Key | Description --- | --- `fieldName` | Field name specified in the form `originalName` | Name of the file on the user's computer (`undefined` if no filename was supplied by the client) -`size` | Size of the file in bytes -`stream` | Stream of file +`size` | Size of the file in bytes 2 +`stream` | A new readable stream for the stored file 2 +`path` | The full path where the file is stored 2 `detectedMimeType` | The detected mime-type, or null if we failed to detect `detectedFileExtension` | The typical file extension for files of the detected type, or empty string if we failed to detect (with leading `.` to match `path.extname`) `clientReportedMimeType` | The mime type reported by the client using the `Content-Type` header, or null1 if the header was absent @@ -79,13 +80,19 @@ Key | Description 1 Currently returns `text/plain` if header is absent, this is a bug and it will be fixed in a patch release. Do not rely on this behavior. +2 Available only when the `handler` option is not used + ### `multer(opts)` -Multer accepts an options object, the following are the options that can be -passed to Multer. +The following are the options that can be passed to Multer. All of them are optional. +The `opts` parameter can also be a string with a path in which case it will be used +as the destination to store files. + Key | Description -------- | ----------- +`dest` | The destination path to store files. If no destination is provided the os temporary folder is used. +`handler` | A function that allows you supply your own writable stream for customization of file storage. Using this causes `dest` to be ignored. See [using streams](#using-streams) for more information `limits` | Limits of the uploaded data [(full description)](#limits) #### `.single(fieldname)` @@ -147,6 +154,62 @@ Key | Description | Default Specifying the limits can help protect your site against denial of service (DoS) attacks. +### Using streams + +Using handlers allows the efficient use of any stream implementation to store files anywhere. +To achieve this, just pass a function to Multer that will be invoked with `req` and `file`. You +have to return a new stream or an object to specify how the writable streams will be created. + +If you return an object instead of a stream this are the properties you should set. Only the `stream` property is required. + +#### stream + +A writable stream or a function that returns a new writable stream to pipe for each incoming file. By default +core `fs` streams are used. You can return a promise that resolves with the stream too. + +#### event + +The event that finish the writes. Defaults to `'close'`. You can change this to another value +like `'finish'` or any event that your custom stream implements (Not all +writable streams emit the 'close' event so make sure to change accordingly). + +#### finish + +A post-processing function that executes after the event specified in the previous property is triggered. +This also gives you an opportunity to extend the file object. If any arguments were received from the event +they will be available as the parameters of the function. Promises are supported here as well. + +A handler could be as simple as + +```javascript +function handler(req, file) { + return new writableStream() +} +``` + +or more complex like + +```javascript +function handler(req, file) { + return { + stream: function() { + // You can use the stream in the property directly. + // The function is only required for async code + return doSomeAsync().then(() => { + return createWriteStream() + }) + }, + event: 'finish', + finish: function() { + return hashFile().then(() => { + file.info = path; + file.stream = createReadStream(name) + }) + } + } +} +``` + ## Error handling When encountering an error, multer will delegate the error to express. You can diff --git a/index.js b/index.js index e4372b90..0953217e 100644 --- a/index.js +++ b/index.js @@ -1,11 +1,14 @@ var createFileFilter = require('./lib/file-filter') var createMiddleware = require('./lib/middleware') +var streamHandler = require('./lib/stream-handler') +var os = require('os') -function _middleware (limits, fields, fileStrategy) { +function _middleware (limits, handler, fields, fileStrategy) { return createMiddleware(function setup () { return { fields: fields, limits: limits, + handler: handler, fileFilter: createFileFilter(fields), fileStrategy: fileStrategy } @@ -13,23 +16,28 @@ function _middleware (limits, fields, fileStrategy) { } function Multer (options) { - this.limits = options.limits + if (typeof options === 'string') { + this.handler = streamHandler.createHandler(options) + } else { + this.limits = options.limits + this.handler = options.handler || streamHandler.createHandler(options.dest || os.tmpdir()) + } } Multer.prototype.single = function (name) { - return _middleware(this.limits, [{ name: name, maxCount: 1 }], 'VALUE') + return _middleware(this.limits, this.handler, [{name: name, maxCount: 1}], 'VALUE') } Multer.prototype.array = function (name, maxCount) { - return _middleware(this.limits, [{ name: name, maxCount: maxCount }], 'ARRAY') + return _middleware(this.limits, this.handler, [{name: name, maxCount: maxCount}], 'ARRAY') } Multer.prototype.fields = function (fields) { - return _middleware(this.limits, fields, 'OBJECT') + return _middleware(this.limits, this.handler, fields, 'OBJECT') } Multer.prototype.none = function () { - return _middleware(this.limits, [], 'NONE') + return _middleware(this.limits, this.handler, [], 'NONE') } Multer.prototype.any = function () { @@ -37,6 +45,7 @@ Multer.prototype.any = function () { return { fields: [], limits: this.limits, + handler: this.handler, fileFilter: function () {}, fileStrategy: 'ARRAY' } @@ -47,11 +56,13 @@ Multer.prototype.any = function () { function multer (options) { if (options === undefined) options = {} - if (options === null) throw new TypeError('Expected object for arugment "options", got null') - if (typeof options !== 'object') throw new TypeError('Expected object for arugment "options", got ' + (typeof options)) + if (options === null) throw new TypeError('Expected object for argument "options", got null') + if (typeof options !== 'object' && typeof options !== 'string') throw new TypeError('Expected object or string for argument "options", got ' + (typeof options)) + + if (options.handler && typeof options.handler !== 'function') throw new TypeError('The handler must be a function') - if (options.dest || options.storage || options.fileFilter) { - throw new Error('The "dest", "storage" and "fileFilter" options where removed in Multer 2.0. Please refer to the latest documentation for new usage.') + if (options.storage || options.fileFilter) { + throw new Error('The "storage" and "fileFilter" options where removed in Multer 2.0. Please refer to the latest documentation for new usage.') } return new Multer(options) diff --git a/lib/middleware.js b/lib/middleware.js index 4865b230..4818cabd 100644 --- a/lib/middleware.js +++ b/lib/middleware.js @@ -1,5 +1,4 @@ var is = require('type-is') -var fs = require('fs') var appendField = require('append-field') var createFileAppender = require('./file-appender') @@ -11,7 +10,7 @@ module.exports = function createMiddleware (setup) { var options = setup() - readBody(req, options.limits, options.fileFilter) + readBody(req, options) .then(function (result) { req.body = Object.create(null) @@ -22,15 +21,8 @@ module.exports = function createMiddleware (setup) { var appendFile = createFileAppender(options.fileStrategy, req, options.fields) result.files.forEach(function (file) { - file.stream = fs.createReadStream(file.path) - - file.stream.on('open', function () { - fs.unlink(file.path, function () {}) - }) - appendFile(file) }) - next() }) .catch(next) diff --git a/lib/read-body.js b/lib/read-body.js index 435e4f76..0ed43239 100644 --- a/lib/read-body.js +++ b/lib/read-body.js @@ -1,8 +1,8 @@ var path = require('path') var pify = require('pify') -var temp = require('fs-temp') var Busboy = require('busboy') var FileType = require('stream-file-type') +var normalize = require('./stream-handler').normalize var pump = pify(require('pump')) var onFinished = pify(require('on-finished')) @@ -13,9 +13,24 @@ function drainStream (stream) { stream.on('readable', stream.read.bind(stream)) } -function collectFields (busboy, limits) { +function isPromise (target) { + var type = typeof target + return target !== null && (type === 'object' || type === 'function') && typeof target.then === 'function' +} + +// Support promises in the stream and the finish properties +function waitFor () { + var fn = arguments[0] + // Replace with rest arguments if engine compatibility allows it + var args = Array.prototype.slice.call(arguments, 1) + var target = fn.apply(fn, args) + return !isPromise(target) ? Promise.resolve(target) : target +} + +function collectFields (busboy, options) { return new Promise(function (resolve, reject) { var result = [] + var limits = options.limits busboy.on('field', function (fieldname, value, fieldnameTruncated, valueTruncated) { if (fieldnameTruncated) return reject(new MulterError('LIMIT_FIELD_KEY')) @@ -26,7 +41,7 @@ function collectFields (busboy, limits) { if (fieldname.length > limits.fieldNameSize) return reject(new MulterError('LIMIT_FIELD_KEY')) } - result.push({ key: fieldname, value: value }) + result.push({key: fieldname, value: value}) }) busboy.on('finish', function () { @@ -35,9 +50,11 @@ function collectFields (busboy, limits) { }) } -function collectFiles (busboy, limits, fileFilter) { +function collectFiles (req, busboy, options) { return new Promise(function (resolve, reject) { var result = [] + var limits = options.limits + var fileFilter = options.fileFilter busboy.on('file', function (fieldname, fileStream, filename, encoding, mimetype) { // Catch all errors on file stream @@ -59,6 +76,8 @@ function collectFiles (busboy, limits, fileFilter) { fileStream.on('limit', resolve) }) + var handler = normalize(options.handler(req, file)) + Promise.resolve() .then(function () { return fileFilter(file) @@ -68,30 +87,50 @@ function collectFiles (busboy, limits, fileFilter) { reject(new MulterError('LIMIT_FILE_SIZE', fieldname)) }) - var target = temp.createWriteStream() - var detector = new FileType() - - var fileClosed = new Promise(function (resolve) { - target.on('close', resolve) - }) - - var promise = pump(fileStream, detector, target) - .then(function () { - return fileClosed - }) - .then(function () { - return detector.fileTypePromise() + return waitFor(handler.stream) + .then(function (target) { + var detector = new FileType() + + var fileClosed = new Promise(function (resolve, reject) { + var evt = 'close' + if (handler.event) { + evt = typeof handler.event === 'function' ? handler.event() : handler.event + } + + target.on(evt, function () { + if (!handler.finish) { + return resolve() + } + + // Different stream implementations could have custom events with unknown number of arguments + // This is why the finish function can be used to gather this arguments and merge them with the file object + // Right after the stream has been consumed + var evtArgs = Array.prototype.slice.call(arguments) + evtArgs.unshift(handler.finish) + waitFor.apply(null, evtArgs) + .then(function () { + resolve() + }) + .catch(reject) + }) + }) + + var promise = pump(fileStream, detector, target) + .then(function () { + return fileClosed + }) + .then(function () { + return detector.fileTypePromise() + }) + .then(function (fileType) { + file.detectedMimeType = (fileType ? fileType.mime : null) + file.detectedFileExtension = (fileType ? '.' + fileType.ext : '') + return file + }) + .catch(reject) + + result.push(promise) }) - .then(function (fileType) { - file.path = target.path - file.size = target.bytesWritten - file.detectedMimeType = (fileType ? fileType.mime : null) - file.detectedFileExtension = (fileType ? '.' + fileType.ext : '') - return file - }) - .catch(reject) - - result.push(promise) }) .catch(reject) }) @@ -102,17 +141,17 @@ function collectFiles (busboy, limits, fileFilter) { }) } -function readBody (req, limits, fileFilter) { +function readBody (req, options) { var busboy try { - busboy = new Busboy({ headers: req.headers, limits: limits }) + busboy = new Busboy({headers: req.headers, limits: options.limits}) } catch (err) { return Promise.reject(err) } - var fields = collectFields(busboy, limits) - var files = collectFiles(busboy, limits, fileFilter) + var fields = collectFields(busboy, options) + var files = collectFiles(req, busboy, options) var guard = new Promise(function (resolve, reject) { req.on('error', function (err) { reject(err) }) busboy.on('error', function (err) { reject(err) }) @@ -129,7 +168,7 @@ function readBody (req, limits, fileFilter) { return Promise.all([fields, files, guard]) .then(function (result) { - return { fields: result[0], files: result[1] } + return {fields: result[0], files: result[1]} }) .catch(function (err) { req.unpipe(busboy) diff --git a/lib/stream-handler.js b/lib/stream-handler.js new file mode 100644 index 00000000..3fba780a --- /dev/null +++ b/lib/stream-handler.js @@ -0,0 +1,54 @@ +var crytpo = require('crypto') +var fs = require('fs') +var path = require('path') +var isStream = require('is-stream') + +function randomBytes () { + return new Promise(function (resolve, reject) { + crytpo.randomBytes(16, function (err, buf) { + if (err) { + return reject(err) + } + resolve(buf) + }) + }) +} + +module.exports.createHandler = function createHandler (destination) { + return function handler (req, file) { + var stream + + return { + stream: function createStream () { + return randomBytes().then(function (buf) { + var filename = buf.toString('hex') + stream = fs.createWriteStream(path.join(destination, filename)) + return stream + }) + }, + event: 'close', + finish: function storeFile () { + var readStream = fs.createReadStream(stream.path) + file.size = stream.bytesWritten + file.path = stream.path + file.stream = readStream + } + } + } +} + +module.exports.normalize = function normalize (handler) { + var result, stream + if (isStream.writable(handler)) { + result = { + stream: function () { return handler } + } + } else if (isStream.writable(handler.stream)) { + result = handler + stream = handler.stream + result.stream = function () { return stream } + } else { + result = handler + } + return result +} diff --git a/test/body.js b/test/body.js index 1db6ae12..ece630bd 100644 --- a/test/body.js +++ b/test/body.js @@ -9,7 +9,7 @@ var multer = require('../') var FormData = require('form-data') var testData = require('testdata-w3c-json-form') -describe('body', function () { +describe('Body', function () { var parser before(function () { diff --git a/test/express-integration.js b/test/express-integration.js index 13a6cbaa..83002362 100644 --- a/test/express-integration.js +++ b/test/express-integration.js @@ -24,7 +24,7 @@ describe('Express Integration', function () { function submitForm (form, path) { return new Promise(function (resolve, reject) { - var req = form.submit('http://localhost:' + port + path) + var req = form.submit('http://127.0.0.1:' + port + path) req.on('error', reject) req.on('response', function (res) { diff --git a/test/functionality.js b/test/functionality.js new file mode 100644 index 00000000..389aa699 --- /dev/null +++ b/test/functionality.js @@ -0,0 +1,56 @@ +/* eslint-env mocha */ + +var assert = require('assert') +var FormData = require('form-data') +var fs = require('fs-temp/promise') + +var multer = require('../') +var util = require('./_util') +var path = require('path') + +describe('Functionality', function () { + var parser + var uploadDir + + before(function () { + return fs.mkdir().then(function (dir) { + uploadDir = dir + }) + }) + + it('should upload the file to the `dest` dir', function () { + var form = new FormData() + + parser = multer({dest: uploadDir}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(path.dirname(req.file.path), uploadDir) + }) + }) + + it('should upload using a string argument as the `dest` dir', function () { + var form = new FormData() + + parser = multer(uploadDir).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(path.dirname(req.file.path), uploadDir) + }) + }) +}) diff --git a/test/handlers.js b/test/handlers.js new file mode 100644 index 00000000..2e1e6339 --- /dev/null +++ b/test/handlers.js @@ -0,0 +1,238 @@ +/* eslint-env mocha */ + +var assert = require('assert') +var FormData = require('form-data') +var fs = require('fs-temp') + +var multer = require('../') +var util = require('./_util') + +var Writable = require('stream').Writable + +class TestWritable extends Writable { + constructor (options) { + super(options) + this.on('finish', function () { + this.emit('unicorn', 'arg') + this.emit('rainbow', 'arg') + }) + } + + _write (chunk, encoding, callback) { + callback() + } +} + +describe('Handlers', function () { + var parser + + it('should accept a handler configuration', function () { + assert.doesNotThrow(function () { + multer({handler: function () {}}) + }) + }) + + it('should accept a handler that returns a stream', function () { + var form = new FormData() + + function handler () { + return fs.createWriteStream() + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(req.file.size, null) + assert.equal(req.file.stream, null) + assert.equal(req.file.path, null) + }) + }) + + it('should accept a handler that returns a object', function () { + var form = new FormData() + + function handler () { + var stream = fs.createWriteStream() + return {stream: stream} + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(req.file.size, null) + assert.equal(req.file.stream, null) + assert.equal(req.file.path, null) + }) + }) + + it('should accept a handler that has a stream function', function () { + var form = new FormData() + + function handler () { + return { + stream: function () { + return fs.createWriteStream() + } + } + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(req.file.size, null) + assert.equal(req.file.stream, null) + assert.equal(req.file.path, null) + }) + }) + + it('should accept a handler that has a post processing function', function () { + var form = new FormData() + + function handler (req, file) { + return { + stream: function () { + return fs.createWriteStream() + }, + finish: function () { + file.metadata = 'random data' + } + } + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(req.file.size, null) + assert.equal(req.file.stream, null) + assert.equal(req.file.path, null) + assert.equal(req.file.metadata, 'random data') + }) + }) + + it('should accept a handler that changes the stream event', function () { + var form = new FormData() + var stream = new TestWritable() + + function handler () { + return { + stream: stream, + event: 'unicorn' + } + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(stream.eventNames()[1], 'unicorn') + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + }) + }) + + it('should receive the arguments after the event is emitted', function () { + var form = new FormData() + var stream = new TestWritable() + var args + + function handler () { + return { + stream: stream, + event: 'unicorn', + finish: function () { + args = arguments + } + } + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(stream.eventNames()[1], 'unicorn') + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(args.length, 1) + assert.equal(args[0], 'arg') + }) + }) + + it('should answer to different events per file', function () { + var form = new FormData() + var events = ['unicorn', 'rainbow'] + var counter = 0 + + function handler (req, file) { + var stream = new TestWritable() + var evt = events[counter] + counter++ + return { + stream: stream, + event: evt, + finish: function () { + file.stream = stream + } + } + } + + parser = multer({handler: handler}).array('file', 2) + + form.append('name', 'Multer') + form.append('file', util.file('small')) + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.files) + + req.files.forEach(function (file, index) { + assert.equal(file.stream.eventNames()[1], events[index]) + assert.equal(file.fieldName, 'file') + assert.equal(file.originalName, 'small.dat') + }) + }) + }) +}) diff --git a/test/limits.js b/test/limits.js index cf162f58..b1b1c837 100644 --- a/test/limits.js +++ b/test/limits.js @@ -7,7 +7,7 @@ var multer = require('../') var FormData = require('form-data') var assertRejects = require('assert-rejects') -describe('limits', function () { +describe('Limits', function () { it('should report limit errors', function () { var form = new FormData() var parser = multer({ limits: { fileSize: 100 } }).single('file') diff --git a/test/misc.js b/test/misc.js index e554a64f..f1d34f80 100644 --- a/test/misc.js +++ b/test/misc.js @@ -81,10 +81,6 @@ describe('Misc', function () { }) it('should give error on old options', function () { - assert.throws(function () { - multer({ dest: '/tmp' }) - }) - assert.throws(function () { multer({ storage: {} }) }) From 321286ef3369e34db53968381e8b2a278543749f Mon Sep 17 00:00:00 2001 From: devconcept Date: Tue, 20 Jun 2017 07:59:27 -0400 Subject: [PATCH 2/4] Second draft. Simplified api --- README.md | 39 ++++++++++++++++------------------- lib/read-body.js | 34 ++++++++---------------------- lib/stream-handler.js | 45 +++++++++++++++------------------------- package.json | 1 + test/handlers.js | 48 ++++++++++++++++++++++++++++++++----------- 5 files changed, 80 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index ae5ea7f3..304c048a 100644 --- a/README.md +++ b/README.md @@ -157,15 +157,15 @@ Specifying the limits can help protect your site against denial of service (DoS) ### Using streams Using handlers allows the efficient use of any stream implementation to store files anywhere. -To achieve this, just pass a function to Multer that will be invoked with `req` and `file`. You -have to return a new stream or an object to specify how the writable streams will be created. +To achieve this, just pass a function to Multer in the `handler` property that will be invoked with `req` and `file`. You +will have to return a new stream or an object specifying how the writable streams will be created. It is also +possible to return a promise from a handler in case you need some async before creating the stream. -If you return an object instead of a stream this are the properties you should set. Only the `stream` property is required. +If you provide an object instead of a stream this are the properties you should set. Only the `stream` property is required. #### stream -A writable stream or a function that returns a new writable stream to pipe for each incoming file. By default -core `fs` streams are used. You can return a promise that resolves with the stream too. +A writable stream to pipe for each incoming file. By default core `fs` streams are used. #### event @@ -176,8 +176,9 @@ writable streams emit the 'close' event so make sure to change accordingly). #### finish A post-processing function that executes after the event specified in the previous property is triggered. -This also gives you an opportunity to extend the file object. If any arguments were received from the event -they will be available as the parameters of the function. Promises are supported here as well. +This also gives you an opportunity to extend the file object or inspect the consumed stream. If any arguments +were received from the event they will be available as the parameters of the function. Promises are supported +here as well to allow additional processing like hashing a file, etc. A handler could be as simple as @@ -191,20 +192,16 @@ or more complex like ```javascript function handler(req, file) { - return { - stream: function() { - // You can use the stream in the property directly. - // The function is only required for async code - return doSomeAsync().then(() => { - return createWriteStream() - }) - }, - event: 'finish', - finish: function() { - return hashFile().then(() => { - file.info = path; - file.stream = createReadStream(name) - }) + return doSomeAsync().then(() => { + return { + stream: createWriteStream(), + event: 'landed', + finish: function() { + return hashFile().then((hash) => { + file.metadata = { hash }; + file.stream = createReadStream() + }) + } } } } diff --git a/lib/read-body.js b/lib/read-body.js index 0ed43239..56bba036 100644 --- a/lib/read-body.js +++ b/lib/read-body.js @@ -13,20 +13,6 @@ function drainStream (stream) { stream.on('readable', stream.read.bind(stream)) } -function isPromise (target) { - var type = typeof target - return target !== null && (type === 'object' || type === 'function') && typeof target.then === 'function' -} - -// Support promises in the stream and the finish properties -function waitFor () { - var fn = arguments[0] - // Replace with rest arguments if engine compatibility allows it - var args = Array.prototype.slice.call(arguments, 1) - var target = fn.apply(fn, args) - return !isPromise(target) ? Promise.resolve(target) : target -} - function collectFields (busboy, options) { return new Promise(function (resolve, reject) { var result = [] @@ -76,8 +62,6 @@ function collectFiles (req, busboy, options) { fileStream.on('limit', resolve) }) - var handler = normalize(options.handler(req, file)) - Promise.resolve() .then(function () { return fileFilter(file) @@ -87,27 +71,25 @@ function collectFiles (req, busboy, options) { reject(new MulterError('LIMIT_FILE_SIZE', fieldname)) }) - return waitFor(handler.stream) - .then(function (target) { + return Promise.resolve(options.handler(req, file)) + .then(function (config) { var detector = new FileType() + var handler = normalize(config) + var target = handler.stream var fileClosed = new Promise(function (resolve, reject) { - var evt = 'close' - if (handler.event) { - evt = typeof handler.event === 'function' ? handler.event() : handler.event - } + var evt = handler.event || 'close' target.on(evt, function () { - if (!handler.finish) { + var finish = handler.finish + if (!finish) { return resolve() } // Different stream implementations could have custom events with unknown number of arguments // This is why the finish function can be used to gather this arguments and merge them with the file object // Right after the stream has been consumed - var evtArgs = Array.prototype.slice.call(arguments) - evtArgs.unshift(handler.finish) - waitFor.apply(null, evtArgs) + Promise.resolve(finish.apply(null, arguments)) .then(function () { resolve() }) diff --git a/lib/stream-handler.js b/lib/stream-handler.js index 3fba780a..62156d1a 100644 --- a/lib/stream-handler.js +++ b/lib/stream-handler.js @@ -1,3 +1,5 @@ +'use strict' + var crytpo = require('crypto') var fs = require('fs') var path = require('path') @@ -16,39 +18,26 @@ function randomBytes () { module.exports.createHandler = function createHandler (destination) { return function handler (req, file) { - var stream - - return { - stream: function createStream () { - return randomBytes().then(function (buf) { - var filename = buf.toString('hex') - stream = fs.createWriteStream(path.join(destination, filename)) - return stream - }) - }, - event: 'close', - finish: function storeFile () { - var readStream = fs.createReadStream(stream.path) - file.size = stream.bytesWritten - file.path = stream.path - file.stream = readStream + return randomBytes().then(function (buf) { + var stream + var filename = buf.toString('hex') + stream = fs.createWriteStream(path.join(destination, filename)) + return { + stream: stream, + event: 'close', + finish: function () { + file.size = stream.bytesWritten + file.path = stream.path + file.stream = fs.createReadStream(stream.path) + } } - } + }) } } module.exports.normalize = function normalize (handler) { - var result, stream if (isStream.writable(handler)) { - result = { - stream: function () { return handler } - } - } else if (isStream.writable(handler.stream)) { - result = handler - stream = handler.stream - result.stream = function () { return stream } - } else { - result = handler + return {stream: handler} } - return result + return handler } diff --git a/package.json b/package.json index dcea6b51..556c1129 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "append-field": "^1.0.0", "busboy": "^0.2.13", "fs-temp": "^1.1.1", + "is-stream": "^1.1.0", "on-finished": "^2.3.0", "pify": "^2.3.0", "pump": "^1.0.1", diff --git a/test/handlers.js b/test/handlers.js index 2e1e6339..67f55f2b 100644 --- a/test/handlers.js +++ b/test/handlers.js @@ -60,8 +60,9 @@ describe('Handlers', function () { var form = new FormData() function handler () { - var stream = fs.createWriteStream() - return {stream: stream} + return { + stream: fs.createWriteStream() + } } parser = multer({handler: handler}).single('file') @@ -81,15 +82,39 @@ describe('Handlers', function () { }) }) - it('should accept a handler that has a stream function', function () { + it('should accept a handler that returns a promise', function () { var form = new FormData() function handler () { - return { - stream: function () { - return fs.createWriteStream() - } - } + return new Promise(function (resolve) { + resolve(fs.createWriteStream()) + }) + } + + parser = multer({handler: handler}).single('file') + + form.append('name', 'Multer') + form.append('file', util.file('small')) + + return util.submitForm(parser, form).then(function (req) { + assert.equal(req.body.name, 'Multer') + + assert.ok(req.file) + assert.equal(req.file.fieldName, 'file') + assert.equal(req.file.originalName, 'small.dat') + assert.equal(req.file.size, null) + assert.equal(req.file.stream, null) + assert.equal(req.file.path, null) + }) + }) + + it('should accept a handler that resolves to an object', function () { + var form = new FormData() + + function handler () { + return new Promise(function (resolve) { + resolve({stream: fs.createWriteStream()}) + }) } parser = multer({handler: handler}).single('file') @@ -114,9 +139,7 @@ describe('Handlers', function () { function handler (req, file) { return { - stream: function () { - return fs.createWriteStream() - }, + stream: fs.createWriteStream(), finish: function () { file.metadata = 'random data' } @@ -143,9 +166,10 @@ describe('Handlers', function () { it('should accept a handler that changes the stream event', function () { var form = new FormData() - var stream = new TestWritable() + var stream function handler () { + stream = new TestWritable() return { stream: stream, event: 'unicorn' From 3c662fe87adfffd300fd6137dbf856bb89aebc9d Mon Sep 17 00:00:00 2001 From: devconcept Date: Tue, 20 Jun 2017 08:39:58 -0400 Subject: [PATCH 3/4] Fixed strict mode issue in test --- test/handlers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/handlers.js b/test/handlers.js index 67f55f2b..3aa35489 100644 --- a/test/handlers.js +++ b/test/handlers.js @@ -1,4 +1,5 @@ /* eslint-env mocha */ +'use strict' var assert = require('assert') var FormData = require('form-data') From 8d26f7a495bda745547839bbb90fe7a522aa7ba2 Mon Sep 17 00:00:00 2001 From: devconcept Date: Tue, 20 Jun 2017 08:48:24 -0400 Subject: [PATCH 4/4] Replaced emitter.eventNames with a more compatible method --- test/handlers.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/handlers.js b/test/handlers.js index 3aa35489..0bef4a28 100644 --- a/test/handlers.js +++ b/test/handlers.js @@ -186,7 +186,7 @@ describe('Handlers', function () { assert.equal(req.body.name, 'Multer') assert.ok(req.file) - assert.equal(stream.eventNames()[1], 'unicorn') + assert.equal(stream.listenerCount('unicorn'), 1) assert.equal(req.file.fieldName, 'file') assert.equal(req.file.originalName, 'small.dat') }) @@ -216,7 +216,7 @@ describe('Handlers', function () { assert.equal(req.body.name, 'Multer') assert.ok(req.file) - assert.equal(stream.eventNames()[1], 'unicorn') + assert.equal(stream.listenerCount('unicorn'), 1) assert.equal(req.file.fieldName, 'file') assert.equal(req.file.originalName, 'small.dat') assert.equal(args.length, 1) @@ -254,7 +254,7 @@ describe('Handlers', function () { assert.ok(req.files) req.files.forEach(function (file, index) { - assert.equal(file.stream.eventNames()[1], events[index]) + assert.equal(file.stream.listenerCount(events[index]), 1) assert.equal(file.fieldName, 'file') assert.equal(file.originalName, 'small.dat') })