fix: Server crash when afterFind trigger returns a non-array value

spec/CloudCode.spec.js

@@ -2345,6 +2345,27 @@ describe('cloud functions', () => {
 });
 
 describe('beforeSave hooks', () => {
+  it('should reject with the trigger error when the object cannot be serialized for logging', async () => {
+    Parse.Cloud.beforeSave('MyObject', req => {
+      const circular = {};
+      circular.self = circular;
+      req.object.set('circular', circular);
+      throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'blocked');
+    });
+    await expectAsync(new Parse.Object('MyObject').save()).toBeRejectedWith(
+      new Parse.Error(Parse.Error.SCRIPT_FAILED, 'blocked')
+    );
+  });
+
+  it('should settle when a successful trigger leaves an object that cannot be serialized', async () => {
+    Parse.Cloud.beforeSave('MyObject', req => {
+      const circular = {};
+      circular.self = circular;
+      req.object.set('circular', circular);
+    });
+    await expectAsync(new Parse.Object('MyObject').save()).toBeRejected();
+  });
+
   it('should have request headers', done => {
     Parse.Cloud.beforeSave('MyObject', req => {
       expect(req.headers).toBeDefined();
@@ -2977,6 +2998,44 @@ describe('beforeFind hooks', () => {
 });
 
 describe('afterFind hooks', () => {
+  const invalidAfterFindError = new Parse.Error(
+    Parse.Error.SCRIPT_FAILED,
+    'afterFind trigger must return an array of objects.'
+  );
+
+  for (const [type, value] of [
+    ['an object', { foo: 'bar' }],
+    ['a string', 'invalid'],
+    ['a number', 1],
+  ]) {
+    it(`should reject a find when afterFind returns ${type}`, async () => {
+      Parse.Cloud.afterFind('MyObject', () => value);
+      await new Parse.Object('MyObject').save();
+      await expectAsync(new Parse.Query('MyObject').find()).toBeRejectedWith(
+        invalidAfterFindError
+      );
+    });
+  }
+
+  it('should reject a get when afterFind returns a non-array', async () => {
+    Parse.Cloud.afterFind('MyObject', () => ({ foo: 'bar' }));
+    const obj = await new Parse.Object('MyObject').save();
+    await expectAsync(new Parse.Query('MyObject').get(obj.id)).toBeRejectedWith(
+      invalidAfterFindError
+    );
+  });
+
+  for (const value of [null, undefined]) {
+    it(`should return the original objects when afterFind returns ${value}`, async () => {
+      Parse.Cloud.afterFind('MyObject', () => value);
+      const obj = await new Parse.Object('MyObject').save({ key: 'value' });
+      const results = await new Parse.Query('MyObject').find();
+      expect(results.length).toBe(1);
+      expect(results[0].id).toBe(obj.id);
+      expect(results[0].get('key')).toBe('value');
+    });
+  }
+
   it('should add afterFind trigger', done => {
     Parse.Cloud.afterFind('MyObject', req => {
       const q = req.query;

src/triggers.js

@@ -379,6 +379,14 @@ export function getResponseObject(request, resolve, reject) {
         if (!response) {
           response = request.objects;
         }
+        if (!Array.isArray(response)) {
+          return reject(
+            new Parse.Error(
+              Parse.Error.SCRIPT_FAILED,
+              'afterFind trigger must return an array of objects.'
+            )
+          );
+        }
         response = response.map(object => {
           return toJSONwithObjects(object);
         });
@@ -455,6 +463,16 @@ function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth
   );
 }
 
+// Logging must not prevent a trigger promise from settling, e.g. when the
+// object cannot be serialized because a trigger set a circular attribute.
+function logSafely(log) {
+  try {
+    log();
+  } catch (e) {
+    logger.error('Failed to log trigger result', { error: e });
+  }
+}
+
 function logTriggerErrorBeforeHook(triggerType, className, input, auth, error, logLevel) {
   if (logLevel === 'silent') {
     return;
@@ -553,7 +571,8 @@ export function maybeRunAfterFindTrigger(
         }
         return responseFromTrigger;
       })
-      .then(success, error);
+      .then(success, error)
+      .catch(error);
   }).then(resultsAsJSON => {
     logTriggerAfterHook(
       triggerType,
@@ -950,15 +969,17 @@ export function maybeRunTrigger(
     var { success, error } = getResponseObject(
       request,
       object => {
-        logTriggerSuccessBeforeHook(
-          triggerType,
-          parseObject.className,
-          parseObject.toJSON(),
-          object,
-          auth,
-          triggerType.startsWith('after')
-            ? config.logLevels.triggerAfter
-            : config.logLevels.triggerBeforeSuccess
+        logSafely(() =>
+          logTriggerSuccessBeforeHook(
+            triggerType,
+            parseObject.className,
+            parseObject.toJSON(),
+            object,
+            auth,
+            triggerType.startsWith('after')
+              ? config.logLevels.triggerAfter
+              : config.logLevels.triggerBeforeSuccess
+          )
         );
         if (
           triggerType === Types.beforeSave ||
@@ -971,13 +992,15 @@ export function maybeRunTrigger(
         resolve(object);
       },
       error => {
-        logTriggerErrorBeforeHook(
-          triggerType,
-          parseObject.className,
-          parseObject.toJSON(),
-          auth,
-          error,
-          config.logLevels.triggerBeforeError
+        logSafely(() =>
+          logTriggerErrorBeforeHook(
+            triggerType,
+            parseObject.className,
+            parseObject.toJSON(),
+            auth,
+            error,
+            config.logLevels.triggerBeforeError
+          )
         );
         reject(error);
       }
@@ -1026,7 +1049,8 @@ export function maybeRunTrigger(
 
         return promise;
       })
-      .then(success, error);
+      .then(success, error)
+      .catch(error);
   });
 }