expressjs/cookie-session — pull request #20 by weilu (2014-08-10 06:48:42+00:00)
Title: Upgrade cookies

This PR addresses a few problems:

1. secureProxy doesn't work with the following configuration: `{ signed: false, secureProxy: true, secure: true }`. The server will crash with `Uncaught Error: Cannot send secure cookie over unencrypted connection`. While I was digging into the cookies module I realized that the `secureProxy` implementation is buggy. While I was working on a patch I found this test case: https://github.com/expressjs/cookies/blob/master/test/express.js#L123-L139. It makes me think that the `secureProxy` option is unnecessary, at least in the case of an express middleware.

2. After switching to using `app.set('trust proxy', true)` with config `signed: false, secure: true`, it was still not working for me, because cookie-session relies on cookies 0.4.x where detection of whether a connection is secure purely depends on `req.connection.encrypted` https://github.com/expressjs/cookies/blob/0.4.1/lib/cookies.js#L54. That behavior has been changed in cookies 0.5.0 to first check `req.protocol` which takes the x-forwarded-proto into account. Therefore the upgrade of the dependency cookies.

3. Also updated the readme to prefer `app.set('trust proxy', true)` over `secureProxy` for a proxy configuration

4. I've added a positive test case for the proxy scenario, to guard against regression. I'd add negative cases as well, but currently due to implementation in the cookies' module, an error is thrown during `res.writeHead` which crashes the server. I don't think it's the expected behavior. Will open an issue with cookies.