From 359f12791a5cb87fab6854d76135430676fba291 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 18 May 2014 23:02:25 -0400 Subject: [PATCH 01/16] build: prevent failure from coveralls --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9d0f9940..4a85ea0f 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,6 @@ "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-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 || true)" } } From 42b982e13c031beb0ba1fdeab9c2160758e55f47 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 20:22:55 -0400 Subject: [PATCH 02/16] build: remove coveralls from devDependencies --- .travis.yml | 1 + package.json | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index bb47c1b0..1ff243c5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,3 +8,4 @@ matrix: - node_js: "0.11" fast_finish: true script: "npm run-script test-travis" +after_script: "npm install coveralls@2.10.0 && cat ./coverage/lcov.info | coveralls" diff --git a/package.json b/package.json index 4a85ea0f..e754dbcc 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,6 @@ "debug": ">= 0.8.0 < 1" }, "devDependencies": { - "coveralls": "2.10.0", "ejs": "~0.8.4", "istanbul": "0.2.10", "mocha": "~1.18.2", @@ -85,6 +84,6 @@ "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 || true)" + "test-travis": "istanbul cover node_modules/mocha/bin/_mocha --report lcovonly -- --require test/support/env --reporter spec --check-leaks test/ test/acceptance/" } } From ed69b688923c8649ef53090a0f94e98d1e133ce3 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 21:20:56 -0400 Subject: [PATCH 03/16] update connect to 2.17.0 --- History.md | 8 ++++++++ package.json | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/History.md b/History.md index 29a307a0..0923cf07 100644 --- a/History.md +++ b/History.md @@ -1,3 +1,11 @@ +3.x +=== + + * update connect to 2.17.0 + - deps: express-session@1.2.0 + - deps: morgan@1.1.1 + - deps: serve-index@1.0.3 + 3.7.0 / 2014-05-18 ================== diff --git a/package.json b/package.json index e754dbcc..365fb798 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "repository": "git://github.com/visionmedia/express", "license": "MIT", "dependencies": { - "connect": "2.16.2", + "connect": "2.17.0", "commander": "1.3.2", "methods": "1.0.0", "mkdirp": "0.5.0", From dcecdc9be630e86c504efe75996c6759a346e516 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 21:22:02 -0400 Subject: [PATCH 04/16] update mocha to 1.19.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 365fb798..f5e3e981 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "devDependencies": { "ejs": "~0.8.4", "istanbul": "0.2.10", - "mocha": "~1.18.2", + "mocha": "~1.19.0", "should": "~3.3.1", "jade": "~0.30.0", "hjs": "~0.0.6", From 392ef1eb064ecd3ceb158a263ce0f187a9bbb092 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 22:57:00 -0400 Subject: [PATCH 05/16] tests: add more app.render tests --- test/app.render.js | 115 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/test/app.render.js b/test/app.render.js index a9586c77..c6d7a5e3 100644 --- a/test/app.render.js +++ b/test/app.render.js @@ -54,6 +54,27 @@ describe('app', function(){ }) }) + it('should handle render error throws', function(done){ + var app = express(); + + function View(name, options){ + this.name = name; + this.path = 'fale'; + } + + View.prototype.render = function(options, fn){ + throw new Error('err!'); + }; + + app.set('view', View); + + app.render('something', function(err, str){ + err.should.be.ok; + err.message.should.equal('err!'); + done(); + }) + }) + describe('when the file does not exist', function(){ it('should provide a helpful error', function(done){ var app = express(); @@ -132,6 +153,68 @@ describe('app', function(){ }) }) }) + + describe('caching', function(){ + it('should always lookup view without cache', function(done){ + var app = express(); + var count = 0; + + function View(name, options){ + this.name = name; + this.path = 'fake'; + count++; + } + + View.prototype.render = function(options, fn){ + fn(null, 'abstract engine'); + }; + + app.set('view cache', false); + app.set('view', View); + + app.render('something', function(err, str){ + if (err) return done(err); + count.should.equal(1); + str.should.equal('abstract engine'); + app.render('something', function(err, str){ + if (err) return done(err); + count.should.equal(2); + str.should.equal('abstract engine'); + done(); + }) + }) + }) + + it('should cache with "view cache" setting', function(done){ + var app = express(); + var count = 0; + + function View(name, options){ + this.name = name; + this.path = 'fake'; + count++; + } + + View.prototype.render = function(options, fn){ + fn(null, 'abstract engine'); + }; + + app.set('view cache', true); + app.set('view', View); + + app.render('something', function(err, str){ + if (err) return done(err); + count.should.equal(1); + str.should.equal('abstract engine'); + app.render('something', function(err, str){ + if (err) return done(err); + count.should.equal(1); + str.should.equal('abstract engine'); + done(); + }) + }) + }) + }) }) describe('.render(name, options, fn)', function(){ @@ -175,5 +258,37 @@ describe('app', function(){ done(); }) }) + + describe('caching', function(){ + it('should cache with cache option', function(done){ + var app = express(); + var count = 0; + + function View(name, options){ + this.name = name; + this.path = 'fake'; + count++; + } + + View.prototype.render = function(options, fn){ + fn(null, 'abstract engine'); + }; + + app.set('view cache', false); + app.set('view', View); + + app.render('something', {cache: true}, function(err, str){ + if (err) return done(err); + count.should.equal(1); + str.should.equal('abstract engine'); + app.render('something', {cache: true}, function(err, str){ + if (err) return done(err); + count.should.equal(1); + str.should.equal('abstract engine'); + done(); + }) + }) + }) + }) }) }) From ff412b927d539234fc1a037c26fc05ca8892084f Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 23:07:53 -0400 Subject: [PATCH 06/16] tests: add req.acceptsEncoding tests --- test/req.acceptsEncoding.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 test/req.acceptsEncoding.js diff --git a/test/req.acceptsEncoding.js b/test/req.acceptsEncoding.js new file mode 100644 index 00000000..54da10ea --- /dev/null +++ b/test/req.acceptsEncoding.js @@ -0,0 +1,36 @@ + +var express = require('../') + , request = require('supertest'); + +describe('req', function(){ + describe('.acceptsEncodings', function(){ + it('should be true if encoding accpeted', function(done){ + var app = express(); + + app.use(function(req, res){ + req.acceptsEncoding('gzip').should.be.true; + req.acceptsEncoding('deflate').should.be.true; + res.end(); + }); + + request(app) + .get('/') + .set('Accept-Encoding', ' gzip, deflate') + .expect(200, done); + }) + + it('should be false if encoding not accpeted', function(done){ + var app = express(); + + app.use(function(req, res){ + req.acceptsEncoding('bogus').should.be.false; + res.end(); + }); + + request(app) + .get('/') + .set('Accept-Encoding', ' gzip, deflate') + .expect(200, done); + }) + }) +}) From e660f195078b914846692a5140233c7bfb184429 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 23:13:29 -0400 Subject: [PATCH 07/16] tests: add req.acceptsLanguage tests --- test/req.acceptsLanguage.js | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 test/req.acceptsLanguage.js diff --git a/test/req.acceptsLanguage.js b/test/req.acceptsLanguage.js new file mode 100644 index 00000000..a095e461 --- /dev/null +++ b/test/req.acceptsLanguage.js @@ -0,0 +1,53 @@ + +var express = require('../') + , request = require('supertest'); + +describe('req', function(){ + describe('.acceptsLanguage', function(){ + it('should be true if language accpeted', function(done){ + var app = express(); + + app.use(function(req, res){ + req.acceptsLanguage('en-us').should.be.true; + req.acceptsLanguage('en').should.be.true; + res.end(); + }); + + request(app) + .get('/') + .set('Accept-Language', 'en;q=.5, en-us') + .expect(200, done); + }) + + it('should be false if language not accpeted', function(done){ + var app = express(); + + app.use(function(req, res){ + req.acceptsLanguage('es').should.be.false; + res.end(); + }); + + request(app) + .get('/') + .set('Accept-Language', 'en;q=.5, en-us') + .expect(200, done); + }) + + describe('when Accept-Language is not present', function(){ + it('should always return true', function(done){ + var app = express(); + + app.use(function(req, res){ + req.acceptsLanguage('en').should.be.true; + req.acceptsLanguage('es').should.be.true; + req.acceptsLanguage('jp').should.be.true; + res.end(); + }); + + request(app) + .get('/') + .expect(200, done); + }) + }) + }) +}) From 83b8b7acb7d90730654f6ee4d707fd9da47826ba Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 23:25:51 -0400 Subject: [PATCH 08/16] tests: add more various tests --- test/app.js | 11 ++++++++++- test/app.param.js | 43 +++++++++++++++++++++++++++++++++++++++++++ test/req.range.js | 9 ++++++++- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/test/app.js b/test/app.js index f8ea0f01..cae72331 100644 --- a/test/app.js +++ b/test/app.js @@ -71,4 +71,13 @@ describe('in production', function(){ app.enabled('view cache').should.be.true; process.env.NODE_ENV = 'test'; }) -}) \ No newline at end of file +}) + +describe('without NODE_ENV', function(){ + it('should default to development', function(){ + process.env.NODE_ENV = ''; + var app = express(); + app.get('env').should.equal('development'); + process.env.NODE_ENV = 'test'; + }) +}) diff --git a/test/app.param.js b/test/app.param.js index 6f0ee8e7..ab296865 100644 --- a/test/app.param.js +++ b/test/app.param.js @@ -37,6 +37,11 @@ describe('app', function(){ }); }) + + it('should fail if not given fn', function(){ + var app = express(); + app.param.bind(app, ':name', 'bob').should.throw(); + }) }) describe('.param(names, fn)', function(){ @@ -95,5 +100,43 @@ describe('app', function(){ .get('/user/123') .expect('123', done); }) + + it('should catch thrown error', function(done){ + var app = express(); + + app.param('id', function(req, res, next, id){ + throw new Error('err!'); + }); + + app.get('/user/:id', function(req, res){ + var id = req.params.id; + res.send('' + id); + }); + + request(app) + .get('/user/123') + .expect(500, done); + }) + + it('should defer to next route', function(done){ + var app = express(); + + app.param('id', function(req, res, next, id){ + next('route'); + }); + + app.get('/user/:id', function(req, res){ + var id = req.params.id; + res.send('' + id); + }); + + app.get('/:name/123', function(req, res){ + res.send('name'); + }); + + request(app) + .get('/user/123') + .expect('name', done); + }) }) }) diff --git a/test/req.range.js b/test/req.range.js index a71a4670..08cf8f91 100644 --- a/test/req.range.js +++ b/test/req.range.js @@ -1,5 +1,6 @@ -var express = require('../'); +var assert = require('assert'); +var express = require('..'); function req(ret) { return { @@ -27,5 +28,11 @@ describe('req', function(){ ret.type = 'users'; req('users=0-').range(Infinity).should.eql(ret); }) + + it('should return undefined if no range', function(){ + var ret = [{ start: 0, end: 50 }, { start: 60, end: 100 }]; + ret.type = 'bytes'; + assert(req('').range(120) === undefined); + }) }) }) From 602e5a82004d243f626002c52d843a0c14ac7ed4 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 23:41:09 -0400 Subject: [PATCH 09/16] tests: add more tests of mvc example --- examples/mvc/controllers/pet/index.js | 2 +- test/acceptance/mvc.js | 64 +++++++++++++++++++++------ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/examples/mvc/controllers/pet/index.js b/examples/mvc/controllers/pet/index.js index bce0eacb..9f3c3452 100644 --- a/examples/mvc/controllers/pet/index.js +++ b/examples/mvc/controllers/pet/index.js @@ -20,7 +20,7 @@ exports.edit = function(req, res, next){ exports.update = function(req, res, next){ var body = req.body; - req.pet.name = body.user.name; + req.pet.name = body.pet.name; res.message('Information updated!'); res.redirect('/pet/' + req.pet.id); }; diff --git a/test/acceptance/mvc.js b/test/acceptance/mvc.js index 3386d833..813c5506 100644 --- a/test/acceptance/mvc.js +++ b/test/acceptance/mvc.js @@ -7,10 +7,38 @@ describe('mvc', function(){ it('should redirect to /users', function(done){ request(app) .get('/') + .expect('Location', '/users') + .expect(302, done) + }) + }) + + describe('GET /pet/0', function(){ + it('should get pet', function(done){ + request(app) + .get('/pet/0') + .expect(200, /Tobi/, done) + }) + }) + + describe('GET /pet/0/edit', function(){ + it('should get pet edit page', function(done){ + request(app) + .get('/pet/0/edit') + .expect(/
Guillermo'); - res.text.should.include('value="put"'); - done(); - }) + .expect(/Guillermo/) + .expect(200, /Tobo'); - done(); - }) + .expect(200, /Tobo/, done) }) }) }) -}) \ No newline at end of file + + describe('POST /user/:id/pet', function(){ + it('should create a pet for user', function(done){ + request(app) + .post('/user/2/pet') + .send({ pet: { name: 'Snickers' }}) + .expect('Location', '/user/2') + .expect(302, function(err, res){ + if (err) return done(err) + request(app) + .get('/user/2') + .expect(200, /Snickers/, done) + }) + }) + }) +}) From 1944451082e8cdb2882f8b00fb1ddc04e8eab1a5 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Tue, 20 May 2014 23:50:58 -0400 Subject: [PATCH 10/16] tests: add more tests of negotiation example --- test/acceptance/content-negotiation.js | 37 ++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/test/acceptance/content-negotiation.js b/test/acceptance/content-negotiation.js index 99842585..ac9dbaf5 100644 --- a/test/acceptance/content-negotiation.js +++ b/test/acceptance/content-negotiation.js @@ -7,16 +7,43 @@ describe('content-negotiation', function(){ it('should default to text/html', function(done){ request(app) .get('/') - .expect('
  • Tobi
  • Loki
  • Jane
') - .end(done); + .expect(200, '
  • Tobi
  • Loki
  • Jane
', done) }) it('should accept to text/plain', function(done){ request(app) .get('/') .set('Accept', 'text/plain') - .expect(' - Tobi\n - Loki\n - Jane\n') - .end(done); + .expect(200, ' - Tobi\n - Loki\n - Jane\n', done) + }) + + it('should accept to application/json', function(done){ + request(app) + .get('/') + .set('Accept', 'application/json') + .expect(200, '[{"name":"Tobi"},{"name":"Loki"},{"name":"Jane"}]', done) }) }) -}) \ No newline at end of file + + describe('GET /users', function(){ + it('should default to text/html', function(done){ + request(app) + .get('/users') + .expect(200, '
  • Tobi
  • Loki
  • Jane
', done) + }) + + it('should accept to text/plain', function(done){ + request(app) + .get('/users') + .set('Accept', 'text/plain') + .expect(200, ' - Tobi\n - Loki\n - Jane\n', done) + }) + + it('should accept to application/json', function(done){ + request(app) + .get('/users') + .set('Accept', 'application/json') + .expect(200, '[{"name":"Tobi"},{"name":"Loki"},{"name":"Jane"}]', done) + }) + }) +}) From cf5de082b5f8a41d56d24d3f7eb885b7e8c84519 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Wed, 21 May 2014 00:08:06 -0400 Subject: [PATCH 11/16] tests: add more tests of cookies example --- test/acceptance/cookies.js | 43 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/test/acceptance/cookies.js b/test/acceptance/cookies.js index d6d4f007..987c539e 100644 --- a/test/acceptance/cookies.js +++ b/test/acceptance/cookies.js @@ -18,6 +18,35 @@ describe('cookies', function(){ done() }) }) + + it('should respond to cookie', function(done){ + request(app) + .post('/') + .send({ remember: 1 }) + .expect(302, function(err, res){ + if (err) return done(err) + request(app) + .get('/') + .set('Cookie', res.headers['set-cookie'][0]) + .expect(200, /Remembered/, done) + }) + }) + }) + + describe('GET /forget', function(){ + it('should clear cookie', function(done){ + request(app) + .post('/') + .send({ remember: 1 }) + .expect(302, function(err, res){ + if (err) return done(err) + request(app) + .get('/forget') + .set('Cookie', res.headers['set-cookie'][0]) + .expect('Set-Cookie', /remember=;/) + .expect(302, done) + }) + }) }) describe('POST /', function(){ @@ -25,10 +54,20 @@ describe('cookies', function(){ request(app) .post('/') .send({ remember: 1 }) - .end(function(err, res){ + .expect(302, function(err, res){ res.headers.should.have.property('set-cookie') done() }) }) + + it('should no set cookie w/o reminder', function(done){ + request(app) + .post('/') + .send({}) + .expect(302, function(err, res){ + res.headers.should.not.have.property('set-cookie') + done() + }) + }) }) -}) \ No newline at end of file +}) From 8d7d80ef9d307a3b76e9d202e7527350d47c4531 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Wed, 21 May 2014 00:08:17 -0400 Subject: [PATCH 12/16] tests: add more tests of web-service example --- test/acceptance/web-service.js | 73 +++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/test/acceptance/web-service.js b/test/acceptance/web-service.js index 67e9b44a..9fdf7a4e 100644 --- a/test/acceptance/web-service.js +++ b/test/acceptance/web-service.js @@ -24,11 +24,72 @@ describe('web-service', function(){ it('should respond users json', function(done){ request(app) .get('/api/users?api-key=foo') - .end(function(err, res){ - res.should.be.json; - res.text.should.equal('[{"name":"tobi"},{"name":"loki"},{"name":"jane"}]'); - done(); - }); + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '[{"name":"tobi"},{"name":"loki"},{"name":"jane"}]', done) + }) + }) + }) + + describe('GET /api/repos', function(){ + describe('without an api key', function(){ + it('should respond with 400 bad request', function(done){ + request(app) + .get('/api/repos') + .expect(400, done); + }) + }) + + describe('with an invalid api key', function(){ + it('should respond with 401 unauthorized', function(done){ + request(app) + .get('/api/repos?api-key=rawr') + .expect(401, done); + }) + }) + + describe('with a valid api key', function(){ + it('should respond repos json', function(done){ + request(app) + .get('/api/repos?api-key=foo') + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(/"name":"express"/) + .expect(/"url":"http:\/\/github.com\/visionmedia\/express"/) + .expect(200, done) + }) + }) + }) + + describe('GET /api/user/:name/repos', function(){ + describe('without an api key', function(){ + it('should respond with 400 bad request', function(done){ + request(app) + .get('/api/user/loki/repos') + .expect(400, done); + }) + }) + + describe('with an invalid api key', function(){ + it('should respond with 401 unauthorized', function(done){ + request(app) + .get('/api/user/loki/repos?api-key=rawr') + .expect(401, done); + }) + }) + + describe('with a valid api key', function(){ + it('should respond user repos json', function(done){ + request(app) + .get('/api/user/loki/repos?api-key=foo') + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(/"name":"stylus"/) + .expect(/"url":"http:\/\/github.com\/learnboost\/stylus"/) + .expect(200, done) + }) + + it('should 404 with unknown user', function(done){ + request(app) + .get('/api/user/bob/repos?api-key=foo') + .expect(404, done) }) }) }) @@ -45,4 +106,4 @@ describe('web-service', function(){ }); }) }) -}) \ No newline at end of file +}) From b0f72e13d95b56e78673e9fdf0bc7214f23f4bcf Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Wed, 21 May 2014 00:55:10 -0400 Subject: [PATCH 13/16] update connect to 2.17.1 --- History.md | 3 ++- package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 0923cf07..a7dcc2ef 100644 --- a/History.md +++ b/History.md @@ -1,7 +1,8 @@ 3.x === - * update connect to 2.17.0 + * update connect to 2.17.1 + - fix `res.charset` appending charset when `content-type` has one - deps: express-session@1.2.0 - deps: morgan@1.1.1 - deps: serve-index@1.0.3 diff --git a/package.json b/package.json index f5e3e981..ef84bfe2 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "repository": "git://github.com/visionmedia/express", "license": "MIT", "dependencies": { - "connect": "2.17.0", + "connect": "2.17.1", "commander": "1.3.2", "methods": "1.0.0", "mkdirp": "0.5.0", From 084f5d891b4dd413caca21283a3127bb28f9069d Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Fri, 2 May 2014 15:33:09 -0300 Subject: [PATCH 14/16] Keep previous Content-Type for res.jsonp backport of commit be997fd654243a8633aa059343665cf2c88fe609 --- History.md | 1 + lib/response.js | 2 +- test/res.jsonp.js | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/History.md b/History.md index a7dcc2ef..9883f469 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,7 @@ 3.x === + * keep previous `Content-Type` for `res.jsonp` * update connect to 2.17.1 - fix `res.charset` appending charset when `content-type` has one - deps: express-session@1.2.0 diff --git a/lib/response.js b/lib/response.js index 8054e5ca..11b1f7a4 100644 --- a/lib/response.js +++ b/lib/response.js @@ -248,7 +248,7 @@ res.jsonp = function(obj){ // content-type this.charset = this.charset || 'utf-8'; - this.set('Content-Type', 'application/json'); + this.get('Content-Type') || this.set('Content-Type', 'application/json'); // fixup callback if (Array.isArray(callback)) { diff --git a/test/res.jsonp.js b/test/res.jsonp.js index 03087ec4..7900b5a4 100644 --- a/test/res.jsonp.js +++ b/test/res.jsonp.js @@ -334,4 +334,22 @@ describe('res', function(){ }) }) }) + + it('should not override previous Content-Types', function(done){ + var app = express(); + + app.get('/', function(req, res){ + res.type('application/vnd.example+json'); + res.jsonp({ hello: 'world' }); + }); + + request(app) + .get('/') + .end(function(err, res){ + res.statusCode.should.equal(200); + res.headers.should.have.property('content-type', 'application/vnd.example+json'); + res.text.should.equal('{"hello":"world"}'); + done(); + }) + }) }) From f14e39d4515c81c1e5c980b7ce31566eeeb84734 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Thu, 8 May 2014 23:27:21 -0400 Subject: [PATCH 15/16] set proper charset in content-type for res.send This will write strings to sockets with an explicit "utf8" encoding (which is the default) and will override the charset in the Content-Type so it properly relfects the encoding of the response. closes #1631 closes #2092 --- History.md | 1 + lib/response.js | 17 +++++- lib/utils.js | 36 ++++++++++++ test/res.attachment.js | 2 +- test/res.charset.js | 4 +- test/res.format.js | 6 +- test/res.json.js | 95 ++++++++++++------------------- test/res.jsonp.js | 123 ++++++++++++++++++----------------------- test/res.send.js | 38 ++++++++++--- 9 files changed, 179 insertions(+), 143 deletions(-) diff --git a/History.md b/History.md index 9883f469..32ee57bc 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,7 @@ === * keep previous `Content-Type` for `res.jsonp` + * set proper `charset` in `Content-Type` for `res.send` * update connect to 2.17.1 - fix `res.charset` appending charset when `content-type` has one - deps: express-session@1.2.0 diff --git a/lib/response.js b/lib/response.js index 11b1f7a4..b7ec7ae5 100644 --- a/lib/response.js +++ b/lib/response.js @@ -9,6 +9,7 @@ var http = require('http') , sign = require('cookie-signature').sign , normalizeType = require('./utils').normalizeType , normalizeTypes = require('./utils').normalizeTypes + , setCharset = require('./utils').setCharset , deprecate = require('./utils').deprecate , etag = require('./utils').etag , statusCodes = http.STATUS_CODES @@ -83,6 +84,8 @@ res.links = function(links){ res.send = function(body){ var req = this.req; var head = 'HEAD' == req.method; + var type; + var encoding; var len; // settings @@ -140,6 +143,17 @@ res.send = function(body){ } } + // write strings in utf-8 + if ('string' === typeof body) { + encoding = 'utf8'; + type = this.get('Content-Type'); + + // reflect this in content-type + if ('string' === typeof type) { + this.set('Content-Type', setCharset(type, 'utf-8')); + } + } + // freshness if (req.fresh) this.statusCode = 304; @@ -152,7 +166,8 @@ res.send = function(body){ } // respond - this.end(head ? null : body); + this.end((head ? null : body), encoding); + return this; }; diff --git a/lib/utils.js b/lib/utils.js index d9dff89d..6abce481 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -14,6 +14,11 @@ var mime = require('connect').mime var toString = {}.toString; +/** + * Simple detection of charset parameter in content-type + */ +var charsetRegExp = /;\s*charset\s*=/; + /** * Deprecate function, like core `util.deprecate`, * but with NODE_ENV and color support. @@ -367,3 +372,34 @@ exports.compileTrust = function(val) { return proxyaddr.compile(val || []); } + +/** + * Set the charset in a given Content-Type string. + * + * @param {String} type + * @param {String} charset + * @return {String} + * @api private + */ + +exports.setCharset = function(type, charset){ + if (!type || !charset) return type; + + var exists = charsetRegExp.test(type); + + // removing existing charset + if (exists) { + var parts = type.split(';'); + + for (var i = 1; i < parts.length; i++) { + if (charsetRegExp.test(';' + parts[i])) { + parts.splice(i, 1); + break; + } + } + + type = parts.join(';'); + } + + return type + '; charset=' + charset; +}; diff --git a/test/res.attachment.js b/test/res.attachment.js index 1049b07c..4e0421f6 100644 --- a/test/res.attachment.js +++ b/test/res.attachment.js @@ -36,7 +36,7 @@ describe('res', function(){ app.use(function(req, res){ res.attachment('/path/to/image.png'); - res.send('foo'); + res.send(new Buffer(4)); }); request(app) diff --git a/test/res.charset.js b/test/res.charset.js index 2403ba4d..8fd2d409 100644 --- a/test/res.charset.js +++ b/test/res.charset.js @@ -18,7 +18,7 @@ describe('res', function(){ .expect("text/x-foo; charset=utf-8", done); }) - it('should take precedence over res.send() defaults', function(done){ + it('should be replaced by real charset in res.send', function(done){ var app = express(); app.use(function(req, res){ @@ -28,7 +28,7 @@ describe('res', function(){ request(app) .get('/') - .expect('Content-Type', 'text/html; charset=whoop', done); + .expect('Content-Type', 'text/html; charset=utf-8', done); }) }) }) diff --git a/test/res.format.js b/test/res.format.js index be1e6cbb..105c2b05 100644 --- a/test/res.format.js +++ b/test/res.format.js @@ -111,7 +111,7 @@ function test(app) { request(app) .get('/') .set('Accept', 'text/html; q=.5, text/plain') - .expect('Content-Type', 'text/plain; charset=UTF-8') + .expect('Content-Type', 'text/plain; charset=utf-8') .expect('hey', done); }) @@ -119,12 +119,12 @@ function test(app) { request(app) .get('/') .set('Accept', 'text/html') - .expect('Content-Type', 'text/html; charset=UTF-8'); + .expect('Content-Type', 'text/html; charset=utf-8'); request(app) .get('/') .set('Accept', 'text/plain') - .expect('Content-Type', 'text/plain; charset=UTF-8'); + .expect('Content-Type', 'text/plain; charset=utf-8'); request(app) .get('/') diff --git a/test/res.json.js b/test/res.json.js index 9cefe259..f9c2213b 100644 --- a/test/res.json.js +++ b/test/res.json.js @@ -17,6 +17,20 @@ describe('res', function(){ .expect('{"foo":"bar"}', done); }) + it('should not override previous Content-Types', function(done){ + var app = express(); + + app.get('/', function(req, res){ + res.type('application/vnd.example+json'); + res.json({ hello: 'world' }); + }); + + request(app) + .get('/') + .expect('Content-Type', 'application/vnd.example+json; charset=utf-8') + .expect(200, '{"hello":"world"}', done); + }) + describe('when given primitives', function(){ it('should respond with json for null', function(done){ var app = express(); @@ -27,11 +41,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('null'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, 'null', done) }) it('should respond with json for Number', function(done){ @@ -43,12 +54,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('300'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '300', done) }) it('should respond with json for String', function(done){ @@ -60,12 +67,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('"str"'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '"str"', done) }) }) @@ -79,11 +82,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('["foo","bar","baz"]'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '["foo","bar","baz"]', done) }) }) @@ -97,11 +97,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"name":"tobi"}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"name":"tobi"}', done) }) }) @@ -121,10 +118,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.text.should.equal('{"name":"tobi"}'); - done(); - }); + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"name":"tobi"}', done) }) }) @@ -152,10 +147,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.text.should.equal('{\n "name": "tobi",\n "age": 2\n}'); - done(); - }); + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{\n "name": "tobi",\n "age": 2\n}', done) }) }) }) @@ -170,12 +163,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"id":1}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '{"id":1}', done) }) }) @@ -189,12 +178,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"id":1}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '{"id":1}', done) }) it('should use status as second number for backwards compat', function(done){ @@ -206,12 +191,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('200'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '200', done) }) }) @@ -225,11 +206,7 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/vnd.example+json'); - res.text.should.equal('{"hello":"world"}'); - done(); - }) + .expect('content-type', 'application/vnd.example+json; charset=utf-8') + .expect(200, '{"hello":"world"}', done) }) }) diff --git a/test/res.jsonp.js b/test/res.jsonp.js index 7900b5a4..3637a23e 100644 --- a/test/res.jsonp.js +++ b/test/res.jsonp.js @@ -46,11 +46,8 @@ describe('res', function(){ request(app) .get('/?callback[a]=something') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"count":1}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"count":1}', done) }) it('should allow renaming callback', function(done){ @@ -119,6 +116,34 @@ describe('res', function(){ }); }); + it('should not override previous Content-Types with no callback', function(done){ + var app = express(); + + app.get('/', function(req, res){ + res.type('application/vnd.example+json'); + res.jsonp({ hello: 'world' }); + }); + + request(app) + .get('/') + .expect('Content-Type', 'application/vnd.example+json; charset=utf-8') + .expect(200, '{"hello":"world"}', done); + }) + + it('should override previous Content-Types with callback', function(done){ + var app = express(); + + app.get('/', function(req, res){ + res.type('application/vnd.example+json'); + res.jsonp({ hello: 'world' }); + }); + + request(app) + .get('/?callback=cb') + .expect('Content-Type', 'text/javascript; charset=utf-8') + .expect(200, /cb\(\{"hello":"world"\}\);$/, done); + }) + describe('when given primitives', function(){ it('should respond with json', function(done){ var app = express(); @@ -129,11 +154,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('null'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, 'null', done) }) }) @@ -147,11 +169,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('["foo","bar","baz"]'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '["foo","bar","baz"]', done) }) }) @@ -165,11 +184,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"name":"tobi"}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"name":"tobi"}', done) }) }) @@ -183,11 +199,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('null'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, 'null', done) }) it('should respond with json for Number', function(done){ @@ -199,12 +212,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('300'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '300', done) }) it('should respond with json for String', function(done){ @@ -216,12 +225,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('"str"'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '"str"', done) }) }) @@ -241,10 +246,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.text.should.equal('{"name":"tobi"}'); - done(); - }); + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"name":"tobi"}', done) }) }) @@ -272,10 +275,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.text.should.equal('{\n "name": "tobi",\n "age": 2\n}'); - done(); - }); + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{\n "name": "tobi",\n "age": 2\n}', done) }) }) }) @@ -290,12 +291,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"id":1}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '{"id":1}', done) }) }) @@ -309,12 +306,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"id":1}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '{"id":1}', done) }) it('should use status as second number for backwards compat', function(done){ @@ -326,12 +319,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(201); - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('200'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(201, '200', done) }) }) @@ -345,11 +334,7 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.statusCode.should.equal(200); - res.headers.should.have.property('content-type', 'application/vnd.example+json'); - res.text.should.equal('{"hello":"world"}'); - done(); - }) + .expect('content-type', 'application/vnd.example+json; charset=utf-8') + .expect(200, '{"hello":"world"}', done) }) }) diff --git a/test/res.send.js b/test/res.send.js index 98d4b875..3eb27766 100644 --- a/test/res.send.js +++ b/test/res.send.js @@ -135,9 +135,34 @@ describe('res', function(){ request(app) .get('/') - .expect('Content-Type', 'text/plain') - .expect('hey') - .expect(200, done); + .expect('Content-Type', 'text/plain; charset=utf-8') + .expect(200, 'hey', done); + }) + + it('should override charset in Content-Type', function(done){ + var app = express(); + + app.use(function(req, res){ + res.set('Content-Type', 'text/plain; charset=iso-8859-1').send('hey'); + }); + + request(app) + .get('/') + .expect('Content-Type', 'text/plain; charset=utf-8') + .expect(200, 'hey', done); + }) + + it('should keep charset in Content-Type for Buffers', function(done){ + var app = express(); + + app.use(function(req, res){ + res.set('Content-Type', 'text/plain; charset=iso-8859-1').send(new Buffer('hi')); + }); + + request(app) + .get('/') + .expect('Content-Type', 'text/plain; charset=iso-8859-1') + .expect(200, 'hi', done); }) }) @@ -201,11 +226,8 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res){ - res.headers.should.have.property('content-type', 'application/json; charset=utf-8'); - res.text.should.equal('{"name":"tobi"}'); - done(); - }) + .expect('Content-Type', 'application/json; charset=utf-8') + .expect(200, '{"name":"tobi"}', done) }) }) From f6bbeafd26daf12891d912ef604bad5b206d904f Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Wed, 21 May 2014 01:52:14 -0400 Subject: [PATCH 16/16] 3.8.0 --- History.md | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/History.md b/History.md index 32ee57bc..c34469bb 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,5 @@ -3.x -=== +3.8.0 / 2014-05-21 +================== * keep previous `Content-Type` for `res.jsonp` * set proper `charset` in `Content-Type` for `res.send` diff --git a/package.json b/package.json index ef84bfe2..9784985e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "express", "description": "Sinatra inspired web development framework", - "version": "3.7.0", + "version": "3.8.0", "author": "TJ Holowaychuk ", "contributors": [ {