Closes #1408, Closes #1409, Closes #1387, Closes #1410, Closes #1411, C…

…loses #1412, Closes #1413, Closes #1414
hapijs · Feb 7, 2014 · 7305863 · 7305863
1 parent b0cf3d8
commit 7305863
Show file tree

Hide file tree

Showing 10 changed files with 374 additions and 171 deletions.
diff --git a/docs/Reference.md b/docs/Reference.md
@@ -1036,14 +1036,14 @@ Each incoming request passes through a pre-defined set of steps, along with opti
 - Route prerequisites
 - Route handler
 - **`'onPostHandler'`** extension point
-    - The response contained in `request.response` may be modified. To return a different response type (for example, replace an error
-      with an HTML response), return a new response via `next(response)`.
+    - The response object contained in `request.response` may be modified (but not assigned a new value). To return a different response type
+      (for example, replace an error with an HTML response), return a new response via `next(response)`.
 - Validate response payload
 - **`'onPreResponse'`** extension point
     - always called.
-    - The response contained in `request.response` may be modified. To return a different response type (for example, replace an error
-      with an HTML response), return a new response via `next(response)`. Note that any errors generated after `next(response)` is called
-      will not be passed back to the `'onPreResponse'` extention method to prevent an infinite loop.
+    - The response contained in `request.response` may be modified (but not assigned a new value). To return a different response type (for
+      example, replace an error with an HTML response), return a new response via `next(response)`. Note that any errors generated after
+      `next(response)` is called will not be passed back to the `'onPreResponse'` extention method to prevent an infinite loop.
 - Send response (may emit `'internalError'` event)
 - Emits `'response'` event
 - Wait for tails

diff --git a/lib/directory.js b/lib/directory.js
@@ -86,16 +86,17 @@ exports.handler = function (route, options) {
                 return reply(Boom.notFound());
             }
 
