diff --git a/core/server/api/utils.js b/core/server/api/utils.js index bbe4d28a39..ab1407d162 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -89,7 +89,8 @@ utils = { return Promise.resolve(options); } - return errors.logAndRejectError(validationErrors); + // For now, we can only handle showing the first validation error + return errors.logAndRejectError(validationErrors[0]); } // If we got an object, check that too @@ -110,6 +111,7 @@ utils = { var globalValidations = { id: {matches: /^\d+|me$/}, uuid: {isUUID: true}, + slug: {isSlug: true}, page: {matches: /^\d+$/}, limit: {matches: /^\d+|all$/}, fields: {matches: /^[a-z0-9_,]+$/}, @@ -125,8 +127,8 @@ utils = { if (globalValidations[key]) { errors = errors.concat(validation.validate(value, key, globalValidations[key])); } else { - // all other keys should be an alphanumeric string + -, like slug, tag, author, status, etc - errors = errors.concat(validation.validate(value, key, {matches: /^[a-z0-9\-_]+$/})); + // all other keys should be alpha-numeric with dashes/underscores, like tag, author, status, etc + errors = errors.concat(validation.validate(value, key, globalValidations.slug)); } } }); diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 592add8c30..8182a3d9ce 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -27,6 +27,10 @@ validator.extend('isEmptyOrURL', function isEmptyOrURL(str) { return (_.isEmpty(str) || validator.isURL(str, {require_protocol: false})); }); +validator.extend('isSlug', function isSlug(str) { + return validator.matches(str, /^[a-z0-9\-_]+$/); +}); + // Validation against schema attributes // values are checked against the validation objects from schema.js validateSchema = function validateSchema(tableName, model) { diff --git a/core/server/errors/index.js b/core/server/errors/index.js index 8e3d9dbd34..df223e16df 100644 --- a/core/server/errors/index.js +++ b/core/server/errors/index.js @@ -336,7 +336,7 @@ errors = { if (!err || !(err instanceof Error)) { next(); } - errors.renderErrorPage(err.status || 500, err, req, res, next); + errors.renderErrorPage(err.status || err.code || 500, err, req, res, next); } else { var statusCode = 500, returnErrors = []; diff --git a/core/test/functional/routes/frontend_spec.js b/core/test/functional/routes/frontend_spec.js index 72a7f5593b..bcd26d07ff 100644 --- a/core/test/functional/routes/frontend_spec.js +++ b/core/test/functional/routes/frontend_spec.js @@ -63,6 +63,14 @@ describe('Frontend Routing', function () { .end(doEnd(done)); }); + it('should 404 for unknown post with invalid characters', function (done) { + request.get('/$pec+acular~/') + .expect('Cache-Control', testUtils.cacheRules['private']) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); + it('should 404 for unknown frontend route', function (done) { request.get('/spectacular/marvellous/') .expect('Cache-Control', testUtils.cacheRules['private']) @@ -102,6 +110,14 @@ describe('Frontend Routing', function () { .expect(/Page not found/) .end(doEnd(done)); }); + + it('should 404 for unknown author with invalid characters', function (done) { + request.get('/author/ghost!user^/') + .expect('Cache-Control', testUtils.cacheRules['private']) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); }); describe('Home', function () { diff --git a/core/test/integration/api/api_posts_spec.js b/core/test/integration/api/api_posts_spec.js index 373b6d969c..70ec48c818 100644 --- a/core/test/integration/api/api_posts_spec.js +++ b/core/test/integration/api/api_posts_spec.js @@ -219,6 +219,28 @@ describe('Post API', function () { done(); }).catch(done); }); + + it('cannot fetch all posts for a tag with invalid slug', function (done) { + PostAPI.browse({tag: 'invalid!'}).then(function () { + done(new Error('Should not return a result with invalid tag')); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Validation (isSlug) failed for tag'); + err.code.should.eql(422); + done(); + }); + }); + + it('cannot fetch all posts for an author with invalid slug', function (done) { + PostAPI.browse({author: 'invalid!'}).then(function () { + done(new Error('Should not return a result with invalid author')); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Validation (isSlug) failed for author'); + err.code.should.eql(422); + done(); + }); + }); }); describe('Read', function () { @@ -372,5 +394,17 @@ describe('Post API', function () { done(); }).catch(done); }); + + // TODO: this should be a 422? + it('cannot fetch a post with an invalid slug', function (done) { + PostAPI.read({slug: 'invalid!'}).then(function () { + done(new Error('Should not return a result with invalid slug')); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Post not found.'); + + done(); + }); + }); }); }); diff --git a/core/test/integration/api/api_slugs_spec.js b/core/test/integration/api/api_slugs_spec.js index b670482a4b..0fad842500 100644 --- a/core/test/integration/api/api_slugs_spec.js +++ b/core/test/integration/api/api_slugs_spec.js @@ -77,7 +77,7 @@ describe('Slug API', function () { .then(function () { done(new Error('Generate a slug for an unknown type is not rejected.')); }).catch(function (errors) { - errors.should.have.enumerable(0).with.property('errorType', 'ValidationError'); + errors.should.have.property('errorType', 'ValidationError'); done(); }).catch(done); }); diff --git a/core/test/integration/api/api_tags_spec.js b/core/test/integration/api/api_tags_spec.js index 55d083b205..c764d69005 100644 --- a/core/test/integration/api/api_tags_spec.js +++ b/core/test/integration/api/api_tags_spec.js @@ -296,5 +296,17 @@ describe('Tags API', function () { done(); }).catch(done); }); + + // TODO: this should be a 422? + it('cannot fetch a tag with an invalid slug', function (done) { + TagAPI.read({slug: 'invalid!'}).then(function () { + done(new Error('Should not return a result with invalid slug')); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Tag not found.'); + + done(); + }); + }); }); }); diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index 2b2fd42294..38bb6b2954 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -194,6 +194,18 @@ describe('Users API', function () { done(); }).catch(done); }); + + // TODO: this should be a 422? + it('cannot fetch a user with an invalid slug', function (done) { + UserAPI.read({slug: 'invalid!'}).then(function () { + done(new Error('Should not return a result with invalid slug')); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('User not found.'); + + done(); + }); + }); }); describe('Edit', function () { diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 05f5ea0174..8a3a398685 100644 --- a/core/test/unit/api_utils_spec.js +++ b/core/test/unit/api_utils_spec.js @@ -163,7 +163,7 @@ describe('API Utils', function () { ).then(function () { done(new Error('Should have thrown a validation error')); }).catch(function (err) { - err.should.have.enumerable('0').with.property('errorType', 'ValidationError'); + err.should.have.property('errorType', 'ValidationError'); done(); }); });