diff --git a/config/local.json b/config/local.json index 960bdf2e9f0c71700529ca8dd2b4c0afa43f4a00..c85c8f828e0e682b5aae8f6d3f8c6d6e6de2dd42 100644 --- a/config/local.json +++ b/config/local.json @@ -9,6 +9,6 @@ "driver": "json" }, "express_log_format": "dev_bid", - "email_to_console": true + "email_to_console": true, + "env": "local" } - diff --git a/lib/browserid/views.js b/lib/browserid/views.js index 0ca111100a1211e636c1e17cda6a818486223e0b..9380f9b5ddc22723e033ecc6445029e17c629487 100644 --- a/lib/browserid/views.js +++ b/lib/browserid/views.js @@ -11,14 +11,14 @@ connect = require('connect'), config = require('../configuration.js'), und = require('underscore'), util = require('util'), -httputils = require('../httputils.js'); +httputils = require('../httputils.js'), +etagify = require('etagify'); // all templated content, redirects, and renames are handled here. // anything that is not an api, and not static const path = require('path'); - const VIEW_PATH = path.join(__dirname, "..", "..", "resources", "views"); // none of our views include dynamic data. all of them should be served @@ -26,27 +26,23 @@ const VIEW_PATH = path.join(__dirname, "..", "..", "resources", "views"); // cache headers maximally leveraging the same logic that connect uses // issue #910 function renderCachableView(req, res, template, options) { - fs.stat(path.join(VIEW_PATH, template), function (err, stat) { - res.setHeader('Date', new Date().toUTCString()); - res.setHeader('Vary', 'Accept-Encoding,Accept-Language'); - if (config.get('env') !== 'local') { - // allow caching, but require revalidation via ETag - res.setHeader('Cache-Control', 'public, max-age=0'); - res.setHeader('ETag', util.format('"%s-%s-%s"', stat.size, Number(stat.mtime), req.lang)); - } else { - res.setHeader('Cache-Control', 'no-store'); - } - res.setHeader('Content-Type', 'text/html; charset=utf8'); - if (connect.utils.conditionalGET(req)) { - if (!connect.utils.modified(req, res)) { - return connect.utils.notModified(res); - } - } - res.render(template, options); - }); + if (config.get('env') !== 'local') { + // allow caching, but require revalidation via ETag + res.etagify(); + res.setHeader('Cache-Control', 'public, max-age=0'); + } else { + // disable all caching for local dev + res.setHeader('Cache-Control', 'no-store'); + } + res.setHeader('Date', new Date().toUTCString()); + res.setHeader('Vary', 'Accept-Encoding,Accept-Language'); + res.setHeader('Content-Type', 'text/html; charset=utf8'); + res.render(template, options); } exports.setup = function(app) { + app.use(etagify()); + app.set("views", VIEW_PATH); app.set('view options', { diff --git a/package.json b/package.json index 2a86e4b4d27c501247ba86a64eb4945666c497f3..ad3bd2c8b2655e1d03ba2be29b1ae03333a4dc2c 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "connect-cookie-session": "0.0.2", "connect-logger-statsd": "0.0.1", "ejs": "0.4.3", + "etagify": "0.0.1", "express": "2.5.0", "iconv": "1.1.3", "jwcrypto": "0.1.1", diff --git a/tests/cache-header-tests.js b/tests/cache-header-tests.js index 690af835b47845bf58764b9a58668b2665b9f603..ab9882b78ab0f0442701c13d08b869367b24ac31 100755 --- a/tests/cache-header-tests.js +++ b/tests/cache-header-tests.js @@ -48,7 +48,19 @@ function doRequest(path, headers, cb) { function hasProperCacheHeaders(path) { return { topic: function() { - doRequest(path, {}, this.callback); + var self = this; + // note we do *two* requests to the same resource. The way + // etagify works is to generate content based hashes on the first + // request, and then use them every subsequent request. This + // minimizes complexity and buffering that we do, at the cost of + // the first client after server restart possibly getting a couple + // extra kilobytes over the wire in a 200-that-shoulda-been-a-304. + // See issue #1331 and https://github.com/lloyd/connect-etagify + // for more context. + doRequest(path, {}, function(err, r) { + if (err) self.callback(err, r); + else doRequest(path, {}, self.callback); + }); }, "returns 200 with content": function(err, r) { assert.strictEqual(r.statusCode, 200);