fix: A request with a malformed pointer and a beforeSave trigger hangs until timeout

spec/CloudCode.spec.js

@@ -153,6 +153,27 @@ describe('Cloud Code', () => {
     );
   });
 
+  it('does not hang when a beforeSave object has a malformed pointer (#7490)', async () => {
+    Parse.Cloud.beforeSave('Stuff', () => {});
+
+    // A pointer with an empty objectId cannot be serialised, which used to throw
+    // inside the trigger response handler and leave the request hanging forever.
+    // The SDK rejects such a pointer client-side, so send it as a raw REST body.
+    const response = await request({
+      method: 'POST',
+      url: 'http://localhost:8378/1/classes/Stuff',
+      headers: {
+        'X-Parse-Application-Id': 'test',
+        'X-Parse-Master-Key': 'test',
+        'Content-Type': 'application/json',
+      },
+      body: JSON.stringify({ Owner: { __type: 'Pointer', className: '_User', objectId: '' } }),
+    }).catch(e => e);
+
+    expect(response.status).toBe(400);
+    expect(response.data.code).toBe(Parse.Error.SCRIPT_FAILED);
+  });
+
   it('returns an error', done => {
     Parse.Cloud.define('cloudCodeWithError', () => {
       /* eslint-disable no-undef */

src/triggers.js

@@ -551,7 +551,12 @@ export function maybeRunAfterFindTrigger(
         }
         return responseFromTrigger;
       })
-      .then(success, error);
+      .then(success, error)
+      // See maybeRunTrigger: a throw in the success/error path must reject rather
+      // than leave this promise unsettled and hang the request (#7490).
+      .catch(e =>
+        reject(resolveError(e, { code: Parse.Error.SCRIPT_FAILED, message: 'Script failed.' }))
+      );
   }).then(resultsAsJSON => {
     logTriggerAfterHook(
       triggerType,
@@ -917,7 +922,7 @@ async function builtInTriggerValidator(options, request, auth) {
 // Resolves to an object, empty or containing an object key. A beforeSave
 // trigger will set the object key to the rest format object to save.
 // originalParseObject is optional, we only need that for before/afterSave functions
-export function maybeRunTrigger(
+export async function maybeRunTrigger(
   triggerType,
   auth,
   parseObject,
@@ -926,100 +931,110 @@ export function maybeRunTrigger(
   context
 ) {
   if (!parseObject) {
-    return Promise.resolve({});
+    return {};
   }
-  return new Promise(function (resolve, reject) {
-    var trigger = getTrigger(parseObject.className, triggerType, config.applicationId);
-    if (!trigger) { return resolve(); }
-    var request = getRequestObject(
-      triggerType,
-      auth,
-      parseObject,
-      originalParseObject,
-      config,
-      context
-    );
-    var { success, error } = getResponseObject(
-      request,
-      object => {
-        logTriggerSuccessBeforeHook(
-          triggerType,
-          parseObject.className,
-          parseObject.toJSON(),
-          object,
-          auth,
-          triggerType.startsWith('after')
-            ? config.logLevels.triggerAfter
-            : config.logLevels.triggerBeforeSuccess
-        );
-        if (
-          triggerType === Types.beforeSave ||
-          triggerType === Types.afterSave ||
-          triggerType === Types.beforeDelete ||
-          triggerType === Types.afterDelete
-        ) {
-          Object.assign(context, request.context);
-        }
-        resolve(object);
-      },
-      error => {
-        logTriggerErrorBeforeHook(
+  const trigger = getTrigger(parseObject.className, triggerType, config.applicationId);
+  if (!trigger) {
+    return;
+  }
+  const request = getRequestObject(
+    triggerType,
+    auth,
+    parseObject,
+    originalParseObject,
+    config,
+    context
+  );
+
+  try {
+    await maybeRunValidator(request, `${triggerType}.${parseObject.className}`, auth);
+
+    // AfterSave and afterDelete triggers can return a promise, which is awaited
+    // here so trigger execution is synced with the RestWrite.execute() call. If
+    // they do not return a promise, they run async code parallel to it.
+    let triggerResult;
+    if (request.skipWithMasterKey) {
+      triggerResult = undefined;
+    } else {
+      const returned = trigger(request);
+      if (
+        triggerType === Types.afterSave ||
+        triggerType === Types.afterDelete ||
+        triggerType === Types.afterLogin
+      ) {
+        logTriggerAfterHook(
           triggerType,
           parseObject.className,
           parseObject.toJSON(),
           auth,
-          error,
-          config.logLevels.triggerBeforeError
+          config.logLevels.triggerAfter
         );
-        reject(error);
       }
-    );
+      if (triggerType === Types.beforeSave) {
+        // beforeSave is expected to return null (nothing); only a returned object
+        // with an `object` key (e.g. from an express before hook) is honoured.
+        const response =
+          returned && typeof returned.then === 'function' ? await returned : null;
+        triggerResult = response && response.object ? response : null;
+      } else {
+        triggerResult = await returned;
+      }
+    }
 
-    // AfterSave and afterDelete triggers can return a promise, which if they
-    // do, needs to be resolved before this promise is resolved,
-    // so trigger execution is synced with RestWrite.execute() call.
-    // If triggers do not return a promise, they can run async code parallel
-    // to the RestWrite.execute() call.
-    return Promise.resolve()
-      .then(() => {
-        return maybeRunValidator(request, `${triggerType}.${parseObject.className}`, auth);
-      })
-      .then(() => {
-        if (request.skipWithMasterKey) {
-          return Promise.resolve();
-        }
-        const promise = trigger(request);
-        if (
-          triggerType === Types.afterSave ||
-          triggerType === Types.afterDelete ||
-          triggerType === Types.afterLogin
-        ) {
-          logTriggerAfterHook(
-            triggerType,
-            parseObject.className,
-            parseObject.toJSON(),
-            auth,
-            config.logLevels.triggerAfter
-          );
-        }
-        // beforeSave is expected to return null (nothing)
-        if (triggerType === Types.beforeSave) {
-          if (promise && typeof promise.then === 'function') {
-            return promise.then(response => {
-              // response.object may come from express routing before hook
-              if (response && response.object) {
-                return response;
-              }
-              return null;
-            });
-          }
-          return null;
-        }
+    // getResponseObject shapes the trigger's return value into the REST response.
+    // Serialising an object with a malformed pointer can throw here; awaiting the
+    // wrapped promise lets that reject cleanly instead of hanging forever (#7490).
+    const response = await new Promise((resolve, reject) => {
+      const { success, error } = getResponseObject(request, resolve, reject);
+      try {
+        success(triggerResult);
+      } catch (e) {
+        error(e);
+      }
+    });
 
-        return promise;
-      })
-      .then(success, error);
-  });
+    logTriggerSuccessBeforeHook(
+      triggerType,
+      parseObject.className,
+      parseObject.toJSON(),
+      response,
+      auth,
+      triggerType.startsWith('after')
+        ? config.logLevels.triggerAfter
+        : config.logLevels.triggerBeforeSuccess
+    );
+    if (
+      triggerType === Types.beforeSave ||
+      triggerType === Types.afterSave ||
+      triggerType === Types.beforeDelete ||
+      triggerType === Types.afterDelete
+    ) {
+      Object.assign(context, request.context);
+    }
+    return response;
+  } catch (e) {
+    const resolvedError = resolveError(e, {
+      code: Parse.Error.SCRIPT_FAILED,
+      message: 'Script failed. Unknown error.',
+    });
+    let objectForLog;
+    try {
+      objectForLog = parseObject.toJSON();
+    } catch {
+      // A malformed object (e.g. a pointer with an empty objectId) cannot be
+      // serialised; log without it rather than mask the original error (#7490).
+      objectForLog = { className: parseObject.className };
+    }
+    logTriggerErrorBeforeHook(
+      triggerType,
+      parseObject.className,
+      objectForLog,
+      auth,
+      resolvedError,
+      config.logLevels.triggerBeforeError
+    );
+    throw resolvedError;
+  }
 }
 
 // Converts a REST-format object to a Parse.Object