Merge tag '3.7.0'

.gitignore

@@ -1,6 +1,5 @@
-coverage.html
+coverage/
 .DS_Store
-lib-cov
 *.seed
 *.log
 *.csv
@@ -13,7 +12,5 @@ benchmarks/graphs
 testing
 node_modules/
 testing
-.coverage_data
-cover_html
 test.js
 .idea

.npmignore

@@ -1,5 +1,6 @@
 .git*
 benchmarks/
+coverage/
 docs/
 examples/
 support/
@@ -7,6 +8,4 @@ test/
 testing.js
 .DS_Store
 .travis.yml
-coverage.html
-lib-cov
 Contributing.md

.travis.yml

@@ -6,3 +6,4 @@ matrix:
   allow_failures:
     - node_js: "0.11"
   fast_finish: true
+script: "npm run-script test-travis"

History.md

@@ -2,6 +2,14 @@ unreleased
 ==========
 
  * fix behavior of multiple `app.VERB` for the same path
+ * proper proxy trust with `app.set('trust proxy', trust)`
+   - `app.set('trust proxy', 1)` trust first hop
+   - `app.set('trust proxy', 'loopback')` trust loopback addresses
+   - `app.set('trust proxy', '10.0.0.1')` trust single IP
+   - `app.set('trust proxy', '10.0.0.1/16')` trust subnet
+   - `app.set('trust proxy', '10.0.0.1, 10.0.0.2')` trust list
+   - `app.set('trust proxy', false)` turn off
+   - `app.set('trust proxy', true)` trust everything
  * update type-is to 1.2.0
    - support suffix matching
 
@@ -101,6 +109,29 @@ unreleased
    - `app.route()` - Proxy to the app's `Router#route()` method to create a new route
    - Router & Route - public API
 
+3.7.0 / 2014-05-18
+==================
+
+ * proper proxy trust with `app.set('trust proxy', trust)`
+   - `app.set('trust proxy', 1)` trust first hop
+   - `app.set('trust proxy', 'loopback')` trust loopback addresses
+   - `app.set('trust proxy', '10.0.0.1')` trust single IP
+   - `app.set('trust proxy', '10.0.0.1/16')` trust subnet
+   - `app.set('trust proxy', '10.0.0.1, 10.0.0.2')` trust list
+   - `app.set('trust proxy', false)` turn off
+   - `app.set('trust proxy', true)` trust everything
+ * update connect to 2.16.2
+   - deprecate `res.headerSent` -- use `res.headersSent`
+   - deprecate `res.on("header")` -- use on-headers module instead
+   - fix edge-case in `res.appendHeader` that would append in wrong order
+   - json: use body-parser
+   - urlencoded: use body-parser
+   - dep: bytes@1.0.0
+   - dep: cookie-parser@1.1.0
+   - dep: csurf@1.2.0
+   - dep: express-session@1.1.0
+   - dep: method-override@1.0.1
+
 3.6.0 / 2014-05-09
 ==================
 

Makefile

