Permalink
Browse files

Add opts.overwrite to clear out previously set cookies of the same name

This requires that the cookie header array is filtered when this flag is set. Since connect/express have custom handling for setHeader('Set-Cookie'), we need to bypass that to be able to fully control which cookies get written to the response. Hence the crazy prototype setup, to find a "working" setHeader without connect/express magic.

Caveat: Currently overwrites all cookies with the same name, regardless of path or domain. This is why I didn't make this global behaviour, so now the default is to behave as previously, but give the user the option to clear out any duplicates out of the request.

Reasoning for the whole commit: we can have a situation where one of our signed authentication cookies is corrupt (e.g. bad sig) but it's "parent" cookie is OK so we can regenerate the corrupt cookie by checking the parent cookie with a SSO-service. In this case, the cookies.get('child') tries to clear the 'child.sig'-cookie, with a 'Set-Cookie' with an empty value. Later, the SSO-check rewrites a correct 'child' cookie with it's sig. Without the overwrite flag, the .sig cookie gets duplicated into the 'Set-Cookie' header.
  • Loading branch information...
1 parent 73aa92d commit 762a836b2dd78802e5a5508a37e7248ab47d584a @mtkopone mtkopone committed Nov 13, 2012
Showing with 38 additions and 9 deletions.
  1. +1 −0 README.md
  2. +14 −4 lib/cookies.js
  3. +10 −1 test/express.js
  4. +13 −4 test/http.js
View
@@ -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
View
@@ -38,7 +38,7 @@ Cookies.prototype = {
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 +50,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 ? res.__proto__.__proto__.__proto__.__proto__.setHeader : res.setHeader
+ setHeader.call(res, 'set-cookie', headers)
@scriby

scriby Apr 12, 2013

Contributor

Did the casing need to change here? This is causing a proxy I'm working with to behave strangely.

@jed

jed Apr 12, 2013

Contributor

this is definitely a proxy bug, but i'd welcome a PR if you'd like to revert it.

return this
}
}
@@ -78,6 +79,7 @@ Cookie.prototype = {
domain: undefined,
httpOnly: true,
secure: false,
+ overwrite: false,
toString: function() {
return this.name + "=" + this.value
@@ -106,6 +108,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)
View
@@ -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,12 +34,17 @@ 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(
@@ -58,7 +67,7 @@ http.get( options, function( res ) {
console.log( "\ncookies set:", header )
console.log( "\n============\n" )
- assert.equal(header.length, 5)
+ assert.equal(header.length, 7)
options.path = res.headers[ "Location" ]
options.headers = { "Cookie": header.join(";") }
View
@@ -9,7 +9,7 @@ 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
@@ -23,19 +23,28 @@ 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" } )
@@ -54,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, 5)
+ assert.equal(cookies.length, 7)
options.path = res.headers[ "location" ]
options.headers = { "Cookie": cookies.join(";") }

0 comments on commit 762a836

Please sign in to comment.