From 64aad129d3d6f4602c08a10e49fbc38f18181290 Mon Sep 17 00:00:00 2001 From: dead_horse Date: Thu, 10 Apr 2014 00:21:15 +0800 Subject: [PATCH] fix this.status= in this.body fix default status set bug in this.body=null. do not call this.status= if this.status exist. make sure empty content status remove content headers --- lib/application.js | 6 +- lib/response.js | 5 +- test/application.js | 129 ++++++++++++++++++++++++++++++++++++++++ test/response/status.js | 35 ++++++++++- 4 files changed, 170 insertions(+), 5 deletions(-) diff --git a/lib/application.js b/lib/application.js index fbf3e87..12c3ea0 100644 --- a/lib/application.js +++ b/lib/application.js @@ -177,7 +177,11 @@ function *respond(next) { var head = 'HEAD' == this.method; // ignore body - if (status.empty[code]) return res.end(); + if (status.empty[code]) { + // strip headers + this.body = null; + return res.end(); + } if (null == body) { // empty body diff --git a/lib/response.js b/lib/response.js index 1415d07..dfb495f 100644 --- a/lib/response.js +++ b/lib/response.js @@ -100,8 +100,7 @@ module.exports = { // no content if (null == val) { - var s = this.status; - this.status = 304 == s ? 304 : 204; + if (!status.empty[this.status]) this.status = 204; this.res.removeHeader('Content-Type'); this.res.removeHeader('Content-Length'); this.res.removeHeader('Transfer-Encoding'); @@ -109,7 +108,7 @@ module.exports = { } // set the status - this.status = this.status || 200; + if (!this.status) this.status = 200; // set the content-type only if not yet set var setType = !this.header['content-type']; diff --git a/test/application.js b/test/application.js index 4b0890d..8e0e25f 100644 --- a/test/application.js +++ b/test/application.js @@ -362,6 +362,95 @@ describe('app.respond', function(){ }) }) + describe('when .body is a null', function(){ + it('should respond 204 by default', function(done){ + var app = koa(); + + app.use(function *(){ + this.body = null; + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(204) + .expect('') + .end(function(err, res){ + if (err) return done(err); + + res.header.should.not.have.property('content-type'); + done(); + }) + }) + + it('should respond 204 with status=200', function(done){ + var app = koa(); + + app.use(function *(){ + this.status = 200; + this.body = null; + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(204) + .expect('') + .end(function(err, res){ + if (err) return done(err); + + res.header.should.not.have.property('content-type'); + done(); + }) + }) + + it('should respond 205 with status=205', function(done){ + var app = koa(); + + app.use(function *(){ + this.status = 205; + this.body = null; + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(205) + .expect('') + .end(function(err, res){ + if (err) return done(err); + + res.header.should.not.have.property('content-type'); + done(); + }) + }) + + it('should respond 304 with status=304', function(done){ + var app = koa(); + + app.use(function *(){ + this.status = 304; + this.body = null; + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(304) + .expect('') + .end(function(err, res){ + if (err) return done(err); + + res.header.should.not.have.property('content-type'); + done(); + }) + }) + }) + describe('when .body is a string', function(){ it('should respond', function(done){ var app = koa(); @@ -545,6 +634,46 @@ describe('app.respond', function(){ .end(done); }) }) + + describe('when status and body property', function(){ + it('should 200', function(done){ + var app = koa(); + + app.use(function *(){ + this.status = 304; + this.body = 'hello'; + this.status = 200; + }); + + var server = app.listen(); + + request(server) + .get('/') + .expect(200) + .expect('hello', done); + }) + + it('should 204', function(done) { + var app = koa(); + + app.use(function *(){ + this.status = 200; + this.body = 'hello'; + this.set('content-type', 'text/plain; charset=utf8'); + this.status = 204; + }); + + var server = app.listen(); + + request(server) + .get('/') + .expect(204) + .end(function (err, res) { + res.should.not.have.header('content-type'); + done(err); + }); + }); + }) }) describe('app.context', function(){ diff --git a/test/response/status.js b/test/response/status.js index 695f684..2af64bf 100644 --- a/test/response/status.js +++ b/test/response/status.js @@ -54,7 +54,36 @@ describe('res.status=', function(){ request(app.listen()) .get('/') .expect(status) - .end(done); + .end(function(err, res) { + res.should.not.have.header('content-type'); + res.should.not.have.header('content-length'); + res.should.not.have.header('content-encoding'); + res.text.should.have.length(0); + done(err); + }); + }) + + it('should strip content releated header fields after status set', function(done) { + var app = koa(); + + app.use(function *(){ + this.status = status; + this.body = { foo: 'bar' }; + this.set('Content-Type', 'application/json'); + this.set('Content-Length', '15'); + this.set('Transfer-Encoding', 'chunked'); + }); + + request(app.listen()) + .get('/') + .expect(status) + .end(function(err, res) { + res.should.not.have.header('content-type'); + res.should.not.have.header('content-length'); + res.should.not.have.header('content-encoding'); + res.text.should.have.length(0); + done(err); + }); }) } @@ -62,6 +91,10 @@ describe('res.status=', function(){ strip(204); }) + describe('when 205', function(){ + strip(205); + }) + describe('when 304', function(){ strip(304); })