From e2f61595b8ef00b8107b36380e879bce92553bf9 Mon Sep 17 00:00:00 2001 From: dead_horse Date: Wed, 6 Aug 2014 21:10:52 +0800 Subject: [PATCH 1/2] fix err.status invalid lead to uncaughtException --- lib/context.js | 2 +- test/context/onerror.js | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/context.js b/lib/context.js index 02b85d5..48abfee 100644 --- a/lib/context.js +++ b/lib/context.js @@ -115,7 +115,7 @@ var proto = module.exports = { if ('ENOENT' == err.code) err.status = 404; // default to 500 - err.status = err.status || 500; + if ('number' != typeof err.status || !http.STATUS_CODES[err.status]) err.status = 500; // respond var code = http.STATUS_CODES[err.status]; diff --git a/test/context/onerror.js b/test/context/onerror.js index 6bf9c85..69c14b6 100644 --- a/test/context/onerror.js +++ b/test/context/onerror.js @@ -50,4 +50,47 @@ describe('ctx.onerror(err)', function(){ done(); }) }) + describe('when invalid err.status', function(){ + describe('not number', function(){ + it('should respond 500', function(done){ + var app = koa(); + + app.use(function *(next){ + this.body = 'something else'; + var err = new Error('some error'); + err.status = 'notnumber'; + this.throw(err); + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(500) + .expect('Content-Type', 'text/plain; charset=utf-8') + .expect('Internal Server Error', done); + }) + }) + + describe('not http status code', function(){ + it('should respond 500', function(done){ + var app = koa(); + + app.use(function *(next){ + this.body = 'something else'; + var err = new Error('some error'); + err.status = 9999; + this.throw(err); + }) + + var server = app.listen(); + + request(server) + .get('/') + .expect(500) + .expect('Content-Type', 'text/plain; charset=utf-8') + .expect('Internal Server Error', done); + }) + }) + }) }) From fa5948cca3b2a70f6ba5e0e93833c631d57e3565 Mon Sep 17 00:00:00 2001 From: dead_horse Date: Wed, 6 Aug 2014 21:31:55 +0800 Subject: [PATCH 2/2] do not expose when err.status not valid --- lib/context.js | 2 +- test/context/onerror.js | 5 +++-- test/context/throw.js | 24 +++++++++++++++++++++++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/context.js b/lib/context.js index 48abfee..5691569 100644 --- a/lib/context.js +++ b/lib/context.js @@ -75,7 +75,7 @@ var proto = module.exports = { var err = msg instanceof Error ? msg : new Error(msg); err.status = status || err.status || 500; - err.expose = err.status < 500; + err.expose = 'number' == typeof err.status && http.STATUS_CODES[err.status] && err.status < 500; throw err; }, diff --git a/test/context/onerror.js b/test/context/onerror.js index 69c14b6..f270bc3 100644 --- a/test/context/onerror.js +++ b/test/context/onerror.js @@ -50,6 +50,7 @@ describe('ctx.onerror(err)', function(){ done(); }) }) + describe('when invalid err.status', function(){ describe('not number', function(){ it('should respond 500', function(done){ @@ -59,7 +60,7 @@ describe('ctx.onerror(err)', function(){ this.body = 'something else'; var err = new Error('some error'); err.status = 'notnumber'; - this.throw(err); + throw err; }) var server = app.listen(); @@ -80,7 +81,7 @@ describe('ctx.onerror(err)', function(){ this.body = 'something else'; var err = new Error('some error'); err.status = 9999; - this.throw(err); + throw err; }) var server = app.listen(); diff --git a/test/context/throw.js b/test/context/throw.js index ee06976..ce332ac 100644 --- a/test/context/throw.js +++ b/test/context/throw.js @@ -10,7 +10,7 @@ describe('ctx.throw(msg)', function(){ ctx.throw('boom'); } catch (err) { assert(500 == err.status); - assert(false === err.expose); + assert(!err.expose); done(); } }) @@ -26,6 +26,7 @@ describe('ctx.throw(err)', function(){ } catch (err) { assert(500 == err.status); assert('test' == err.message); + assert(!err.expose); done(); } }) @@ -41,6 +42,7 @@ describe('ctx.throw(err, status)', function(){ } catch (err) { assert(422 == err.status); assert('test' == err.message); + assert(true === err.expose); done(); } }) @@ -56,6 +58,7 @@ describe('ctx.throw(status, err)', function(){ } catch (err) { assert(422 == err.status); assert('test' == err.message); + assert(true === err.expose); done(); } }) @@ -70,6 +73,7 @@ describe('ctx.throw(msg, status)', function(){ } catch (err) { assert('name required' == err.message); assert(400 == err.status); + assert(true === err.expose); done(); } }) @@ -84,6 +88,7 @@ describe('ctx.throw(status, msg)', function(){ } catch (err) { assert('name required' == err.message); assert(400 == err.status); + assert(true === err.expose); done(); } }) @@ -98,7 +103,24 @@ describe('ctx.throw(status)', function(){ } catch (err) { assert('Bad Request' == err.message); assert(400 == err.status); + assert(true === err.expose); done(); } }) + + describe('when not valid status', function(){ + it('should not expose', function(done){ + var ctx = context(); + + try { + var err = new Error('some error'); + err.status = -1; + ctx.throw(err); + } catch(err) { + assert('some error' == err.message); + assert(!err.expose); + done(); + } + }) + }) })