From a59cfc70a160aac4247f97d869983fa69e2947ad Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Sun, 27 Sep 2015 20:14:09 +0100
Subject: [PATCH] Serve immediate 404 for static files (no fallthru)

closes #5887

- make use of the new 'fallthrough' option which landed in express-static 1.10.0
- change local-file-store and middleware serving `/public/` and `/shared/` files to use the new `fallthrough: false` option
- 404s are now served directly, without slashes or uncapitalise getting triggered
---
 core/server/middleware/index.js              | 17 ++++++++++++----
 core/server/storage/local-file-store.js      | 21 ++++++++++----------
 core/test/functional/routes/frontend_spec.js |  8 ++++++++
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js
index f0b4bdf944..b73a72f65f 100644
--- a/core/server/middleware/index.js
+++ b/core/server/middleware/index.js
@@ -1,10 +1,10 @@
 var bodyParser      = require('body-parser'),
     config          = require('../config'),
     errors          = require('../errors'),
-    express         = require('express'),
     logger          = require('morgan'),
     path            = require('path'),
     routes          = require('../routes'),
+    serveStatic     = require('express').static,
     slashes         = require('connect-slashes'),
     storage         = require('../storage'),
     passport        = require('passport'),
@@ -78,9 +78,15 @@ setupMiddleware = function setupMiddleware(blogApp, adminApp) {
     blogApp.use(serveSharedFile('shared/ghost-url.min.js', 'application/javascript', utils.ONE_HOUR_S));
 
     // Static assets
-    blogApp.use('/shared', express.static(path.join(corePath, '/shared'), {maxAge: utils.ONE_HOUR_MS}));
+    blogApp.use('/shared', serveStatic(
+        path.join(corePath, '/shared'),
+        {maxAge: utils.ONE_HOUR_MS, fallthrough: false}
+    ));
     blogApp.use('/content/images', storage.getStorage().serve());
-    blogApp.use('/public', express.static(path.join(corePath, '/built/public'), {maxAge: utils.ONE_YEAR_MS}));
+    blogApp.use('/public', serveStatic(
+        path.join(corePath, '/built/public'),
+        {maxAge: utils.ONE_YEAR_MS, fallthrough: false}
+    ));
 
     // First determine whether we're serving admin or theme content
     blogApp.use(decideIsAdmin);
@@ -88,7 +94,10 @@ setupMiddleware = function setupMiddleware(blogApp, adminApp) {
     blogApp.use(themeHandler.configHbsForContext);
 
     // Admin only config
-    blogApp.use('/ghost', express.static(config.paths.clientAssets, {maxAge: utils.ONE_YEAR_MS}));
+    blogApp.use('/ghost', serveStatic(
+        config.paths.clientAssets,
+        {maxAge: utils.ONE_YEAR_MS}
+    ));
 
     // Force SSL
     // NOTE: Importantly this is _after_ the check above for admin-theme static resources,
diff --git a/core/server/storage/local-file-store.js b/core/server/storage/local-file-store.js
index d1b4d39511..39f9436a27 100644
--- a/core/server/storage/local-file-store.js
+++ b/core/server/storage/local-file-store.js
@@ -1,15 +1,15 @@
 // # Local File System Image Storage module
 // The (default) module for storing images, using the local file system
 
-var express   = require('express'),
-    fs        = require('fs-extra'),
-    path      = require('path'),
-    util      = require('util'),
-    Promise   = require('bluebird'),
-    errors    = require('../errors'),
-    config    = require('../config'),
-    utils     = require('../utils'),
-    baseStore = require('./base');
+var serveStatic = require('express').static,
+    fs          = require('fs-extra'),
+    path        = require('path'),
+    util        = require('util'),
+    Promise     = require('bluebird'),
+    errors      = require('../errors'),
+    config      = require('../config'),
+    utils       = require('../utils'),
+    baseStore   = require('./base');
 
 function LocalFileStore() {
 }
@@ -52,7 +52,8 @@ LocalFileStore.prototype.exists = function (filename) {
 // middleware for serving the files
 LocalFileStore.prototype.serve = function () {
     // For some reason send divides the max age number by 1000
-    return express.static(config.paths.imagesPath, {maxAge: utils.ONE_YEAR_MS});
+    // Fallthrough: false ensures that if an image isn't found, it automatically 404s
+    return serveStatic(config.paths.imagesPath, {maxAge: utils.ONE_YEAR_MS, fallthrough: false});
 };
 
 module.exports = LocalFileStore;
diff --git a/core/test/functional/routes/frontend_spec.js b/core/test/functional/routes/frontend_spec.js
index 3efe9cd248..720e88a288 100644
--- a/core/test/functional/routes/frontend_spec.js
+++ b/core/test/functional/routes/frontend_spec.js
@@ -86,6 +86,14 @@ describe('Frontend Routing', function () {
                     .expect(/Page not found/)
                     .end(doEnd(done));
             });
+
+            it('should 404 for unknown file', function (done) {
+                request.get('/content/images/some/file/that/doesnt-exist.jpg')
+                    .expect('Cache-Control', testUtils.cacheRules['private'])
+                    .expect(404)
+                    .expect(/Page not found/)
+                    .end(doEnd(done));
+            });
         });
 
         describe('Single post', function () {