Allow specifying file descriptor and use fs.open and fs.fstat

Resolves pillarjs#122
Resolves pillarjs#123

README.md

@@ -44,6 +44,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
@@ -83,6 +91,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.
+
 ##### index
 
 By default send supports "index.html" files, to disable this
@@ -116,9 +129,11 @@ The `SendStream` is an event emitter and will emit the following events:
   - `error` an error occurred `(err)`
   - `directory` a directory was requested
   - `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
 

index.js

@@ -166,9 +166,15 @@ function SendStream (req, path, options) {
     ? resolve(opts.root)
     : null
 
+  this.fd = typeof opts.fd === 'number'
+    ? opts.fd
+    : null
+
   if (!this._root && opts.from) {
     this.from(opts.from)
   }
+
+  this.onFileSystemError = this.onFileSystemError.bind(this)
 }
 
 /**
@@ -374,7 +380,7 @@ 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
 }
 
 /**
@@ -384,7 +390,7 @@ SendStream.prototype.isCachable = function isCachable () {
  * @private
  */
 
-SendStream.prototype.onStatError = function onStatError (error) {
+SendStream.prototype.onFileSystemError = function onFileSystemError (error) {
   switch (error.code) {
     case 'ENAMETOOLONG':
     case 'ENOENT':
@@ -469,10 +475,33 @@ 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
+  onFinished(res, function onfinished () {
+    var fd = self.fd
+    var autoClose = self.options.autoClose
+    if (self._stream) destroy(self._stream)
+    if (typeof fd === 'number' && autoClose !== false) {
+      fs.close(fd, function () {
+        self.fd = null
+        self.emit('close')
+      })
+    }
+  })
+
+  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) {
@@ -573,7 +602,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)
@@ -664,34 +693,41 @@ 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(self.path)
-    self.emit('file', path, stat)
-    self.send(path, stat)
+  var redirect = this.redirect.bind(this, this.path)
+
+  debug('open "%s"', path)
+  fs.open(path, 'r', function onopen (err, fd) {
+    return !err
+      ? sendStats(path, fd, self.onFileSystemError, redirect)
+      : 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 () { /* istanbul ignore next */
+        return err ? onError(err) : onDirectory()
+      })
+      self.fd = fd
+      self.emit('file', path, stat)
+      self.emit('open', fd)
+      self.send(path, stat)
     })
   }
 }
@@ -702,28 +738,33 @@ 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 () {
+          next(err)
+        })
+        self.fd = fd
+        self.emit('file', p, stat)
+        self.emit('open', fd)
+        self.send(p, stat)
+      })
     })
-  }
-
-  next()
+  })()
 }
 
 /**
@@ -735,39 +776,27 @@ SendStream.prototype.sendIndex = function sendIndex (path) {
  */
 
 SendStream.prototype.stream = function stream (path, options) {
-  // TODO: this is all lame, refactor meeee
-  var finished = false
-  var self = this
-  var res = this.res
-
-  // 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)
-  })
+  options.fd = this.fd
+  options.autoClose = false
 
-  // error handling code-smell
-  stream.on('error', function onerror (err) {
-    // request already finished
-    if (finished) return
-
-    // clean up stream
-    finished = true
-    destroy(stream)
+  var self = this
+  var stream = this._stream = fs.createReadStream(path, options)
 
-    // error
-    self.onStatError(err)
+  // error
+  stream.on('error', function onerror (e) {
+    stream.fd = null
+    self.onFileSystemError(e)
   })
 
   // end
   stream.on('end', function onend () {
+    stream.fd = null
     self.emit('end')
   })
+
+  // pipe
+  this.emit('stream', stream)
+  stream.pipe(this.res)
 }
 
 /**

test/send.js

@@ -24,6 +24,40 @@ var app = http.createServer(function (req, res) {
   .pipe(res)
 })
 
+var fsOpen = fs.open
+var fsClose = fs.close
+var fds = {
+  opened: 0,
+  closed: 0
+}
+
+before(function () {
+  fs.open = function () {
+    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)
+  }
+  fs.close = function (fd, cb) {
+    fds.closed++
+    return fsClose.call(fs, fd, cb)
+  }
+})
+
+after(function () {
+  fs.open = fsOpen
+  fs.close = fsClose
+})
+
+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,24 +189,25 @@ 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) {
-          path += '__xxx_no_exist'
-          fn.call(this, path, stat)
-        }
+      .on('file', function (path) {
+        fs.unlinkSync(tmpPath)
       })
       .pipe(res)
     })
 
-    request(app)
-    .get('/name.txt')
-    .expect('Content-Type', /plain/)
-    .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) {
@@ -873,6 +908,39 @@ 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)
+    })
+  })
+
+  describe('autoClose', function () {
+    it('should prevent the file descriptor from being closed automatically', 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}))