From 71167b0e75a6ed68eb6b9e6c5d88b823c36fe154 Mon Sep 17 00:00:00 2001 From: Sebastien Vaucouleur Date: Fri, 24 Jul 2015 13:57:29 +0200 Subject: [PATCH] The article middleware was calling getErrorMessage with a null argument, causing a crash when this method tried to access 'code' on an null parameter. The bug was not exposed by the original test, since it was mixing two (related) aspects: * An invalid Id (a badly formed mongodb identifier) * An non-existent Id (an identifier with no corresponding document in the database) Modifications: - Fixed the message property in the article controller (the error message follows the wording of the error message in "users.password.server.controller.js", in case of username not found) - Added a new test to check modifications and avoid regressions --- .../controllers/articles.server.controller.js | 2 +- .../tests/server/article.server.routes.tests.js | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/modules/articles/server/controllers/articles.server.controller.js b/modules/articles/server/controllers/articles.server.controller.js index 8f063226f2..02c9b9df0e 100644 --- a/modules/articles/server/controllers/articles.server.controller.js +++ b/modules/articles/server/controllers/articles.server.controller.js @@ -100,7 +100,7 @@ exports.articleByID = function(req, res, next, id) { if (err) return next(err); if (!article) { return res.status(404).send({ - message: errorHandler.getErrorMessage(err) + message: 'No article with that identifier has been found' }); } req.article = article; diff --git a/modules/articles/tests/server/article.server.routes.tests.js b/modules/articles/tests/server/article.server.routes.tests.js index 7f9baeabf4..3c76cc6442 100644 --- a/modules/articles/tests/server/article.server.routes.tests.js +++ b/modules/articles/tests/server/article.server.routes.tests.js @@ -223,7 +223,8 @@ describe('Article CRUD tests', function () { }); }); - it('should return proper error for single article which doesnt exist, if not signed in', function (done) { + it('should return proper error for single article with an invalid Id, if not signed in', function (done) { + // test is not a valid mongoose Id request(app).get('/api/articles/test') .end(function (req, res) { // Set assertion @@ -234,6 +235,18 @@ describe('Article CRUD tests', function () { }); }); + it('should return proper error for single article which doesnt exist, if not signed in', function (done) { + // This is a valid mongoose Id but a non-existent article + request(app).get('/api/articles/559e9cd815f80b4c256a8f41') + .end(function (req, res) { + // Set assertion + res.body.should.be.instanceof(Object).and.have.property('message', 'No article with that identifier has been found'); + + // Call the assertion callback + done(); + }); + }); + it('should be able to delete an article if signed in', function (done) { agent.post('/api/auth/signin') .send(credentials)