TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-12-31 23:06:19 +03:00
Code Issues Projects Releases Wiki Activity

Return a single 422 error for invalid values

Browse Source 
refs #5808

- Fix the API to return a single 422 error when an invalid value is passed
- Only affects Browse, and not Read at present due to differences in how they are handled
- Frontend was changed to always 404 in #5851
- Adds tests to ensure all cases are covered
This commit is contained in:
Hannah Wolfe 2015-09-22 17:38:30 +01:00
parent 63f09687bb
commit 545bea0eaf
9 changed files with 86 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
core/server/api/utils.js
View File

@ -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));
}
}
});

4
core/server/data/validation/index.js
View File

@ -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) {

2
core/server/errors/index.js
View File

@ -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 = [];

16
core/test/functional/routes/frontend_spec.js
View File

@ -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 () {

34
core/test/integration/api/api_posts_spec.js
View File

@ -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();
});
});
});
});

2
core/test/integration/api/api_slugs_spec.js
View File

@ -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);
});

12
core/test/integration/api/api_tags_spec.js
View File

@ -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();
});
});
});
});

12
core/test/integration/api/api_users_spec.js
View File

@ -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 () {

2
core/test/unit/api_utils_spec.js
View File

@ -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();
});
});