From 11d0f741152e1cc25d859975be7fe8dfbd2012cd Mon Sep 17 00:00:00 2001 From: Chiyu Zhong Date: Thu, 23 Jul 2015 18:35:46 +0800 Subject: [PATCH 1/8] Change prototype construction for proper "constructor" property closes #60 --- History.md | 5 ++ lib/cookies.js | 144 ++++++++++++++++++++++++------------------------- test/cookie.js | 5 ++ test/http.js | 2 + 4 files changed, 82 insertions(+), 74 deletions(-) diff --git a/History.md b/History.md index 28be558..803f55e 100644 --- a/History.md +++ b/History.md @@ -1,3 +1,8 @@ +0.6.x +===== + + * Change prototype construction for proper "constructor" property + 0.5.1 / 2014-07-27 ================== diff --git a/lib/cookies.js b/lib/cookies.js index 201b93c..12e4b0c 100644 --- a/lib/cookies.js +++ b/lib/cookies.js @@ -27,67 +27,65 @@ function Cookies(request, response, keys) { } } -Cookies.prototype = { - get: function(name, opts) { - var sigName = name + ".sig" - , header, match, value, remote, data, index - , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys +Cookies.prototype.get = function(name, opts) { + var sigName = name + ".sig" + , header, match, value, remote, data, index + , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys - header = this.request.headers["cookie"] - if (!header) return + header = this.request.headers["cookie"] + if (!header) return - match = header.match(getPattern(name)) - if (!match) return + match = header.match(getPattern(name)) + if (!match) return - value = match[1] - if (!opts || !signed) return value + value = match[1] + if (!opts || !signed) return value - remote = this.get(sigName) - if (!remote) return + remote = this.get(sigName) + if (!remote) return - data = name + "=" + value - if (!this.keys) throw new Error('.keys required for signed cookies'); - index = this.keys.index(data, remote) - - 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.getHeader("Set-Cookie") || [] - , secure = req.protocol === 'https' || req.connection.encrypted - , cookie = new Cookie(name, value, opts) - , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys - - if (typeof headers == "string") headers = [headers] - - if (!secure && opts && opts.secure) { - throw new Error('Cannot send secure cookie over unencrypted connection') - } - - cookie.secure = secure - if (opts && "secure" in opts) cookie.secure = opts.secure - if (opts && "secureProxy" in opts) cookie.secure = opts.secureProxy - headers = pushCookie(headers, cookie) + data = name + "=" + value + if (!this.keys) throw new Error('.keys required for signed cookies'); + index = this.keys.index(data, remote) + + if (index < 0) { + this.set(sigName, null, {path: "/", signed: false }) + } else { + index && this.set(sigName, this.keys.sign(data), { signed: false }) + return value + } +}; - if (opts && signed) { - if (!this.keys) throw new Error('.keys required for signed cookies'); - cookie.value = this.keys.sign(cookie.toString()) - cookie.name += ".sig" - headers = pushCookie(headers, cookie) - } +Cookies.prototype.set = function(name, value, opts) { + var res = this.response + , req = this.request + , headers = res.getHeader("Set-Cookie") || [] + , secure = req.protocol === 'https' || req.connection.encrypted + , cookie = new Cookie(name, value, opts) + , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys - var setHeader = res.set ? http.OutgoingMessage.prototype.setHeader : res.setHeader - setHeader.call(res, 'Set-Cookie', headers) - return this + if (typeof headers == "string") headers = [headers] + + if (!secure && opts && opts.secure) { + throw new Error('Cannot send secure cookie over unencrypted connection') } -} + + cookie.secure = secure + if (opts && "secure" in opts) cookie.secure = opts.secure + if (opts && "secureProxy" in opts) cookie.secure = opts.secureProxy + headers = pushCookie(headers, cookie) + + if (opts && signed) { + if (!this.keys) throw new Error('.keys required for signed cookies'); + cookie.value = this.keys.sign(cookie.toString()) + cookie.name += ".sig" + headers = pushCookie(headers, cookie) + } + + var setHeader = res.set ? http.OutgoingMessage.prototype.setHeader : res.setHeader + setHeader.call(res, 'Set-Cookie', headers) + return this +}; function Cookie(name, value, attrs) { if (!fieldContentRegExp.test(name)) { @@ -116,32 +114,30 @@ function Cookie(name, value, attrs) { } } -Cookie.prototype = { - path: "/", - expires: undefined, - domain: undefined, - httpOnly: true, - secure: false, - overwrite: false, +Cookie.prototype.path = "/"; +Cookie.prototype.expires = undefined; +Cookie.prototype.domain = undefined; +Cookie.prototype.httpOnly = true; +Cookie.prototype.secure = false; +Cookie.prototype.overwrite = false; - toString: function() { - return this.name + "=" + this.value - }, +Cookie.prototype.toString = function() { + return this.name + "=" + this.value +}; - toHeader: function() { - var header = this.toString() +Cookie.prototype.toHeader = function() { + var header = this.toString() - if (this.maxAge) this.expires = new Date(Date.now() + this.maxAge); + if (this.maxAge) this.expires = new Date(Date.now() + this.maxAge); - if (this.path ) header += "; path=" + this.path - if (this.expires ) header += "; expires=" + this.expires.toUTCString() - if (this.domain ) header += "; domain=" + this.domain - if (this.secure ) header += "; secure" - if (this.httpOnly ) header += "; httponly" + if (this.path ) header += "; path=" + this.path + if (this.expires ) header += "; expires=" + this.expires.toUTCString() + if (this.domain ) header += "; domain=" + this.domain + if (this.secure ) header += "; secure" + if (this.httpOnly ) header += "; httponly" - return header - } -} + return header +}; // back-compat so maxage mirrors maxAge Object.defineProperty(Cookie.prototype, 'maxage', { diff --git a/test/cookie.js b/test/cookie.js index 994d566..38c61bb 100644 --- a/test/cookie.js +++ b/test/cookie.js @@ -3,6 +3,11 @@ var assert = require('assert') var cookies = require('..') describe('new Cookie', function () { + it('should have correct constructor', function () { + var cookie = new cookies.Cookie('foo', 'bar') + assert.equal(cookie.constructor, cookies.Cookie) + }) + it('should throw on invalid name', function () { assert.throws(function () { new cookies.Cookie('foo\n', 'bar') diff --git a/test/http.js b/test/http.js index cbb2d50..4f4ed1a 100644 --- a/test/http.js +++ b/test/http.js @@ -14,6 +14,8 @@ describe('HTTP', function () { var cookies = new Cookies( req, res, keys ) , unsigned, signed, tampered, overwrite + assert.equal( cookies.constructor, Cookies ) + if ( req.url == "/set" ) { cookies // set a regular cookie From 9f09c3b410c0f127ab3ccce571b97182238ac65e Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 20:21:39 -0500 Subject: [PATCH 2/8] build: remove .npmignore --- .npmignore | 2 -- package.json | 6 ++++++ 2 files changed, 6 insertions(+), 2 deletions(-) delete mode 100644 .npmignore diff --git a/.npmignore b/.npmignore deleted file mode 100644 index ac0bfb9..0000000 --- a/.npmignore +++ /dev/null @@ -1,2 +0,0 @@ -test/ -.travis.yml diff --git a/package.json b/package.json index 284727c..70c563f 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,12 @@ "license": "MIT", "author": "Jed Schmidt (http://jed.is)", "repository": "pillarjs/cookies", + "files": [ + "lib/", + "History.md", + "LICENSE.txt", + "README.md" + ], "scripts": { "test": "mocha --reporter spec" } From d0d6abad5768d3220113b3933c9c311c2d970095 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 21:33:41 -0500 Subject: [PATCH 3/8] Change constructor to signature `new Cookies(req, res, [options])` closes #43 --- History.md | 2 ++ README.md | 8 +++++--- lib/cookies.js | 30 ++++++++++++++++++++---------- package.json | 3 ++- test/support/env.js | 2 ++ 5 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 test/support/env.js diff --git a/History.md b/History.md index 803f55e..6d133be 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,8 @@ 0.6.x ===== + * Change constructor to signature `new Cookies(req, res, [options])` + - Replace `new Cookies(req, res, key)` with `new Cookies(req, res, {'keys': keys})` * Change prototype construction for proper "constructor" property 0.5.1 / 2014-07-27 diff --git a/README.md b/README.md index c6e4c61..bd2e22b 100644 --- a/README.md +++ b/README.md @@ -26,9 +26,11 @@ Cookies is a [node.js](http://nodejs.org/) module for getting and setting HTTP(S ## API -### cookies = new Cookies( request, response, [ keys ] ) +### cookies = new Cookies( request, response, [ options ] ) -This creates a cookie jar corresponding to the current _request_ and _response_. A [Keygrip](https://www.npmjs.com/package/keygrip) object or an array of keys can optionally be passed as the third argument _keygrip_ to enable cryptographic signing based on SHA1 HMAC, using rotated credentials. +This creates a cookie jar corresponding to the current _request_ and _response_, additionally passing an object _options_. + +A [Keygrip](https://www.npmjs.com/package/keygrip) object or an array of keys can optionally be passed as _options.keys_ to enable cryptographic signing based on SHA1 HMAC, using rotated credentials. Note that since this only saves parameters without any other processing, it is very lightweight. Cookies are only parsed on demand when they are accessed. @@ -73,7 +75,7 @@ var http = require( "http" ) var Cookies = require( "cookies" ) server = http.createServer( function( req, res ) { - var cookies = new Cookies( req, res, keys ) + var cookies = new Cookies( req, res, { "keys": keys } ) , unsigned, signed, tampered if ( req.url == "/set" ) { diff --git a/lib/cookies.js b/lib/cookies.js index 12e4b0c..ec97e7a 100644 --- a/lib/cookies.js +++ b/lib/cookies.js @@ -1,3 +1,4 @@ +var deprecate = require('depd')('cookies') var Keygrip = require('keygrip') var http = require('http') var cache = {} @@ -12,18 +13,24 @@ var cache = {} var fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/; -function Cookies(request, response, keys) { - if (!(this instanceof Cookies)) return new Cookies(request, response, keys) +function Cookies(request, response, options) { + if (!(this instanceof Cookies)) return new Cookies(request, response, options) this.request = request this.response = response - if (keys) { - // array of key strings - if (Array.isArray(keys)) - this.keys = new Keygrip(keys) - // any keygrip constructor to allow different versions - else if (keys.constructor && keys.constructor.name === 'Keygrip') - this.keys = keys + + if (options) { + if (Array.isArray(options)) { + // array of key strings + deprecate('"keys" argument; provide using options {"key": [...]}') + this.keys = new Keygrip(options) + } else if (options.constructor && options.constructor.name === 'Keygrip') { + // any keygrip constructor to allow different versions + deprecate('"keys" argument; provide using options {"key": keygrip}') + this.keys = options + } else { + this.keys = options.keys + } } } @@ -167,7 +174,10 @@ function pushCookie(cookies, cookie) { Cookies.connect = Cookies.express = function(keys) { return function(req, res, next) { - req.cookies = res.cookies = new Cookies(req, res, keys) + req.cookies = res.cookies = new Cookies(req, res, { + keys: keys + }) + next() } } diff --git a/package.json b/package.json index 70c563f..502601a 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,7 @@ "description": "Cookies, optionally signed using Keygrip.", "main": "./lib/cookies", "dependencies": { + "depd": "~1.1.0", "keygrip": "~1.0.0" }, "devDependencies": { @@ -25,6 +26,6 @@ "README.md" ], "scripts": { - "test": "mocha --reporter spec" + "test": "mocha --require test/support/env --reporter spec" } } diff --git a/test/support/env.js b/test/support/env.js new file mode 100644 index 0000000..1fb2f30 --- /dev/null +++ b/test/support/env.js @@ -0,0 +1,2 @@ +process.env.NODE_ENV = 'test' +process.env.NO_DEPRECATION = 'cookies' From 51006921f0d57fba18bad0313e392c9610852e8b Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 22:03:14 -0500 Subject: [PATCH 4/8] Add "secure" constructor option for secure connection checking closes #46 closes #51 --- History.md | 1 + README.md | 2 ++ lib/cookies.js | 4 +++- test/http.js | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/History.md b/History.md index 6d133be..3106d57 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,7 @@ 0.6.x ===== + * Add `secure` constructor option for secure connection checking * Change constructor to signature `new Cookies(req, res, [options])` - Replace `new Cookies(req, res, key)` with `new Cookies(req, res, {'keys': keys})` * Change prototype construction for proper "constructor" property diff --git a/README.md b/README.md index bd2e22b..d43f380 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ This creates a cookie jar corresponding to the current _request_ and _response_, A [Keygrip](https://www.npmjs.com/package/keygrip) object or an array of keys can optionally be passed as _options.keys_ to enable cryptographic signing based on SHA1 HMAC, using rotated credentials. +A Boolean can optionally be passed as _options.secure_ to explicitally specify if the connection is secure, rather than this module examining _request_. + Note that since this only saves parameters without any other processing, it is very lightweight. Cookies are only parsed on demand when they are accessed. ### express.createServer( Cookies.express( keys ) ) diff --git a/lib/cookies.js b/lib/cookies.js index ec97e7a..15a64e7 100644 --- a/lib/cookies.js +++ b/lib/cookies.js @@ -16,6 +16,7 @@ var fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/; function Cookies(request, response, options) { if (!(this instanceof Cookies)) return new Cookies(request, response, options) + this.secure = undefined this.request = request this.response = response @@ -30,6 +31,7 @@ function Cookies(request, response, options) { this.keys = options } else { this.keys = options.keys + this.secure = options.secure } } } @@ -67,7 +69,7 @@ Cookies.prototype.set = function(name, value, opts) { var res = this.response , req = this.request , headers = res.getHeader("Set-Cookie") || [] - , secure = req.protocol === 'https' || req.connection.encrypted + , secure = this.secure !== undefined ? !!this.secure : req.protocol === 'https' || req.connection.encrypted , cookie = new Cookie(name, value, opts) , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys diff --git a/test/http.js b/test/http.js index 4f4ed1a..38fcd44 100644 --- a/test/http.js +++ b/test/http.js @@ -82,4 +82,46 @@ describe('HTTP', function () { .set('Cookie', header.join(';')) .expect(200, done) }) + + describe('with "secure" option', function () { + it('should check connection when undefined; unencrypted', function (done) { + request(createServer( "http", { "keys": keys } )) + .get('/') + .expect(500, 'Cannot send secure cookie over unencrypted connection', done) + }) + + it('should check connection when undefined; encrypted', function (done) { + request(createServer( "https", { "keys": keys } )) + .get('/') + .expect(200, done) + }) + + it('should not check connection when defined; true', function (done) { + request(createServer( "http", { "keys": keys, "secure": true } )) + .get('/') + .expect(200, done) + }) + + it('should not check connection when defined; false', function (done) { + request(createServer( "https", { "keys": keys, "secure": false } )) + .get('/') + .expect(500, 'Cannot send secure cookie over unencrypted connection', done) + }) + }) }) + +function createServer(proto, opts) { + return http.createServer(function (req, res) { + var cookies = new Cookies( req, res, opts ) + req.protocol = proto + + try { + cookies.set( "foo", "bar", { "secure": true } ) + } catch (e) { + res.statusCode = 500 + res.write(e.message) + } + + res.end() + }) +} From 8a37c56e255c9bce46c9c23d9efc9ef8a3a95b5b Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 22:19:27 -0500 Subject: [PATCH 5/8] docs: use shields.io badges --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d43f380..e456843 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ Cookies ======= -[![NPM Version](https://badge.fury.io/js/cookies.svg)](https://badge.fury.io/js/cookies) -[![Build Status](https://travis-ci.org/pillarjs/cookies.svg?branch=master)](https://travis-ci.org/pillarjs/cookies) +[![NPM Version][npm-image]][npm-url] +[![Build Status][travis-image]][travis-url] Cookies is a [node.js](http://nodejs.org/) module for getting and setting HTTP(S) cookies. Cookies can be signed to prevent tampering, using [Keygrip](https://www.npmjs.com/package/keygrip). It can be used with the built-in node.js HTTP library, or as Connect/Express middleware. @@ -123,3 +123,8 @@ Copyright Copyright (c) 2014 Jed Schmidt. See LICENSE.txt for details. Send any questions or comments [here](http://twitter.com/jedschmidt). + +[npm-image]: https://img.shields.io/npm/v/cookies.svg +[npm-url]: https://npmjs.org/package/cookies +[travis-image]: https://img.shields.io/travis/pillarjs/cookies/master.svg +[travis-url]: https://travis-ci.org/pillarjs/cookies From 6c7bf643fe3e7072b1d29f4d4aa6ccb777beb5be Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 22:20:58 -0500 Subject: [PATCH 6/8] docs: add minimum Node.js version badge --- README.md | 3 +++ package.json | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e456843..00f76cf 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,7 @@ Cookies ======= [![NPM Version][npm-image]][npm-url] +[![Node.js Version][node-version-image]][node-version-url] [![Build Status][travis-image]][travis-url] Cookies is a [node.js](http://nodejs.org/) module for getting and setting HTTP(S) cookies. Cookies can be signed to prevent tampering, using [Keygrip](https://www.npmjs.com/package/keygrip). It can be used with the built-in node.js HTTP library, or as Connect/Express middleware. @@ -126,5 +127,7 @@ Send any questions or comments [here](http://twitter.com/jedschmidt). [npm-image]: https://img.shields.io/npm/v/cookies.svg [npm-url]: https://npmjs.org/package/cookies +[node-version-image]: https://img.shields.io/node/v/cookies.svg +[node-version-url]: https://nodejs.org/en/download/ [travis-image]: https://img.shields.io/travis/pillarjs/cookies/master.svg [travis-url]: https://travis-ci.org/pillarjs/cookies diff --git a/package.json b/package.json index 502601a..b578902 100644 --- a/package.json +++ b/package.json @@ -13,9 +13,6 @@ "restify": "2.8.1", "supertest": "1.1.0" }, - "engines": { - "node": ">= 0.8.0" - }, "license": "MIT", "author": "Jed Schmidt (http://jed.is)", "repository": "pillarjs/cookies", @@ -25,6 +22,9 @@ "LICENSE.txt", "README.md" ], + "engines": { + "node": ">= 0.8" + }, "scripts": { "test": "mocha --require test/support/env --reporter spec" } From d6edb8ed5c32acff18e9e74ba7e73639627f7f77 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 22:21:47 -0500 Subject: [PATCH 7/8] docs: add npm downloads badge --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 00f76cf..3e3a247 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,7 @@ Cookies ======= [![NPM Version][npm-image]][npm-url] +[![NPM Downloads][downloads-image]][downloads-url] [![Node.js Version][node-version-image]][node-version-url] [![Build Status][travis-image]][travis-url] @@ -127,6 +128,8 @@ Send any questions or comments [here](http://twitter.com/jedschmidt). [npm-image]: https://img.shields.io/npm/v/cookies.svg [npm-url]: https://npmjs.org/package/cookies +[downloads-image]: https://img.shields.io/npm/dm/cookies.svg +[downloads-url]: https://npmjs.org/package/cookies [node-version-image]: https://img.shields.io/node/v/cookies.svg [node-version-url]: https://nodejs.org/en/download/ [travis-image]: https://img.shields.io/travis/pillarjs/cookies/master.svg From c7cf922795020bcfd284103d5c8ad792d4261699 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 28 Feb 2016 23:48:33 -0500 Subject: [PATCH 8/8] Deprecate "secureProxy" option in ".set"; use "secure" option instead --- History.md | 2 ++ README.md | 1 - lib/cookies.js | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 3106d57..2ebf063 100644 --- a/History.md +++ b/History.md @@ -5,6 +5,8 @@ * Change constructor to signature `new Cookies(req, res, [options])` - Replace `new Cookies(req, res, key)` with `new Cookies(req, res, {'keys': keys})` * Change prototype construction for proper "constructor" property + * Deprecate `secureProxy` option in `.set`; use `secure` option instead + - If `secure: true` throws even over SSL, use the `secure` constructor option 0.5.1 / 2014-07-27 ================== diff --git a/README.md b/README.md index 3e3a247..f53ea4e 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,6 @@ If the _options_ object is provided, it will be used to generate the outbound co * `path`: a string indicating the path of the cookie (`/` by default). * `domain`: a string indicating the domain of the cookie (no default). * `secure`: a boolean indicating whether the cookie is only to be sent over HTTPS (`false` by default for HTTP, `true` by default for HTTPS). -* `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://www.npmjs.com/package/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. diff --git a/lib/cookies.js b/lib/cookies.js index 15a64e7..871abf0 100644 --- a/lib/cookies.js +++ b/lib/cookies.js @@ -81,7 +81,12 @@ Cookies.prototype.set = function(name, value, opts) { cookie.secure = secure if (opts && "secure" in opts) cookie.secure = opts.secure - if (opts && "secureProxy" in opts) cookie.secure = opts.secureProxy + + if (opts && "secureProxy" in opts) { + deprecate('"secureProxy" option; use "secure" option, provide "secure" to constructor if needed') + cookie.secure = opts.secureProxy + } + headers = pushCookie(headers, cookie) if (opts && signed) {