@@ -1,34 +0,0 @@
-
-MOCHA_OPTS= --check-leaks
-REPORTER = dot
-
-check: test
-
-test: test-unit test-acceptance
-
-test-unit:
-	@NODE_ENV=test ./node_modules/.bin/mocha \
-		--reporter $(REPORTER) \
-		--globals setImmediate,clearImmediate \
-		$(MOCHA_OPTS)
-
-test-acceptance:
-	@NODE_ENV=test ./node_modules/.bin/mocha \
-		--reporter $(REPORTER) \
-		--bail \
-		test/acceptance/*.js
-
-test-cov: lib-cov
-	@EXPRESS_COV=1 $(MAKE) test REPORTER=html-cov > coverage.html
-
-lib-cov:
-	@jscoverage lib lib-cov
-
-bench:
-	@$(MAKE) -C benchmarks
-
-clean:
-	rm -f coverage.html
-	rm -fr lib-cov
-
-.PHONY: test test-unit test-acceptance bench clean

Readme.md

@@ -2,7 +2,7 @@
 
   Fast, unopinionated, minimalist web framework for [node](http://nodejs.org).
 
-  [![Build Status](https://travis-ci.org/visionmedia/express.svg?branch=master)](https://travis-ci.org/visionmedia/express) [![Gittip](http://img.shields.io/gittip/visionmedia.svg)](https://www.gittip.com/visionmedia/) [![NPM version](https://badge.fury.io/js/express.svg)](http://badge.fury.io/js/express)
+  [![NPM version](https://badge.fury.io/js/express.svg)](http://badge.fury.io/js/express) [![Build Status](https://travis-ci.org/visionmedia/express.svg?branch=master)](https://travis-ci.org/visionmedia/express) [![Coverage Status](https://img.shields.io/coveralls/visionmedia/express.svg)](https://coveralls.io/r/visionmedia/express) [![Gittip](http://img.shields.io/gittip/visionmedia.svg)](https://www.gittip.com/visionmedia/)
 
 ```js
 var express = require('express');
@@ -95,7 +95,9 @@ To run the test suite, first invoke the following command within the repo, insta
 
 Then run the tests:
 
-    $ make test
+```sh
+$ npm test
+```
 
 ## Contributors
   

examples/auth/app.js

@@ -78,7 +78,7 @@ function restrict(req, res, next) {
 }
 
 app.get('/', function(req, res){
-  res.redirect('login');
+  res.redirect('/login');
 });
 
 app.get('/restricted', restrict, function(req, res){
@@ -116,7 +116,7 @@ app.post('/login', function(req, res){
       req.session.error = 'Authentication failed, please check your '
         + ' username and password.'
         + ' (use "tj" and "foobar")';
-      res.redirect('login');
+      res.redirect('/login');
     }
   });
 });

index.js

@@ -1,4 +1,2 @@
 
-module.exports = process.env.EXPRESS_COV
-  ? require('./lib-cov/express')
-  : require('./lib/express');
+module.exports = require('./lib/express');

lib/application.js

@@ -11,6 +11,7 @@ var query = require('./middleware/query');
 var debug = require('debug')('express:application');
 var View = require('./view');
 var http = require('http');
+var compileTrust = require('./utils').compileTrust;
 var deprecate = require('./utils').deprecate;
 
 /**
@@ -50,6 +51,7 @@ app.defaultConfiguration = function(){
   var env = process.env.NODE_ENV || 'development';
   this.set('env', env);
   this.set('subdomain offset', 2);
+  this.set('trust proxy', false);
 
   debug('booting in %s mode', env);
 
@@ -308,6 +310,12 @@ app.set = function(setting, val){
     return this.settings[setting];
   } else {
     this.settings[setting] = val;
+
+    if (setting === 'trust proxy') {
+      debug('compile trust proxy %j', val);
+      this.set('trust proxy fn', compileTrust(val));
+    }
+
     return this;
   }
 };

lib/request.js

@@ -8,6 +8,7 @@ var http = require('http');
 var fresh = require('fresh');
 var parseRange = require('range-parser');
 var parse = require('parseurl');
+var proxyaddr = require('proxy-addr');
 
 /**
  * Request prototype.
@@ -239,19 +240,26 @@ req.is = function(types){
 /**
  * Return the protocol string "http" or "https"
  * when requested with TLS. When the "trust proxy"
- * setting is enabled the "X-Forwarded-Proto" header
- * field will be trusted. If you're running behind
- * a reverse proxy that supplies https for you this
- * may be enabled.
+ * setting trusts the socket address, the
+ * "X-Forwarded-Proto" header field will be trusted.
+ * If you're running behind a reverse proxy that
+ * supplies https for you this may be enabled.
  *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('protocol', function(){
-  var trustProxy = this.app.get('trust proxy');
-  if (this.connection.encrypted) return 'https';
-  if (!trustProxy) return 'http';
+  var trust = this.app.get('trust proxy fn');
+
+  if (!trust(this.connection.remoteAddress)) {
+    return this.connection.encrypted
+      ? 'https'
+      : 'http';
+  }
+
+  // Note: X-Forwarded-Proto is normally only ever a
+  //       single value, but this is to be safe.
   var proto = this.get('X-Forwarded-Proto') || 'http';
   return proto.split(/\s*,\s*/)[0];
 });
