diff --git a/.travis.yml b/.travis.yml index 73568f23..c6be9817 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,7 @@ language: node_js node_js: - "0.10" - - "0.11" -matrix: - allow_failures: - - node_js: "0.11" - fast_finish: true + - "0.12" +sudo: false script: "npm run-script test-travis" after_script: "npm install coveralls@2.10.0 && cat ./coverage/lcov.info | coveralls" diff --git a/History.md b/History.md index 6184768c..88cf92df 100644 --- a/History.md +++ b/History.md @@ -1,3 +1,16 @@ +unreleased +========== + + * Fix `"trust proxy"` setting to inherit when app is mounted + * Generate `ETag`s for all request responses + - No longer restricted to only responses for `GET` and `HEAD` requests + * Use `content-type` to parse `Content-Type` headers + * deps: cookie-signature@1.0.6 + * deps: send@0.12.1 + - Always read the stat size from the file + - Fix mutating passed-in `options` + - deps: mime@1.3.4 + 4.11.2 / 2015-02-01 =================== @@ -678,6 +691,34 @@ - `app.route()` - Proxy to the app's `Router#route()` method to create a new route - Router & Route - public API +3.20.0 / 2015-02-18 +=================== + + * Fix `"trust proxy"` setting to inherit when app is mounted + * Generate `ETag`s for all request responses + - No longer restricted to only responses for `GET` and `HEAD` requests + * Use `content-type` to parse `Content-Type` headers + * deps: connect@2.29.0 + - Use `content-type` to parse `Content-Type` headers + - deps: body-parser@~1.12.0 + - deps: compression@~1.4.1 + - deps: connect-timeout@~1.6.0 + - deps: cookie-parser@~1.3.4 + - deps: cookie-signature@1.0.6 + - deps: csurf@~1.7.0 + - deps: errorhandler@~1.3.4 + - deps: express-session@~1.10.3 + - deps: http-errors@~1.3.1 + - deps: response-time@~2.3.0 + - deps: serve-index@~1.6.2 + - deps: serve-static@~1.9.1 + - deps: type-is@~1.6.0 + * deps: cookie-signature@1.0.6 + * deps: send@0.12.1 + - Always read the stat size from the file + - Fix mutating passed-in `options` + - deps: mime@1.3.4 + 3.19.2 / 2015-02-01 =================== diff --git a/LICENSE b/LICENSE index 0f3c7678..aa927e44 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,8 @@ (The MIT License) Copyright (c) 2009-2014 TJ Holowaychuk +Copyright (c) 2013-2014 Roman Shtylman +Copyright (c) 2014-2015 Douglas Christopher Wilson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/lib/application.js b/lib/application.js index dc18fe37..5ec44248 100644 --- a/lib/application.js +++ b/lib/application.js @@ -1,5 +1,14 @@ +/*! + * express + * Copyright(c) 2009-2013 TJ Holowaychuk + * Copyright(c) 2013 Roman Shtylman + * Copyright(c) 2014-2015 Douglas Christopher Wilson + * MIT Licensed + */ + /** * Module dependencies. + * @api private */ var finalhandler = require('finalhandler'); @@ -25,6 +34,13 @@ var slice = Array.prototype.slice; var app = exports = module.exports = {}; +/** + * Variable for trust proxy inheritance back-compat + * @api private + */ + +var trustProxyDefaultSymbol = '@@symbol:trust_proxy_default'; + /** * Initialize the server. * @@ -58,10 +74,23 @@ app.defaultConfiguration = function(){ this.set('subdomain offset', 2); this.set('trust proxy', false); + // trust proxy inherit back-compat + Object.defineProperty(this.settings, trustProxyDefaultSymbol, { + configurable: true, + value: true + }); + debug('booting in %s mode', env); - // inherit protos - this.on('mount', function(parent){ + this.on('mount', function onmount(parent) { + // inherit trust proxy + if (this.settings[trustProxyDefaultSymbol] === true + && typeof parent.settings['trust proxy fn'] === 'function') { + delete this.settings['trust proxy']; + delete this.settings['trust proxy fn']; + } + + // inherit protos this.request.__proto__ = parent.request; this.response.__proto__ = parent.response; this.engines.__proto__ = parent.engines; @@ -327,6 +356,13 @@ app.set = function(setting, val){ case 'trust proxy': debug('compile trust proxy %s', val); this.set('trust proxy fn', compileTrust(val)); + + // trust proxy inherit back-compat + Object.defineProperty(this.settings, trustProxyDefaultSymbol, { + configurable: true, + value: false + }); + break; } diff --git a/lib/response.js b/lib/response.js index 14aa9cab..d3759713 100644 --- a/lib/response.js +++ b/lib/response.js @@ -1,5 +1,13 @@ +/*! + * express + * Copyright(c) 2009-2013 TJ Holowaychuk + * Copyright(c) 2014-2015 Douglas Christopher Wilson + * MIT Licensed + */ + /** * Module dependencies. + * @api private */ var contentDisposition = require('content-disposition'); @@ -159,15 +167,12 @@ res.send = function send(body) { this.set('Content-Length', len); } - // method check - var isHead = req.method === 'HEAD'; - - // ETag support - if (len !== undefined && (isHead || req.method === 'GET')) { - var etag = app.get('etag fn'); - if (etag && !this.get('ETag')) { - etag = etag(chunk, encoding); - etag && this.set('ETag', etag); + // populate ETag + var etag; + var generateETag = len !== undefined && app.get('etag fn'); + if (typeof generateETag === 'function' && !this.get('ETag')) { + if ((etag = generateETag(chunk, encoding))) { + this.set('ETag', etag); } } @@ -182,7 +187,7 @@ res.send = function send(body) { chunk = ''; } - if (isHead) { + if (req.method === 'HEAD') { // skip body for HEAD this.end(); } else { diff --git a/lib/utils.js b/lib/utils.js index 9c004cc7..ce53ad8b 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,8 +1,17 @@ +/*! + * express + * Copyright(c) 2009-2013 TJ Holowaychuk + * Copyright(c) 2014-2015 Douglas Christopher Wilson + * MIT Licensed + */ + /** * Module dependencies. + * @api private */ var contentDisposition = require('content-disposition'); +var contentType = require('content-type'); var deprecate = require('depd')('express'); var mime = require('send').mime; var basename = require('path').basename; @@ -10,7 +19,6 @@ var etag = require('etag'); var proxyaddr = require('proxy-addr'); var qs = require('qs'); var querystring = require('querystring'); -var typer = require('media-typer'); /** * Return strong ETag for `body`. @@ -258,17 +266,19 @@ exports.compileTrust = function(val) { * @api private */ -exports.setCharset = function(type, charset){ - if (!type || !charset) return type; +exports.setCharset = function setCharset(type, charset) { + if (!type || !charset) { + return type; + } // parse type - var parsed = typer.parse(type); + var parsed = contentType.parse(type); // set charset parsed.parameters.charset = charset; // format type - return typer.format(parsed); + return contentType.format(parsed); }; /** diff --git a/package.json b/package.json index 7179ee7b..6ced2f3c 100644 --- a/package.json +++ b/package.json @@ -29,14 +29,14 @@ "dependencies": { "accepts": "~1.2.3", "content-disposition": "0.5.0", - "cookie-signature": "1.0.5", + "content-type": "~1.0.1", + "cookie-signature": "1.0.6", "debug": "~2.1.1", "depd": "~1.0.0", "escape-html": "1.0.1", "etag": "~1.5.1", "finalhandler": "0.3.3", "fresh": "0.2.4", - "media-typer": "0.3.0", "methods": "~1.1.1", "on-finished": "~2.2.0", "parseurl": "~1.3.0", @@ -44,7 +44,7 @@ "proxy-addr": "~1.0.6", "qs": "2.3.3", "range-parser": "~1.0.2", - "send": "0.11.1", + "send": "0.12.1", "serve-static": "~1.8.1", "type-is": "~1.5.6", "vary": "~1.0.0", @@ -58,7 +58,7 @@ "istanbul": "0.3.5", "marked": "0.3.3", "mocha": "~2.1.0", - "should": "~4.6.2", + "should": "~5.0.0", "supertest": "~0.15.0", "hjs": "~0.0.6", "body-parser": "~1.11.0", diff --git a/test/config.js b/test/config.js index 59d38f07..e298e76a 100644 --- a/test/config.js +++ b/test/config.js @@ -1,30 +1,36 @@ -var express = require('../') - , assert = require('assert'); +var assert = require('assert'); +var express = require('..'); -describe('config', function(){ - describe('.set()', function(){ - it('should set a value', function(){ +describe('config', function () { + describe('.set()', function () { + it('should set a value', function () { var app = express(); - app.set('foo', 'bar').should.equal(app); + app.set('foo', 'bar'); + assert.equal(app.get('foo'), 'bar'); }) - it('should return the app when undefined', function(){ + it('should return the app', function () { var app = express(); - app.set('foo', undefined).should.equal(app); + assert.equal(app.set('foo', 'bar'), app); + }) + + it('should return the app when undefined', function () { + var app = express(); + assert.equal(app.set('foo', undefined), app); }) describe('"etag"', function(){ it('should throw on bad value', function(){ - var app = express() - app.set.bind(app, 'etag', 42).should.throw(/unknown value/) + var app = express(); + assert.throws(app.set.bind(app, 'etag', 42), /unknown value/); }) it('should set "etag fn"', function(){ var app = express() var fn = function(){} app.set('etag', fn) - app.get('etag fn').should.equal(fn) + assert.equal(app.get('etag fn'), fn) }) }) @@ -33,7 +39,7 @@ describe('config', function(){ var app = express() var fn = function(){} app.set('trust proxy', fn) - app.get('trust proxy fn').should.equal(fn) + assert.equal(app.get('trust proxy fn'), fn) }) }) }) @@ -41,34 +47,73 @@ describe('config', function(){ describe('.get()', function(){ it('should return undefined when unset', function(){ var app = express(); - assert(undefined === app.get('foo')); + assert.strictEqual(app.get('foo'), undefined); }) it('should otherwise return the value', function(){ var app = express(); app.set('foo', 'bar'); - app.get('foo').should.equal('bar'); + assert.equal(app.get('foo'), 'bar'); }) describe('when mounted', function(){ it('should default to the parent app', function(){ - var app = express() - , blog = express(); + var app = express(); + var blog = express(); app.set('title', 'Express'); app.use(blog); - blog.get('title').should.equal('Express'); + assert.equal(blog.get('title'), 'Express'); }) - + it('should given precedence to the child', function(){ - var app = express() - , blog = express(); + var app = express(); + var blog = express(); app.use(blog); app.set('title', 'Express'); blog.set('title', 'Some Blog'); - blog.get('title').should.equal('Some Blog'); + assert.equal(blog.get('title'), 'Some Blog'); + }) + + it('should inherit "trust proxy" setting', function () { + var app = express(); + var blog = express(); + + function fn() { return false } + + app.set('trust proxy', fn); + assert.equal(app.get('trust proxy'), fn); + assert.equal(app.get('trust proxy fn'), fn); + + app.use(blog); + + assert.equal(blog.get('trust proxy'), fn); + assert.equal(blog.get('trust proxy fn'), fn); + }) + + it('should prefer child "trust proxy" setting', function () { + var app = express(); + var blog = express(); + + function fn1() { return false } + function fn2() { return true } + + app.set('trust proxy', fn1); + assert.equal(app.get('trust proxy'), fn1); + assert.equal(app.get('trust proxy fn'), fn1); + + blog.set('trust proxy', fn2); + assert.equal(blog.get('trust proxy'), fn2); + assert.equal(blog.get('trust proxy fn'), fn2); + + app.use(blog); + + assert.equal(app.get('trust proxy'), fn1); + assert.equal(app.get('trust proxy fn'), fn1); + assert.equal(blog.get('trust proxy'), fn2); + assert.equal(blog.get('trust proxy fn'), fn2); }) }) }) @@ -76,42 +121,42 @@ describe('config', function(){ describe('.enable()', function(){ it('should set the value to true', function(){ var app = express(); - app.enable('tobi').should.equal(app); - app.get('tobi').should.be.true; + assert.equal(app.enable('tobi'), app); + assert.strictEqual(app.get('tobi'), true); }) }) describe('.disable()', function(){ it('should set the value to false', function(){ var app = express(); - app.disable('tobi').should.equal(app); - app.get('tobi').should.be.false; + assert.equal(app.disable('tobi'), app); + assert.strictEqual(app.get('tobi'), false); }) }) describe('.enabled()', function(){ it('should default to false', function(){ var app = express(); - app.enabled('foo').should.be.false; + assert.strictEqual(app.enabled('foo'), false); }) it('should return true when set', function(){ var app = express(); app.set('foo', 'bar'); - app.enabled('foo').should.be.true; + assert.strictEqual(app.enabled('foo'), true); }) }) describe('.disabled()', function(){ it('should default to true', function(){ var app = express(); - app.disabled('foo').should.be.true; + assert.strictEqual(app.disabled('foo'), true); }) it('should return false when set', function(){ var app = express(); app.set('foo', 'bar'); - app.disabled('foo').should.be.false; + assert.strictEqual(app.disabled('foo'), false); }) }) }) diff --git a/test/req.ip.js b/test/req.ip.js index 19d0fee1..3ca575e1 100644 --- a/test/req.ip.js +++ b/test/req.ip.js @@ -35,6 +35,23 @@ describe('req', function(){ .set('X-Forwarded-For', 'client, p1, p2') .expect('p1', done); }) + + it('should return the addr after trusted proxy, from sub app', function (done) { + var app = express(); + var sub = express(); + + app.set('trust proxy', 2); + app.use(sub); + + sub.use(function (req, res, next) { + res.send(req.ip); + }); + + request(app) + .get('/') + .set('X-Forwarded-For', 'client, p1, p2') + .expect(200, 'p1', done); + }) }) describe('when "trust proxy" is disabled', function(){ diff --git a/test/res.send.js b/test/res.send.js index 6148e002..6c1361b5 100644 --- a/test/res.send.js +++ b/test/res.send.js @@ -1,7 +1,8 @@ -var express = require('../') - , request = require('supertest') - , assert = require('assert'); +var assert = require('assert'); +var express = require('..'); +var methods = require('methods'); +var request = require('supertest'); describe('res', function(){ describe('.send(null)', function(){ @@ -114,10 +115,10 @@ describe('res', function(){ }) }) - it('should set ETag', function(done){ + it('should set ETag', function (done) { var app = express(); - app.use(function(req, res){ + app.use(function (req, res) { var str = Array(1000).join('-'); res.send(str); }); @@ -125,24 +126,7 @@ describe('res', function(){ request(app) .get('/') .expect('ETag', 'W/"3e7-8084ccd1"') - .end(done); - }) - - it('should not set ETag for non-GET/HEAD', function(done){ - var app = express(); - - app.use(function(req, res){ - var str = Array(1000).join('-'); - res.send(str); - }); - - request(app) - .post('/') - .end(function(err, res){ - if (err) return done(err); - assert(!res.header.etag, 'has an ETag'); - done(); - }); + .expect(200, done); }) it('should not override Content-Type', function(done){ @@ -203,10 +187,10 @@ describe('res', function(){ }) }) - it('should set ETag', function(done){ + it('should set ETag', function (done) { var app = express(); - app.use(function(req, res){ + app.use(function (req, res) { var str = Array(1000).join('-'); res.send(new Buffer(str)); }); @@ -214,7 +198,7 @@ describe('res', function(){ request(app) .get('/') .expect('ETag', 'W/"3e7-8084ccd1"') - .end(done); + .expect(200, done); }) it('should not override Content-Type', function(done){ @@ -364,12 +348,12 @@ describe('res', function(){ .expect('{"foo":"bar"}', done); }) - describe('"etag" setting', function(){ - describe('when enabled', function(){ - it('should send ETag', function(done){ + describe('"etag" setting', function () { + describe('when enabled', function () { + it('should send ETag', function (done) { var app = express(); - app.use(function(req, res){ + app.use(function (req, res) { res.send('kajdslfkasdf'); }); @@ -377,76 +361,95 @@ describe('res', function(){ request(app) .get('/') - .expect('etag', 'W/"c-5aee35d8"', done) - }) - - it('should send ETag for empty string response', function(done){ - var app = express() - - app.use(function(req, res){ - res.send('') - }); - - app.enable('etag') - - request(app) - .get('/') - .expect('etag', 'W/"0-0"', done) - }) - - it('should send ETag for long response', function(done){ - var app = express(); - - app.use(function(req, res){ - var str = Array(1000).join('-'); - res.send(str); - }); - - app.enable('etag'); - - request(app) - .get('/') - .expect('etag', 'W/"3e7-8084ccd1"', done) + .expect('ETag', 'W/"c-5aee35d8"') + .expect(200, done); }); - it('should not override ETag when manually set', function(done){ - var app = express(); + methods.forEach(function (method) { + if (method === 'connect') return; - app.use(function(req, res){ - res.set('etag', '"asdf"'); - res.send(200); - }); + it('should send ETag in response to ' + method.toUpperCase() + ' request', function (done) { + var app = express(); - app.enable('etag'); + app[method]('/', function (req, res) { + res.send('kajdslfkasdf'); + }); - request(app) - .get('/') - .expect('etag', '"asdf"', done) - }); - - it('should not send ETag for res.send()', function(done){ - var app = express() - - app.use(function(req, res){ - res.send() - }); - - app.enable('etag') - - request(app) - .get('/') - .end(function(err, res){ - res.headers.should.not.have.property('etag'); - done(); + request(app) + [method]('/') + .expect('ETag', 'W/"c-5aee35d8"') + .expect(200, done); }) + }); + + it('should send ETag for empty string response', function (done) { + var app = express(); + + app.use(function (req, res) { + res.send(''); + }); + + app.enable('etag'); + + request(app) + .get('/') + .expect('ETag', 'W/"0-0"') + .expect(200, done); + }) + + it('should send ETag for long response', function (done) { + var app = express(); + + app.use(function (req, res) { + var str = Array(1000).join('-'); + res.send(str); + }); + + app.enable('etag'); + + request(app) + .get('/') + .expect('ETag', 'W/"3e7-8084ccd1"') + .expect(200, done); + }); + + it('should not override ETag when manually set', function (done) { + var app = express(); + + app.use(function (req, res) { + res.set('etag', '"asdf"'); + res.send(200); + }); + + app.enable('etag'); + + request(app) + .get('/') + .expect('ETag', '"asdf"') + .expect(200, done); + }); + + it('should not send ETag for res.send()', function (done) { + var app = express(); + + app.use(function (req, res) { + res.send(); + }); + + app.enable('etag'); + + request(app) + .get('/') + .expect(shouldNotHaveHeader('ETag')) + .expect(200, done); }) }); - describe('when disabled', function(){ - it('should send no ETag', function(done){ + describe('when disabled', function () { + it('should send no ETag', function (done) { var app = express(); - app.use(function(req, res){ + app.use(function (req, res) { var str = Array(1000).join('-'); res.send(str); }); @@ -455,99 +458,105 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.not.have.property('etag'); - done(); - }); + .expect(shouldNotHaveHeader('ETag')) + .expect(200, done); }); - it('should send ETag when manually set', function(done){ + it('should send ETag when manually set', function (done) { var app = express(); app.disable('etag'); - app.use(function(req, res){ + app.use(function (req, res) { res.set('etag', '"asdf"'); res.send(200); }); request(app) .get('/') - .expect('etag', '"asdf"', done) + .expect('ETag', '"asdf"') + .expect(200, done); }); }); - describe('when "strong"', function(){ - it('should send strong ETag', function(done){ - var app = express() + describe('when "strong"', function () { + it('should send strong ETag', function (done) { + var app = express(); app.set('etag', 'strong'); - app.use(function(req, res){ + app.use(function (req, res) { res.send('hello, world!'); }); request(app) .get('/') - .expect('etag', '"Otu60XkfuuPskIiUxJY4cA=="', done) + .expect('ETag', '"Otu60XkfuuPskIiUxJY4cA=="') + .expect(200, done); }) }) - describe('when "weak"', function(){ - it('should send weak ETag', function(done){ - var app = express() + describe('when "weak"', function () { + it('should send weak ETag', function (done) { + var app = express(); app.set('etag', 'weak'); - app.use(function(req, res){ + app.use(function (req, res) { res.send('hello, world!'); }); request(app) .get('/') - .expect('etag', 'W/"d-58988d13"', done) + .expect('ETag', 'W/"d-58988d13"') + .expect(200, done) }) }) - describe('when a function', function(){ - it('should send custom ETag', function(done){ - var app = express() + describe('when a function', function () { + it('should send custom ETag', function (done) { + var app = express(); - app.set('etag', function(body, encoding){ + app.set('etag', function (body, encoding) { var chunk = !Buffer.isBuffer(body) ? new Buffer(body, encoding) : body; - chunk.toString().should.equal('hello, world!') - return '"custom"' + chunk.toString().should.equal('hello, world!'); + return '"custom"'; }); - app.use(function(req, res){ + app.use(function (req, res) { res.send('hello, world!'); }); request(app) .get('/') - .expect('etag', '"custom"', done) + .expect('ETag', '"custom"') + .expect(200, done); }) - it('should not send falsy ETag', function(done){ - var app = express() + it('should not send falsy ETag', function (done) { + var app = express(); - app.set('etag', function(body, encoding){ - return undefined + app.set('etag', function (body, encoding) { + return undefined; }); - app.use(function(req, res){ + app.use(function (req, res) { res.send('hello, world!'); }); request(app) .get('/') - .end(function(err, res){ - res.headers.should.not.have.property('etag') - done(); - }) + .expect(shouldNotHaveHeader('ETag')) + .expect(200, done); }) }) }) }) + +function shouldNotHaveHeader(header) { + return function (res) { + assert.ok(!(header.toLowerCase() in res.headers), 'should not have header ' + header) + } +}