Merge pull request #19 from mtkopone/fix-bogus-sig-and-set-cookie-ove…

…rrides

Fix bogus sig problem add overwrite-flag

README.md

@@ -62,6 +62,7 @@ If the _options_ object is provided, it will be used to generate the outbound co
 * `secureProxy`: a boolean indicating whether the cookie is only to be sent over HTTPS (use this if you handle SSL not in your node process).
 * `httpOnly`: a boolean indicating whether the cookie is only to be sent over HTTP(S), and not made available to client JavaScript (`true` by default).
 * `signed`: a boolean indicating whether the cookie is to be signed (`false` by default). If this is true, another cookie of the same name with the `.sig` suffix appended will also be sent, with a 27-byte url-safe base64 SHA1 value representing the hash of _cookie-name_=_cookie-value_ against the first [Keygrip](https://github.com/jed/keygrip) key. This signature key is used to detect tampering the next time a cookie is received.
+* `overwrite`: a boolean indicating whether to overwrite previously set cookies of the same name (`false` by default). If this is true, all cookies set during the same request with the same name (regardless of path or domain) are filtered out of the Set-Cookie header when setting this cookie.
 
 ## Example
 

lib/cookies.js

@@ -1,3 +1,4 @@
+var http = require('http')
 var cache = {}
 
 function Cookies(request, response, keys) {
@@ -27,18 +28,18 @@ Cookies.prototype = {
     data = name + "=" + value
     index = this.keys.index(data, remote)
 
-    if (index < 0) this.set(sigName, null, {path: "/"})
-
-    else {
-      index && this.set(sigName, this.keys.sign(data))
+    if (index < 0) {
+      this.set(sigName, null, {path: "/", signed: false })
+    } else {
+      index && this.set(sigName, this.keys.sign(data), { signed: false })
       return value
     }
   },
 
   set: function(name, value, opts) {
     var res = this.response
       , req = this.request
-      , headers = res.set ? [] : res.getHeader("Set-Cookie") || []
+      , headers = res.getHeader("Set-Cookie") || []
       , secure = req.connection.encrypted
       , cookie = new Cookie(name, value, opts)
       , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys
@@ -50,15 +51,16 @@ Cookies.prototype = {
     cookie.secure = secure
     if (opts && "secure" in opts) cookie.secure = opts.secure
     if (opts && "secureProxy" in opts) cookie.secure = opts.secureProxy
-    headers.push(cookie.toHeader())
+    headers = pushCookie(headers, cookie)
 
     if (opts && signed) {
       cookie.value = this.keys.sign(cookie.toString())
       cookie.name += ".sig"
-      headers.push(cookie.toHeader())
+      headers = pushCookie(headers, cookie)
     }
 
-    res.setHeader("Set-Cookie", headers)
+    var setHeader = res.set ? http.OutgoingMessage.prototype.setHeader : res.setHeader
+    setHeader.call(res, 'set-cookie', headers)
     return this
   }
 }
@@ -78,6 +80,7 @@ Cookie.prototype = {
   domain: undefined,
   httpOnly: true,
   secure: false,
+  overwrite: false,
 
   toString: function() {
     return this.name + "=" + this.value
@@ -106,6 +109,14 @@ function getPattern(name) {
   )
 }
 
+function pushCookie(cookies, cookie) {
+  if (cookie.overwrite) {
+    cookies = cookies.filter(function(c) { return c.indexOf(cookie.name+'=') !== 0 })
+  }
+  cookies.push(cookie.toHeader())
+  return cookies
+}
+
 Cookies.connect = Cookies.express = function(keys) {
   return function(req, res, next) {
     req.cookies = res.cookies = new Cookies(req, res, keys)

test/express.js

@@ -13,7 +13,7 @@ app.use( cookies( keys ) )
 app.get( "/set", function(req, res) {
   res.cookies
     // set a regular cookie
-    .set( "unsigned", "foo", { httpOnly: false } )
+    .set( "unsigned", "foo", { signed:false, httpOnly: false } )
 
     // set a signed cookie
     .set( "signed", "bar", { signed: true } )
@@ -22,6 +22,10 @@ app.get( "/set", function(req, res) {
     .set( "tampered", "baz" )
     .set( "tampered.sig", "bogus" )
 
+    // set a cookie that will be overwritten
+    .set( "overwrite", "old-value", { signed: true } )
+    .set( "overwrite", "new-value", { overwrite: true, signed: true } )
+
   res.writeHead(302, {Location: "/"})
   res.end()
 })
@@ -30,11 +34,18 @@ app.get("/", function(req, res) {
   var unsigned = req.cookies.get( "unsigned" )
     , signed = req.cookies.get( "signed", { signed: true } )
     , tampered = req.cookies.get( "tampered", { signed: true } )
+    , overwrite = req.cookies.get( "overwrite", { signed: true } )
 
   assert.equal( unsigned, "foo" )
+  assert.equal( req.cookies.get( "unsigned.sig", { signed:false } ), undefined)
   assert.equal( signed, "bar" )
+  assert.equal( req.cookies.get( "signed.sig", { signed: false } ), keys.sign('signed=bar') )
   assert.notEqual( tampered, "baz" )
   assert.equal( tampered, undefined )
+  assert.equal( overwrite, "new-value" )
+  assert.equal( req.cookies.get( "overwrite.sig", { signed:false } ), keys.sign('overwrite=new-value') )
+
+  assert.equal(res.getHeader('Set-Cookie'), 'tampered.sig=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly')
 
   res.send(
     "unsigned expected: foo\n" +
@@ -56,7 +67,7 @@ http.get( options, function( res ) {
 
   console.log( "\ncookies set:", header )
   console.log( "\n============\n" )
-  assert.equal(header.length, 6)
+  assert.equal(header.length, 7)
 
   options.path = res.headers[ "Location" ]
   options.headers = { "Cookie": header.join(";") }

test/http.js

@@ -9,12 +9,12 @@ var assert = require( "assert" )
 
 server = http.createServer( function( req, res ) {
   var cookies = new Cookies( req, res, keys )
-    , unsigned, signed, tampered
+    , unsigned, signed, tampered, overwrite
 
   if ( req.url == "/set" ) {
     cookies
       // set a regular cookie
-      .set( "unsigned", "foo", { httpOnly: false } )
+      .set( "unsigned", "foo", { signed:false, httpOnly: false } )
 
       // set a signed cookie
       .set( "signed", "bar", { signed: true } )
@@ -23,18 +23,29 @@ server = http.createServer( function( req, res ) {
       .set( "tampered", "baz" )
       .set( "tampered.sig", "bogus" )
 
+      // set a cookie that will be overwritten
+      .set( "overwrite", "old-value", { signed: true } )
+      .set( "overwrite", "new-value", { overwrite: true, signed: true } )
+
     res.writeHead( 302, { "Location": "/" } )
     return res.end( "Now let's check." )
   }
 
   unsigned = cookies.get( "unsigned" )
   signed = cookies.get( "signed", { signed: true } )
   tampered = cookies.get( "tampered", { signed: true } )
-
+  overwrite = cookies.get( "overwrite", { signed: true } )
+
   assert.equal( unsigned, "foo" )
+  assert.equal( cookies.get( "unsigned.sig", { signed:false } ), undefined)
   assert.equal( signed, "bar" )
+  assert.equal( cookies.get( "signed.sig", { signed: false } ), keys.sign('signed=bar') )
   assert.notEqual( tampered, "baz" )
   assert.equal( tampered, undefined )
+  assert.equal( overwrite, "new-value" )
+  assert.equal( cookies.get( "overwrite.sig", { signed:false } ), keys.sign('overwrite=new-value') )
+
+  assert.equal(res.getHeader('Set-Cookie'), 'tampered.sig=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly')
 
   res.writeHead( 200, { "Content-Type": "text/plain" } )
   res.end(
@@ -52,10 +63,10 @@ server.listen( 8000 )
 http.get( options, function( res ) {
   var cookies = res.headers[ "set-cookie" ]
     , body = ""
-  
+
   console.log( "\ncookies set:", cookies )
   console.log( "\n============\n" )
-  assert.equal(cookies.length, 6)
+  assert.equal(cookies.length, 7)
 
   options.path = res.headers[ "location" ]
   options.headers = { "Cookie": cookies.join(";") }