@@ -270,36 +278,36 @@ req.__defineGetter__('secure', function(){
 });
 
 /**
- * Return the remote address, or when
- * "trust proxy" is `true` return
- * the upstream addr.
+ * Return the remote address from the trusted proxy.
+ *
+ * The is the remote address on the socket unless
+ * "trust proxy" is set.
  *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('ip', function(){
-  return this.ips[0] || this.connection.remoteAddress;
+  var trust = this.app.get('trust proxy fn');
+  return proxyaddr(this, trust);
 });
 
 /**
- * When "trust proxy" is `true`, parse
- * the "X-Forwarded-For" ip address list.
+ * When "trust proxy" is set, trusted proxy addresses + client.
  *
  * For example if the value were "client, proxy1, proxy2"
  * you would receive the array `["client", "proxy1", "proxy2"]`
- * where "proxy2" is the furthest down-stream.
+ * where "proxy2" is the furthest down-stream and "proxy1" and
+ * "proxy2" were trusted.
  *
  * @return {Array}
  * @api public
  */
 
 req.__defineGetter__('ips', function(){
-  var trustProxy = this.app.get('trust proxy');
-  var val = this.get('X-Forwarded-For');
-  return trustProxy && val
-    ? val.split(/ *, */)
-    : [];
+  var trust = this.app.get('trust proxy fn');
+  var addrs = proxyaddr.all(this, trust);
+  return addrs.slice(1).reverse();
 });
 
 /**
@@ -339,19 +347,30 @@ req.__defineGetter__('path', function(){
 /**
  * Parse the "Host" header field hostname.
  *
+ * When the "trust proxy" setting trusts the socket
+ * address, the "X-Forwarded-Host" header field will
+ * be trusted.
+ *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('host', function(){
-  var trustProxy = this.app.get('trust proxy');
-  var host = trustProxy && this.get('X-Forwarded-Host');
-  host = host || this.get('Host');
+  var trust = this.app.get('trust proxy fn');
+  var host = this.get('X-Forwarded-Host');
+
+  if (!host || !trust(this.connection.remoteAddress)) {
+    host = this.get('Host');
+  }
+
   if (!host) return;
+
+  // IPv6 literal support
   var offset = host[0] === '['
     ? host.indexOf(']') + 1
     : 0;
   var index = host.indexOf(':', offset);
+
   return ~index
     ? host.substring(0, index)
     : host;

lib/utils.js

@@ -6,9 +6,11 @@ var mime = require('send').mime;
 var crc32 = require('buffer-crc32');
 var basename = require('path').basename;
 var deprecate = require('util').deprecate;
+var proxyaddr = require('proxy-addr');
 
 /**
- * Deprecate function, like core `util.deprecate`
+ * Deprecate function, like core `util.deprecate`,
+ * but with NODE_ENV and color support.
  *
  * @param {Function} fn
  * @param {String} msg
@@ -17,9 +19,17 @@ var deprecate = require('util').deprecate;
  */
 
 exports.deprecate = function(fn, msg){
-  return 'test' !== process.env.NODE_ENV
-    ? deprecate(fn, 'express: ' + msg)
-    : fn;
+  if (process.env.NODE_ENV === 'test') return fn;
+
+  // prepend module name
+  msg = 'express: ' + msg;
+
+  if (process.stderr.isTTY) {
+    // colorize
+    msg = '\x1b[31;1m' + msg + '\x1b[0m';
+  }
+
+  return deprecate(fn, msg);
 };
 
 /**
@@ -148,3 +158,32 @@ function acceptParams(str, index) {
 
   return ret;
 }
+
+/**
+ * Compile "proxy trust" value to function.
+ *
+ * @param  {Boolean|String|Number|Array|Function} val
+ * @return {Function}
+ * @api private
+ */
+
+exports.compileTrust = function(val) {
+  if (typeof val === 'function') return val;
+
+  if (val === true) {
+    // Support plain true/false
+    return function(){ return true };
+  }
+
+  if (typeof val === 'number') {
+    // Support trusting hop count
+    return function(a, i){ return i < val };
+  }
+
+  if (typeof val === 'string') {
+    // Support comma-separated values
+    val = val.split(/ *, */);
+  }
+
+  return proxyaddr.compile(val || []);
+}

package.json

@@ -33,11 +33,25 @@
       "email": "shtylman+expressjs@gmail.com"
     }
   ],
