diff --git a/README.md b/README.md index 1a24403..43a0427 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,14 @@ Enable or disable accepting ranged requests, defaults to true. Disabling this will not send `Accept-Ranges` and ignore the contents of the `Range` request header. +##### autoClose + +If `autoClose` is `false`, then the file descriptor won't be closed, +even if there's an error. It is your responsibility to close it and +make sure there's no file descriptor leak. If `autoClose` is set to +`true` (default behavior), on `error` or `end` the file descriptor +will be closed automatically. + ##### cacheControl Enable or disable setting `Cache-Control` response header, defaults to @@ -85,6 +93,11 @@ in the given order. By default, this is disabled (set to `false`). An example value that will serve extension-less HTML files: `['html', 'htm']`. This is skipped if the requested file already has an extension. +##### fd + +If `fd` is specified, send will ignore the `path` argument and will use the +specified file descriptor. This means that no `'open'` event will be emitted. + ##### immutable Enable or diable the `immutable` directive in the `Cache-Control` response @@ -126,9 +139,11 @@ The `SendStream` is an event emitter and will emit the following events: - `error` an error occurred `(err)` - `directory` a directory was requested `(res, path)` - `file` a file was requested `(path, stat)` + - `open` a file descriptor was opened for streaming `(fd)` - `headers` the headers are about to be set on a file `(res, path, stat)` - `stream` file streaming has started `(stream)` - `end` streaming has completed + - `close` the file descriptor was closed #### .pipe diff --git a/index.js b/index.js index c4c9677..c880728 100644 --- a/index.js +++ b/index.js @@ -15,7 +15,6 @@ var createError = require('http-errors') var debug = require('debug')('send') var deprecate = require('depd')('send') -var destroy = require('destroy') var encodeUrl = require('encodeurl') var escapeHtml = require('escape-html') var etag = require('etag') @@ -24,6 +23,7 @@ var fs = require('fs') var mime = require('mime') var ms = require('ms') var onFinished = require('on-finished') +var isFinished = onFinished.isFinished var parseRange = require('range-parser') var path = require('path') var statuses = require('statuses') @@ -62,6 +62,18 @@ var MAX_MAXAGE = 60 * 60 * 24 * 365 * 1000 // 1 year var UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/ +/** + * Regular expression to match a bad file number error code + * Node on Windows incorrectly sets the error code to `OK` + * instead of `EBADF` because of a bug in libuv + * @type {RegExp} + * @const + * @see {@link https://github.com/nodejs/node/issues/3718} + * @see {@link https://github.com/libuv/libuv/pull/613} + */ + +var BAD_FD_REGEXP = /^(EBADF|OK)$/ + /** * Module exports. * @public @@ -161,9 +173,17 @@ function SendStream (req, path, options) { ? resolve(opts.root) : null + this.fd = typeof opts.fd === 'number' + ? opts.fd + : null + + this.autoClose = opts.autoClose !== false + if (!this._root && opts.from) { this.from(opts.from) } + + this.onFileSystemError = this.onFileSystemError.bind(this) } /** @@ -403,17 +423,17 @@ SendStream.prototype.headersAlreadySent = function headersAlreadySent () { SendStream.prototype.isCachable = function isCachable () { var statusCode = this.res.statusCode return (statusCode >= 200 && statusCode < 300) || - statusCode === 304 + /* istanbul ignore next */ statusCode === 304 } /** - * Handle stat() error. + * Handle file system error. * * @param {Error} error * @private */ -SendStream.prototype.onStatError = function onStatError (error) { +SendStream.prototype.onFileSystemError = function onFileSystemError (error) { switch (error.code) { case 'ENAMETOOLONG': case 'ENOENT': @@ -500,7 +520,35 @@ SendStream.prototype.redirect = function redirect (path) { } /** - * Pipe to `res. + * End the stream. + * + * @private + */ + +SendStream.prototype.end = function end () { + this.send = this.close + if (this._stream) this._stream.destroy() + if (this.autoClose) this.close() +} + +/** + * Close the file descriptor. + * + * @private + */ + +SendStream.prototype.close = function close () { + if (typeof this.fd !== 'number') return + var self = this + fs.close(this.fd, function (err) { /* istanbul ignore next */ + if (err && !BAD_FD_REGEXP.test(err.code)) return self.onFileSystemError(err) + self.fd = null + self.emit('close') + }) +} + +/** + * Pipe to `res`. * * @param {Stream} res * @return {Stream} res @@ -510,10 +558,28 @@ SendStream.prototype.redirect = function redirect (path) { SendStream.prototype.pipe = function pipe (res) { // root path var root = this._root + var self = this // references this.res = res + // response finished, done with the fd + if (isFinished(res)) { + this.end() + return res + } + + onFinished(res, this.end.bind(this)) + + if (typeof this.fd === 'number') { + fs.fstat(this.fd, function (err, stat) { + if (err) return self.onFileSystemError(err) + self.emit('file', self.path, stat) + self.send(self.path, stat) + }) + return res + } + // decode the path var path = decode(this.path) if (path === -1) { @@ -619,7 +685,7 @@ SendStream.prototype.send = function send (path, stat) { return } - debug('pipe "%s"', path) + debug('pipe fd "%d" for path "%s"', this.fd, path) // set header fields this.setHeader(path, stat) @@ -705,7 +771,7 @@ SendStream.prototype.send = function send (path, stat) { return } - this.stream(path, opts) + this.stream(opts) } /** @@ -717,34 +783,42 @@ SendStream.prototype.send = function send (path, stat) { SendStream.prototype.sendFile = function sendFile (path) { var i = 0 var self = this - - debug('stat "%s"', path) - fs.stat(path, function onstat (err, stat) { - if (err && err.code === 'ENOENT' && !extname(path) && path[path.length - 1] !== sep) { - // not found, check extensions - return next(err) - } - if (err) return self.onStatError(err) - if (stat.isDirectory()) return self.redirect(path) - self.emit('file', path, stat) - self.send(path, stat) + var redirect = this.redirect.bind(this, path) + + debug('open "%s"', path) + fs.open(path, 'r', function onopen (err, fd) { + if (!err) return sendStats(path, fd, self.onFileSystemError, redirect) + return err.code === 'ENOENT' && !extname(path) && path[path.length - 1] !== sep + ? next(err) // not found, check extensions + : self.onFileSystemError(err) }) function next (err) { if (self._extensions.length <= i) { return err - ? self.onStatError(err) + ? self.onFileSystemError(err) : self.error(404) } - var p = path + '.' + self._extensions[i++] - - debug('stat "%s"', p) - fs.stat(p, function (err, stat) { + debug('open "%s"', p) + fs.open(p, 'r', function (err, fd) { if (err) return next(err) - if (stat.isDirectory()) return next() - self.emit('file', p, stat) - self.send(p, stat) + sendStats(p, fd, next, next) + }) + } + + function sendStats (path, fd, onError, onDirectory) { + debug('stat fd "%d" for path "%s"', fd, path) + fs.fstat(fd, function onstat (err, stat) { + if (err || stat.isDirectory()) { + return fs.close(fd, function (e) { /* istanbul ignore next */ + return (err || e) ? onError(err || e) : onDirectory() + }) + } + self.fd = fd + self.emit('file', path, stat) + self.emit('open', fd) + self.send(path, stat) }) } } @@ -755,72 +829,59 @@ SendStream.prototype.sendFile = function sendFile (path) { * @param {String} path * @api private */ + SendStream.prototype.sendIndex = function sendIndex (path) { var i = -1 var self = this - function next (err) { + return (function next (err) { if (++i >= self._index.length) { - if (err) return self.onStatError(err) + if (err) return self.onFileSystemError(err) return self.error(404) } var p = join(path, self._index[i]) - debug('stat "%s"', p) - fs.stat(p, function (err, stat) { + fs.open(p, 'r', function onopen (err, fd) { if (err) return next(err) - if (stat.isDirectory()) return next() - self.emit('file', p, stat) - self.send(p, stat) + debug('stat fd "%d" for path "%s"', fd, p) + fs.fstat(fd, function (err, stat) { + if (err || stat.isDirectory()) { + return fs.close(fd, function (e) { + next(err || e) + }) + } + self.fd = fd + self.emit('file', p, stat) + self.emit('open', fd) + self.send(p, stat) + }) }) - } - - next() + })() } /** - * Stream `path` to the response. + * Stream to the response. * - * @param {String} path * @param {Object} options * @api private */ -SendStream.prototype.stream = function stream (path, options) { - // TODO: this is all lame, refactor meeee - var finished = false - var self = this - var res = this.res +SendStream.prototype.stream = function stream (options) { + options.fd = this.fd + options.autoClose = false - // pipe - var stream = fs.createReadStream(path, options) - this.emit('stream', stream) - stream.pipe(res) - - // response finished, done with the fd - onFinished(res, function onfinished () { - finished = true - destroy(stream) - }) + var stream = this._stream = new PartStream(options) - // error handling code-smell - stream.on('error', function onerror (err) { - // request already finished - if (finished) return - - // clean up stream - finished = true - destroy(stream) - - // error - self.onStatError(err) - }) + // error + stream.on('error', this.onFileSystemError) // end - stream.on('end', function onend () { - self.emit('end') - }) + stream.on('end', this.emit.bind(this, 'end')) + + // pipe + this.emit('stream', stream) + stream.pipe(this.res) } /** @@ -892,6 +953,17 @@ SendStream.prototype.setHeader = function setHeader (path, stat) { } } +util.inherits(PartStream, fs.ReadStream) + +function PartStream (opts) { + fs.ReadStream.call(this, null, opts) + this.bufferSize = opts.highWaterMark || this.bufferSize +} + +PartStream.prototype.destroy = PartStream.prototype.close = function closePartStream () { + this.readable = !(this.destroyed = this.closed = !(this.fd = null)) +} + /** * Clear all headers from a response. * diff --git a/package.json b/package.json index ced8bc8..051437e 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,6 @@ "dependencies": { "debug": "2.6.9", "depd": "~1.1.1", - "destroy": "~1.0.4", "encodeurl": "~1.0.1", "escape-html": "~1.0.3", "etag": "~1.8.1", diff --git a/test/send.js b/test/send.js index ec7ff0d..e716d4b 100644 --- a/test/send.js +++ b/test/send.js @@ -1,7 +1,7 @@ process.env.NO_DEPRECATION = 'send' -var after = require('after') +var afterCalls = require('after') // do not interfere with mocha's after() global var assert = require('assert') var fs = require('fs') var http = require('http') @@ -24,6 +24,52 @@ var app = http.createServer(function (req, res) { .pipe(res) }) +var noop = Function.prototype +var fsOpen = fs.open +var fsClose = fs.close +var fds = { opened: 0, closed: 0 } +// Before any of the tests run wrap `fs.open()` and `fs.close()` +// so we can test that all the file descriptors that get opened +// by `send()` are also closed properly after each response +before(function () { + // When `fs.open()` is called we wrap the last argument (the + // callback which receives the file descriptor) so that when + // it is eventually called we can increment the number of + // opened file descriptors + fs.open = function wrappedFSopen () { + var args = Array.prototype.slice.call(arguments) + var last = args.length - 1 + var done = args[last] + args[last] = function (err, fd) { + if (typeof fd === 'number') fds.opened++ + done(err, fd) + } + return fsOpen.apply(fs, args) + } + + // When `fs.close()` is called we increment the number of closed + // file descriptors immediately since closing is asynchronous and + // we check after each response that any opened file descriptors + // will eventually be closed, not necessarily that they *are* now + fs.close = function wrappedFSclose (fd, cb) { + if (typeof fd === 'number') fds.closed++ + return fsClose.call(fs, fd, cb) + } +}) + +// After all the tests have run then restore `fs.open()` +// and `fs.close()` to their original un-wrapped versions +after(function () { + fs.open = fsOpen + fs.close = fsClose +}) + +// After each test: ensure all opened file descriptors have been closed +afterEach(function () { + var reason = 'all opened file descriptors (' + fds.opened + ') should have been closed (' + fds.closed + ')' + assert.equal(fds.closed, fds.opened, reason) +}) + describe('send(file).pipe(res)', function () { it('should stream the file contents', function (done) { request(app) @@ -155,22 +201,24 @@ describe('send(file).pipe(res)', function () { }) }) - it('should 404 if file disappears after stat, before open', function (done) { + it('should 200 even if file disappears after stat', function (done) { + var resource = '/tmp.txt' + var tmpPath = path.join(__dirname, 'fixtures', resource) var app = http.createServer(function (req, res) { send(req, req.url, {root: 'test/fixtures'}) - .on('file', function () { - // simulate file ENOENT after on open, after stat - var fn = this.send - this.send = function (path, stat) { - fn.call(this, (path + '__xxx_no_exist'), stat) - } + .on('file', function (path) { + fs.unlinkSync(tmpPath) }) .pipe(res) }) - request(app) - .get('/name.txt') - .expect(404, done) + fs.writeFile(tmpPath, 'howdy', { flag: 'wx' }, function (err) { + if (err) return done(err) + request(app) + .get(resource) + .expect('Content-Type', /plain/) + .expect(200, done) + }) }) it('should 500 on file stream error', function (done) { @@ -192,7 +240,7 @@ describe('send(file).pipe(res)', function () { describe('"headers" event', function () { it('should fire when sending file', function (done) { - var cb = after(2, done) + var cb = afterCalls(2, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', function () { cb() }) @@ -205,7 +253,7 @@ describe('send(file).pipe(res)', function () { }) it('should not fire on 404', function (done) { - var cb = after(1, done) + var cb = afterCalls(1, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', function () { cb() }) @@ -218,7 +266,7 @@ describe('send(file).pipe(res)', function () { }) it('should fire on index', function (done) { - var cb = after(2, done) + var cb = afterCalls(2, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', function () { cb() }) @@ -231,7 +279,7 @@ describe('send(file).pipe(res)', function () { }) it('should not fire on redirect', function (done) { - var cb = after(1, done) + var cb = afterCalls(1, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', function () { cb() }) @@ -244,7 +292,7 @@ describe('send(file).pipe(res)', function () { }) it('should provide path', function (done) { - var cb = after(2, done) + var cb = afterCalls(2, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', onHeaders) @@ -263,7 +311,7 @@ describe('send(file).pipe(res)', function () { }) it('should provide stat', function (done) { - var cb = after(2, done) + var cb = afterCalls(2, done) var server = http.createServer(function (req, res) { send(req, req.url, {root: fixtures}) .on('headers', onHeaders) @@ -1014,6 +1062,82 @@ describe('send(file, options)', function () { }) }) + describe('fd', function () { + it('should support providing an existing file descriptor', function (done) { + var resource = '/nums' + fs.open(path.join(fixtures, resource), 'r', function (err, fd) { + if (err) return done(err) + request(createServer({fd: fd})) + .get(resource) + .expect(200, done) + }) + }) + + it('should still error if the fd cannot be streamed', function (done) { + request(createServer({fd: 999, autoClose: false})) + .get('/anything') + .expect(500, done) + }) + + it('should close the file descriptor if the response ends before pipe', function (done) { + var app = http.createServer(function (req, res) { + if (req.url === '/fd') { + fs.open(path.join(fixtures, '/name.txt'), 'r', function (err, fd) { + res.end() + if (err) return done(err) + send(req, req.url, {fd: fd}) + .on('close', done) + .pipe(res) + }) + } else { + res.end() + send(req, req.url, {root: 'test/fixtures'}) + .on('close', done) + .pipe(res) + } + }) + + request(app) + .get('/nums') + .expect(200, function (err) { + if (err) return done(err) + request(app) + .get('/fd') + .expect(200, function (err) { + if (err) done(err) + }) + }) + }) + + it('should close the file descriptor if the response ends immediately after pipe', function (done) { + var app = http.createServer(function (req, res) { + send(req, req.url, {root: 'test/fixtures'}) + .on('close', done) + .pipe(res) + res.end() + }) + + request(app) + .get('/name.txt') + .expect(200, noop) + }) + }) + + describe('autoClose', function () { + it('should prevent the file descriptor from being closed automatically if set to `false`', function (done) { + var resource = '/nums' + fs.open(path.join(fixtures, resource), 'r', function (err, fd) { + if (err) return done(err) + request(createServer({fd: fd, autoClose: false})) + .get(resource) + .expect(200, function (err) { + if (err) return done(err) + fs.close(fd, done) + }) + }) + }) + }) + describe('lastModified', function () { it('should support disabling last-modified', function (done) { request(createServer({lastModified: false, root: fixtures}))