set Vary header when using built-in header checking

History.md

@@ -8,6 +8,7 @@ unreleased
   * Only `POST` requests are examined by default
   * Remove `req.body` support for more standard query param support
     - Use custom `getter` function if `req.body` support is needed
+  * Set `Vary` header when using built-in header checking
 
 1.0.2 / 2014-05-22
 ==================

README.md

@@ -33,8 +33,9 @@ value will be stored in `req.originalMethod`.
 #### getter
 
 This is the method of getting the override value from the request. If a function is provided,
-the `req` is passed as the first argument and the method is expected to be returned. If a
-string is provided, the string is used to look up the method with the following rules:
+the `req` is passed as the first argument, the `res as the second argument and the method is
+expected to be returned. If a string is provided, the string is used to look up the method
+with the following rules:
 
 - If the string starts with `X-`, then it is treated as the name of a header and that header
   is used for the method override. If the request contains the same header multiple times, the
@@ -94,7 +95,7 @@ var connect        = require('connect')
 var methodOverride = require('method-override')
 
 app.use(bodyParser.urlencoded())
-app.use(methodOverride(function(req){
+app.use(methodOverride(function(req, res){
   if (req.body && typeof req.body === 'object' && '_method' in req.body) {
     // look in urlencoded POST bodies and delete it
     var method = req.body._method

index.js

@@ -59,7 +59,7 @@ module.exports = function methodOverride(getter, options){
       return next()
     }
 
-    val = get(req)
+    val = get(req, res)
     method = Array.isArray(val)
       ? val[0]
       : val
@@ -91,7 +91,7 @@ function createGetter(str) {
  */
 
 function createQueryGetter(key) {
-  return function(req) {
+  return function(req, res) {
     var url = parseurl(req)
     var query = querystring.parse(url.query || '')
     return query[key]
@@ -105,7 +105,10 @@ function createQueryGetter(key) {
 function createHeaderGetter(str) {
   var header = str.toLowerCase()
 
-  return function(req) {
+  return function(req, res) {
+    // set appropriate Vary header
+    vary(res, str)
+
     // multiple headers get joined with comma by node.js core
     return (req.headers[header] || '').split(/ *, */)
   }
@@ -120,3 +123,29 @@ function supports(method) {
     && typeof method === 'string'
     && methods.indexOf(method.toLowerCase()) !== -1
 }
+
+/**
+ * Add val to Vary header
+ */
+
+function vary(res, val) {
+  var header = res.getHeader('Vary') || ''
+  var headers = Array.isArray(header)
+    ? header.join(', ')
+    : header
+
+  // enumerate current values
+  var vals = headers.toLowerCase().split(/ *, */)
+
+  if (vals.indexOf(val.toLowerCase()) !== -1) {
+    // already set
+    return
+  }
+
+  // append value (in existing format)
+  header = headers
+    ? headers + ', ' + val
+    : val
+
+  res.setHeader('Vary', header)
+}

test/test.js

@@ -102,6 +102,47 @@ describe('methodOverride(getter)', function(){
       .set('X-HTTP-Method-Override', 'DELETE, PUT')
       .expect('X-Got-Method', 'DELETE', done)
     })
+
+    it('should set Vary header', function(done){
+      request(server)
+      .post('/')
+      .set('Content-Type', 'application/json')
+      .set('X-HTTP-Method-Override', 'DELETE')
+      .expect('Vary', 'X-HTTP-Method-Override')
+      .expect('X-Got-Method', 'DELETE', done)
+    })
+
+    it('should set Vary header even with no override', function(done){
+      request(server)
+      .post('/')
+      .set('Content-Type', 'application/json')
+      .expect('Vary', 'X-HTTP-Method-Override')
+      .expect('X-Got-Method', 'POST', done)
+    })
+
+    it('should set Vary header when header is array', function(done){
+      var server = createServer('X-HTTP-Method-Override', null, function(req, res){
+        res.setHeader('Vary', ['User-Agent', 'Accept-Encoding'])
+      })
+
+      request(server)
+      .post('/')
+      .set('Content-Type', 'application/json')
+      .expect('Vary', 'User-Agent, Accept-Encoding, X-HTTP-Method-Override')
+      .expect('X-Got-Method', 'POST', done)
+    })
+
+    it('should not double-set Vary header', function(done){
+      var server = createServer('X-HTTP-Method-Override', null, function(req, res){
+        res.setHeader('Vary', 'X-HTTP-Method-Override')
+      })
+
+      request(server)
+      .post('/')
+      .set('Content-Type', 'application/json')
+      .expect('Vary', 'X-HTTP-Method-Override')
+      .expect('X-Got-Method', 'POST', done)
+    })
   })
 
   describe('with function', function(){
@@ -165,9 +206,10 @@ describe('methodOverride(getter)', function(){
   })
 })
 
-function createServer(getter, opts) {
+function createServer(getter, opts, fn) {
   var _override = methodOverride(getter, opts)
   return http.createServer(function (req, res) {
+    fn && fn(req, res)
     _override(req, res, function (err) {
       res.statusCode = err ? 500 : 200
       res.setHeader('X-Got-Method', req.method)