+  "keywords": [
+    "express",
+    "framework",
+    "sinatra",
+    "web",
+    "rest",
+    "restful",
+    "router",
+    "app",
+    "api"
+  ],
+  "repository": "git://github.com/visionmedia/express",
+  "license": "MIT",
   "dependencies": {
-    "parseurl": "1.0.1",
     "accepts": "1.0.1",
-    "type-is": "1.2.0",
+    "parseurl": "1.0.1",
+    "proxy-addr": "1.0.0",
     "range-parser": "1.0.0",
+    "type-is": "1.2.0",
     "cookie": "0.1.2",
     "buffer-crc32": "0.2.1",
     "fresh": "0.2.2",
@@ -53,7 +67,11 @@
     "debug": "0.8.1"
   },
   "devDependencies": {
+    "coveralls": "2.10.0",
+    "istanbul": "0.2.10",
     "mocha": "~1.18.2",
+    "should": "~3.3.1",
+    "supertest": "~0.12.0",
     "body-parser": "~1.1.2",
     "connect-redis": "~2.0.0",
     "ejs": "~1.0.0",
@@ -61,32 +79,19 @@
     "marked": "0.3.2",
     "multiparty": "~3.2.4",
     "hjs": "~0.0.6",
-    "should": "~3.3.1",
-    "supertest": "~0.12.0",
     "method-override": "1.0.0",
     "cookie-parser": "1.0.1",
     "express-session": "1.0.4",
     "morgan": "1.0.1",
     "vhost": "1.0.0"
   },
-  "keywords": [
-    "express",
-    "framework",
-    "sinatra",
-    "web",
-    "rest",
-    "restful",
-    "router",
-    "app",
-    "api"
-  ],
-  "repository": "git://github.com/visionmedia/express",
-  "scripts": {
-    "prepublish": "npm prune",
-    "test": "make test"
-  },
   "engines": {
     "node": ">= 0.10.0"
   },
-  "license": "MIT"
+  "scripts": {
+    "prepublish": "npm prune",
+    "test": "mocha --require test/support/env --reporter dot --check-leaks test/ test/acceptance/",
+    "test-cov": "istanbul cover node_modules/mocha/bin/_mocha -- --require test/support/env --reporter dot --check-leaks test/ test/acceptance/",
+    "test-travis": "istanbul cover node_modules/mocha/bin/_mocha --report lcovonly -- --require test/support/env --reporter spec --check-leaks test/ test/acceptance/ && cat ./coverage/lcov.info | coveralls"
+  }
 }

test/acceptance/auth.js

@@ -1,13 +1,5 @@
 var app = require('../../examples/auth/app')
-  , request = require('supertest');
-
-function redirects(to, fn){
-  return function(err, res){
-    res.statusCode.should.equal(302)
-    res.headers.should.have.property('location').match(to);
-    fn()
-  }
-}
+var request = require('supertest')
 
 function getCookie(res) {
   return res.headers['set-cookie'][0].split(';')[0];
@@ -18,25 +10,93 @@ describe('auth', function(){
     it('should redirect to /login', function(done){
       request(app)
       .get('/')
-      .end(redirects(/login$/, done))
+      .expect('Location', '/login')
+      .expect(302, done)
     })
   })
 
-  describe('GET /restricted (w/o cookie)',function(){
-    it('should redirect to /login', function(done){
+  describe('GET /login',function(){
+    it('should render login form', function(done){
       request(app)
-      .get('/restricted')
-      .end(redirects(/login$/,done))
+      .get('/login')
+      .expect(200, /<form/, done)
     })
-  })
 
-  describe('POST /login', function(){
-    it('should fail without proper credentials', function(done){
+    it('should display login error', function(done){
       request(app)
       .post('/login')
       .type('urlencoded')
       .send('username=not-tj&password=foobar')
-      .end(redirects(/login$/, done))
+      .expect('Location', '/login')
+      .expect(302, function(err, res){
+        if (err) return done(err)
+        request(app)
+        .get('/login')
+        .set('Cookie', getCookie(res))
+        .expect(200, /Authentication failed/, done)
+      })
     })
   })
-})
+
+  describe('GET /logout',function(){
+    it('should redirect to /', function(done){
+      request(app)
+      .get('/logout')
+      .expect('Location', '/')
+      .expect(302, done)
+    })
+  })
+
+  describe('GET /restricted',function(){
+    it('should redirect to /login without cookie', function(done){
+      request(app)
+      .get('/restricted')
+      .expect('Location', '/login')
+      .expect(302, done)
+    })
+
+    it('should succeed with proper cookie', function(done){
+      request(app)
+      .post('/login')
+      .type('urlencoded')
+      .send('username=tj&password=foobar')
+      .expect('Location', '/')
+      .expect(302, function(err, res){
+        if (err) return done(err)
+        request(app)
+        .get('/restricted')
+        .set('Cookie', getCookie(res))
+        .expect(200, done)
+      })
+    })
+  })
+
+  describe('POST /login', function(){
+    it('should fail without proper username', function(done){
+      request(app)
+      .post('/login')
+      .type('urlencoded')
+      .send('username=not-tj&password=foobar')
+      .expect('Location', '/login')
+      .expect(302, done)
+    })
+
+    it('should fail without proper password', function(done){
+      request(app)
+      .post('/login')
+      .type('urlencoded')
+      .send('username=tj&password=baz')
+      .expect('Location', '/login')
+      .expect(302, done)
+    })
+
+    it('should succeed with proper credentials', function(done){
+      request(app)
+      .post('/login')
+      .type('urlencoded')
+      .send('username=tj&password=foobar')
+      .expect('Location', '/')
+      .expect(302, done)
+    })
+  })
+})

test/middleware.basic.js

@@ -41,4 +41,4 @@ describe('middleware', function(){
       })
     })
   })
