From f592d8a884084c39b002a38b84fbbaddaf82dc66 Mon Sep 17 00:00:00 2001 From: Ben Weintraub Date: Tue, 25 Sep 2012 13:27:27 -0700 Subject: [PATCH] Fix for requests having both If-None-Match && If-Modified-Since Previously, requests having both the If-None-Match and If-Modified-Since headers would be serviced with a 304 response if either condition were to pass. This is not correct per RFC 2616, section 14.26 (see below). Also, added a new test to verify behavior with If-None-Match condition fails, but If-Modified-Since condition passes. From http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.26 : "If none of the entity tags match, then the server MAY perform the requested method as if the If-None-Match header field did not exist, but MUST also ignore any If-Modified-Since header field(s) in the request. That is, if no entity tags match, then the server MUST NOT return a 304 (Not Modified) response." --- lib/node-static.js | 13 ++++++++----- test/integration/node-static-test.js | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lib/node-static.js b/lib/node-static.js index 136311b..db16685 100644 --- a/lib/node-static.js +++ b/lib/node-static.js @@ -180,9 +180,11 @@ this.Server.prototype.serve = function (req, res, callback) { }; this.Server.prototype.respond = function (pathname, status, _headers, files, stat, req, res, finish) { - var mtime = Date.parse(stat.mtime), - key = pathname || files[0], - headers = {}; + var mtime = Date.parse(stat.mtime), + key = pathname || files[0], + headers = {}, + clientETag = req.headers['if-none-match'], + clientMTime = Date.parse(req.headers['if-modified-since']); // Copy default headers for (var k in this.options.headers) { headers[k] = this.options.headers[k] } @@ -194,8 +196,9 @@ this.Server.prototype.respond = function (pathname, status, _headers, files, sta // Conditional GET // If the "If-Modified-Since" or "If-None-Match" headers // match the conditions, send a 304 Not Modified. - if (req.headers['if-none-match'] === headers['ETag'] || - Date.parse(req.headers['if-modified-since']) >= mtime) { + if ((clientMTime || clientETag) && + (!clientETag || clientETag === headers['ETag']) && + (!clientMTime || clientMTime >= mtime)) { finish(304, headers); } else { var fileExtension = path.extname(files[0]).slice(1).toLowerCase(); diff --git a/test/integration/node-static-test.js b/test/integration/node-static-test.js index 5501343..0d65cfc 100644 --- a/test/integration/node-static-test.js +++ b/test/integration/node-static-test.js @@ -90,6 +90,28 @@ suite.addBatch({ 'should respond with 304' : function(error, response, body){ assert.equal(response.statusCode, 304); } + }, + 'requesting with If-None-Match and If-Modified-Since': { + topic : function(){ + var _this = this; + request.get(TEST_SERVER + '/index.html', function(error, response, body){ + var modified = Date.parse(response.headers['last-modified']); + var oneDayLater = new Date(modified + (24 * 60 * 60 * 1000)).toUTCString(); + var nonMatchingEtag = '1111222233334444'; + request({ + method: 'GET', + uri: TEST_SERVER + '/index.html', + headers: { + 'if-none-match': nonMatchingEtag, + 'if-modified-since': oneDayLater + } + }, + _this.callback); + }); + }, + 'should respond with a 200': function(error, response, body){ + assert.equal(response.statusCode, 200); + } } }).addBatch({ 'requesting HEAD': {