mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-04-08 02:52:39 -05:00
🐛 Fixed a 500 error for incorrect fields parameter in API
closes https://github.com/TryGhost/Team/issues/817
refs 6d083ee00e/packages/bookshelf-pagination/lib/bookshelf-pagination.js (L256)
- The 500 error is not the best we can do in this situation and throwing a 400 just like we doo in a referenced commit would keep the convention
- The underlying problem of the bug is bigger - we allow the fields named the same way as relations to leak into the db query and that causes an incorrect SQL syntax. It's a bigger problem which would need a separate, holistic approach
This commit is contained in:
parent
9c2cfb5d00
commit
5584430ddc
2 changed files with 27 additions and 2 deletions
|
@ -1,5 +1,10 @@
|
|||
const _ = require('lodash');
|
||||
const errors = require('@tryghost/errors');
|
||||
const tpl = require('@tryghost/tpl');
|
||||
|
||||
const messages = {
|
||||
couldNotUnderstandRequest: 'Could not understand request.'
|
||||
};
|
||||
|
||||
/**
|
||||
* @param {Bookshelf} Bookshelf
|
||||
|
@ -116,7 +121,7 @@ module.exports = function (Bookshelf) {
|
|||
* @param {Object} [unfilteredOptions]
|
||||
* @return {Promise<Bookshelf['Model']>} Single Model
|
||||
*/
|
||||
findOne: function findOne(data, unfilteredOptions) {
|
||||
findOne: async function findOne(data, unfilteredOptions) {
|
||||
const options = this.filterOptions(unfilteredOptions, 'findOne');
|
||||
data = this.filterData(data);
|
||||
const model = this.forge(data);
|
||||
|
@ -131,7 +136,19 @@ module.exports = function (Bookshelf) {
|
|||
options.columns = _.intersection(options.columns, this.prototype.permittedAttributes());
|
||||
}
|
||||
|
||||
return model.fetch(options);
|
||||
try {
|
||||
return await model.fetch(options);
|
||||
} catch (err) {
|
||||
// CASE: SQL syntax is incorrect
|
||||
if (err.errno === 1054 || err.errno === 1) {
|
||||
throw new errors.BadRequestError({
|
||||
message: tpl(messages.couldNotUnderstandRequest),
|
||||
err
|
||||
});
|
||||
}
|
||||
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
|
@ -283,6 +283,14 @@ describe('Posts API (canary)', function () {
|
|||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('throws a 400 when a non-existing field is requested', async function () {
|
||||
await request.get(localUtils.API.getApiQuery(`posts/slug/${testUtils.DataGenerator.Content.posts[0].slug}/?fields=tags`))
|
||||
.set('Origin', config.get('url'))
|
||||
.expect('Content-Type', /json/)
|
||||
.expect('Cache-Control', testUtils.cacheRules.private)
|
||||
.expect(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Add', function () {
|
||||
|
|
Loading…
Add table
Reference in a new issue