-})
+})

test/req.host.js

@@ -69,5 +69,70 @@ describe('req', function(){
       .set('Host', '[::1]:3000')
       .expect('[::1]', done);
     })
+
+    describe('when "trust proxy" is enabled', function(){
+      it('should respect X-Forwarded-Host', function(done){
+        var app = express();
+
+        app.enable('trust proxy');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'example.com')
+        .expect('example.com', done);
+      })
+
+      it('should ignore X-Forwarded-Host if socket addr not trusted', function(done){
+        var app = express();
+
+        app.set('trust proxy', '10.0.0.1');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'example.com')
+        .expect('localhost', done);
+      })
+
+      it('should default to Host', function(done){
+        var app = express();
+
+        app.enable('trust proxy');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'example.com')
+        .expect('example.com', done);
+      })
+    })
+
+    describe('when "trust proxy" is disabled', function(){
+      it('should ignore X-Forwarded-Host', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'evil')
+        .expect('localhost', done);
+      })
+    })
   })
 })

test/req.ip.js

@@ -20,6 +20,21 @@ describe('req', function(){
           .set('X-Forwarded-For', 'client, p1, p2')
           .expect('client', done);
         })
+
+        it('should return the addr after trusted proxy', function(done){
+          var app = express();
+
+          app.set('trust proxy', 2);
+
+          app.use(function(req, res, next){
+            res.send(req.ip);
+          });
+
+          request(app)
+          .get('/')
+          .set('X-Forwarded-For', 'client, p1, p2')
+          .expect('p1', done);
+        })
       })
 
       describe('when "trust proxy" is disabled', function(){

test/req.ips.js

@@ -20,6 +20,21 @@ describe('req', function(){
           .set('X-Forwarded-For', 'client, p1, p2')
           .expect('["client","p1","p2"]', done);
         })
+
+        it('should stop at first untrusted', function(done){
+          var app = express();
+
+          app.set('trust proxy', 2);
+
+          app.use(function(req, res, next){
+            res.send(req.ips);
+          });
+
+          request(app)
+          .get('/')
+          .set('X-Forwarded-For', 'client, p1, p2')
+          .expect('["p1","p2"]', done);
+        })
       })
 
       describe('when "trust proxy" is disabled', function(){

test/req.protocol.js

@@ -32,6 +32,21 @@ describe('req', function(){
         .expect('https', done);
       })
 
+      it('should ignore X-Forwarded-Proto if socket addr not trusted', function(done){
+        var app = express();
+
+        app.set('trust proxy', '10.0.0.1');
+
+        app.use(function(req, res){
+          res.end(req.protocol);
+        });
+
+        request(app)
+        .get('/')
+        .set('X-Forwarded-Proto', 'https')
+        .expect('http', done);
+      })
+
       it('should default to http', function(done){
         var app = express();
 

test/support/env.js

@@ -0,0 +1,2 @@
+
+process.env.NODE_ENV = 'test';