From 5584430ddcc148932a3332dd1bfcd0f872b48858 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Mon, 5 Jul 2021 18:46:02 +0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20a=20500=20error=20for=20?=
 =?UTF-8?q?incorrect=20fields=20parameter=20in=20API?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes https://github.com/TryGhost/Team/issues/817
refs https://github.com/TryGhost/framework/blob/6d083ee00e9b6d73dac71c1737fc76b68f9ea5f2/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
---
 core/server/models/base/plugins/crud.js       | 21 +++++++++++++++++--
 .../regression/api/canary/admin/posts_spec.js |  8 +++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/core/server/models/base/plugins/crud.js b/core/server/models/base/plugins/crud.js
index faa6944d62..4bdfe0e803 100644
--- a/core/server/models/base/plugins/crud.js
+++ b/core/server/models/base/plugins/crud.js
@@ -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;
+            }
         },
 
         /**
diff --git a/test/regression/api/canary/admin/posts_spec.js b/test/regression/api/canary/admin/posts_spec.js
index 732f6d0ffd..d6bceb5e27 100644
--- a/test/regression/api/canary/admin/posts_spec.js
+++ b/test/regression/api/canary/admin/posts_spec.js
@@ -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 () {