-            File.load(path, {}, null, request, function (err, response) {
+            File.load(path, request, function (response) {
 
                 // File loaded successfully
 
-                if (!err) {
+                if (!response.isBoom) {
                     return reply(response);
                 }
 
                 // Not found
 
+                var err = response;
                 if (err.output.statusCode === 404) {
                     return next();
                 }
@@ -120,16 +121,17 @@ exports.handler = function (route, options) {
                 }
 
                 var indexFile = Path.join(path, 'index.html');
-                File.load(indexFile, {}, null, request, function (err, indexResponse) {
+                File.load(indexFile, request, function (indexResponse) {
 
                     // File loaded successfully
 
-                    if (!err) {
+                    if (!indexResponse.isBoom) {
                         return reply(indexResponse);
                     }
 
                     // Directory
 
+                    var err = indexResponse;
                     if (err.output.statusCode !== 404) {
                         return reply(Boom.badImplementation('index.html is a directory'));
                     }

diff --git a/lib/file.js b/lib/file.js
@@ -19,48 +19,34 @@ exports.handler = function (route, options) {
     var settings = (typeof options !== 'object' ? { path: options } : Utils.clone(options));                        // options can be reused
     Utils.assert(typeof settings.path !== 'string' || settings.path[settings.path.length - 1] !== '/', 'File path cannot end with a \'/\':', route.path);
 
-    var relativeTo = (route.env.path || route.server.settings.files.relativeTo);
-
-    // Normalize static string path
-
-    if (typeof settings.path === 'string') {
-        var staticPath = settings.path;
-        if (staticPath[0] !== '/') {
-            staticPath = Path.join(relativeTo, staticPath);
-        }
-    }
-
     var handler = function (request, reply) {
 
-        var path = null;
-
-        if (typeof settings.path === 'function') {
-            path = settings.path(request);
-
-            // Normalize function string path
-
-            if (path[0] !== '/') {
-                path = Path.join(relativeTo, path);
-            }
-        }
-        else {
-            path = staticPath;
-        }
-
+        var path = (typeof settings.path === 'function' ? settings.path(request) : settings.path);
         return reply.file(path, settings);
     };
 
     return handler;
 };
 
 
-exports.Response = internals.Response = function (path, settings, request) {
+exports.load = function (path, request, callback) {
+
+    var response = new internals.Response(path, {}, request);
+    return response._prepare(request, callback);
+};
+
+
+exports.Response = internals.Response = function (path, options, request) {
+
+    options = options || {};
+    Utils.assert(!options.mode || ['attachment', 'inline'].indexOf(options.mode) !== -1, 'options.mode must be either false, attachment, or inline');
 
     var relativeTo = (request._route.env.path || request.server.settings.files.relativeTo);
 
     var source = {
-        path: (path[0] === '/' ? path : Path.join(relativeTo, path)),
-        settings: settings
+        path: Path.normalize(path[0] === '/' ? path : Path.join(relativeTo, path)),
+        settings: options,
+        stat: null
     };
 
     Response.Plain.call(this, source, request, 'file');
@@ -69,19 +55,12 @@ exports.Response = internals.Response = function (path, settings, request) {
 Utils.inherits(internals.Response, Response.Plain);
 
 
-internals.Response.prototype._marshall = function (request, callback) {
+internals.Response.prototype._prepare = function (request, callback) {
 
-    return exports.load(this.source.path, this.source.settings, this, request, callback);      // Ignoring callback second argument
-};
-
-
-exports.load = function (path, options, response, request, callback) {
+    var self = this;
 
-    options = options || {};
-    Utils.assert(!options.mode || ['attachment', 'inline'].indexOf(options.mode) !== -1, 'options.mode must be either false, attachment, or inline');
-
-    var filePath = Path.normalize(path);
-    Fs.stat(filePath, function (err, stat) {
+    var path = this.source.path;
+    Fs.stat(path, function (err, stat) {
 
         if (err) {
             return callback(Boom.notFound());
@@ -91,62 +70,79 @@ exports.load = function (path, options, response, request, callback) {
             return callback(Boom.forbidden());
         }
 
-        // Prepare Stream response
-
-        var fileStream = Fs.createReadStream(filePath);
-        response = response || new internals.Response(path, options, request);
-        response._payload = fileStream;
+        self.source.stat = stat;
 
-        response.bytes(stat.size);
-        response.type(Mime.lookup(filePath) || 'application/octet-stream');
-        response._header('last-modified', stat.mtime);
+        self.bytes(stat.size);
+        self.type(Mime.lookup(path) || 'application/octet-stream');
+        self._header('last-modified', stat.mtime);
 
         if (request.server._etags) {
 
             // Use stat info for an LRU cache key.
 
-            var cachekey = [filePath, stat.ino, stat.size, stat.mtime.getTime()].join('-');
+            var cachekey = [path, stat.ino, stat.size, stat.mtime.getTime()].join('-');
 
             // The etag must hash the file contents in order to be consistent across distributed deployments
 
             var cachedEtag = request.server._etags.get(cachekey);
             if (cachedEtag) {
-                response._header('etag', cachedEtag);
+                self._header('etag', cachedEtag);
             }
             else {
                 var hash = Crypto.createHash('sha1');
-                response.on('peek', function (chunk) {
+                self.on('peek', function (chunk) {
 
                     hash.update(chunk);
                 });
 
-                response.once('finish', function () {
+                self.once('finish', function () {
 
                     var etag = hash.digest('hex');
                     request.server._etags.set(cachekey, etag);
                 });
             }
         }
 
-        if (options.mode) {
-            var fileName = Path.basename(filePath);
-            response._header('content-disposition', options.mode + '; filename=' + encodeURIComponent(fileName));
+        if (self.source.settings.mode) {
+            var fileName = Path.basename(path);
+            self._header('content-disposition', self.source.settings.mode + '; filename=' + encodeURIComponent(fileName));
         }
 
-        if (!options.lookupCompressed) {
-            return callback(null, response);
+        return callback(self);
+    });
+};
+
+
+internals.Response.prototype._marshall = function (request, callback) {
+
+    var self = this;
+
+    // Prepare Stream response
+
+    var path = this.source.path;
+    var fileStream = Fs.createReadStream(path);
+    fileStream.once('error', callback);
+    fileStream.once('open', function () {
+
+        fileStream.removeListener('error', callback);
+
+        self._payload = fileStream;
+
+        if (!self.source.settings.lookupCompressed) {
+            return callback();
         }
 
-        var gzFile = filePath + '.gz';
+        var gzFile = path + '.gz';
         Fs.stat(gzFile, function (err, stat) {
 
             if (!err &&
                 !stat.isDirectory()) {
 
-                fileStream._hapi = { gzipped: Fs.createReadStream(gzFile) };
+                var gzipped = Fs.createReadStream(gzFile);
+                fileStream._hapi = { gzipped: gzipped };
             }
 
-            return callback(null, response);
+            return callback();
         });
     });
 };
diff --git a/lib/handler.js b/lib/handler.js
@@ -156,31 +156,39 @@ internals.wrap = function (result, request, finalize) {
     var response = Response.wrap(result, request);
 
     if (response.isBoom) {
-        finalize(response)
+        return finalize(response)
     }
-    else {
-        response.hold = function () {
 
-            response.hold = undefined;
+    var prepare = function () {
 
-            response.send = function () {
+        if (response._prepare) {
+            return response._prepare(request, finalize);
+        }
+
+        return finalize(response);
+    };
 
-                response.send = undefined;
-                finalize(response);
-            };
+    response.hold = function () {
 
-            return response;
+        response.hold = undefined;
+
+        response.send = function () {
+
+            response.send = undefined;
+            prepare();
         };
 
-        process.nextTick(function () {
+        return response;
+    };
 
-            response.hold = undefined;
+    process.nextTick(function () {
 
-            if (!response.send) {
-                finalize(response);
-            }
-        });
-    }
+        response.hold = undefined;
+
+        if (!response.send) {
+            prepare();
+        }
+    });
 
     return response;
 };

diff --git a/lib/request.js b/lib/request.js
@@ -403,6 +403,14 @@ internals.Request.prototype._setResponse = function (response) {
 
     var self = this;
 
+    if (this.response &&
+        !this.response.isBoom &&
+        this.response !== response &&
+        (response.isBoom || this.response.source !== response.source)) {
+
+        this.response._close();
+    }
+
     this.response = response;
 
     if (response.isBoom &&