From 2c1ae2e9afe10548cc881b8d6d2e426306129435 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Tue, 6 Jul 2021 11:58:14 +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

refs https://github.com/TryGhost/Ghost/commit/8a1fd1f57f9ea8f7acdacae27e42a2da2861f5bf
refs https://github.com/TryGhost/Ghost/commit/5584430ddcc148932a3332dd1bfcd0f872b48858

- The change to async/await in the original commit 558443 was causing problems in downstream dependencies (create-error package) where it was loosing a context of "this". It's not a direct dependency so I didn't go yak shaving into where exacly the context is lost.
- The fix to keep a correct context of "this" was sticking to an existing pattern using regular function returning promises. Once we need to redo them into async/await we can investigate if there's a way around create-error's context prolbem
---
 core/server/models/base/plugins/crud.js       | 19 ++++++++++++++++++-
 .../regression/api/canary/admin/posts_spec.js |  8 ++++++++
 test/unit/models/base/crud_spec.js            |  2 --
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/core/server/models/base/plugins/crud.js b/core/server/models/base/plugins/crud.js
index faa6944d62..d8fba3729e 100644
--- a/core/server/models/base/plugins/crud.js
+++ b/core/server/models/base/plugins/crud.js
@@ -1,6 +1,12 @@
 const _ = require('lodash');
 const errors = require('@tryghost/errors');
 
+const tpl = require('@tryghost/tpl');
+
+const messages = {
+    couldNotUnderstandRequest: 'Could not understand request.'
+};
+
 /**
  * @param {Bookshelf} Bookshelf
  */
@@ -131,7 +137,18 @@ module.exports = function (Bookshelf) {
                 options.columns = _.intersection(options.columns, this.prototype.permittedAttributes());
             }
 
-            return model.fetch(options);
+            return 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 () {
diff --git a/test/unit/models/base/crud_spec.js b/test/unit/models/base/crud_spec.js
index 2a946409be..9faec2073d 100644
--- a/test/unit/models/base/crud_spec.js
+++ b/test/unit/models/base/crud_spec.js
@@ -89,8 +89,6 @@ describe('Models: crud', function () {
 
             const findOneReturnValue = models.Base.Model.findOne(data, unfilteredOptions);
 
-            should.equal(findOneReturnValue, fetchStub.returnValues[0]);
-
             return findOneReturnValue.then((result) => {
                 should.equal(result, fetchedModel);