Handle null values in read sync jobs

ts/jobs/helpers/readAndViewSyncHelpers.ts

@@ -6,12 +6,54 @@ import type { LoggerType } from '../../logging/log';
 import { getSendOptions } from '../../util/getSendOptions';
 import { handleMessageSend, SendTypesType } from '../../util/handleMessageSend';
 import { isNotNil } from '../../util/isNotNil';
+import { strictAssert } from '../../util/assert';
+import { isRecord } from '../../util/isRecord';
 
 import { commonShouldJobContinue } from './commonShouldJobContinue';
 import { handleCommonJobRequestError } from './handleCommonJobRequestError';
 
 const CHUNK_SIZE = 100;
 
+export type SyncType = {
+  messageId?: string;
+  senderE164?: string;
+  senderUuid?: string;
+  timestamp: number;
+};
+
+/**
+ * Parse what _should_ be an array of `SyncType`s.
+ *
+ * Notably, `null`s made it into the job system and caused jobs to fail. This cleans that
+ * up in addition to validating the data.
+ */
+export function parseRawSyncDataArray(value: unknown): Array<SyncType> {
+  strictAssert(Array.isArray(value), 'syncs are not an array');
+  return value.map((item: unknown) => {
+    strictAssert(isRecord(item), 'sync is not an object');
+
+    const { messageId, senderE164, senderUuid, timestamp } = item;
+    strictAssert(typeof timestamp === 'number', 'timestamp should be a number');
+
+    return {
+      messageId: parseOptionalString('messageId', messageId),
+      senderE164: parseOptionalString('senderE164', senderE164),
+      senderUuid: parseOptionalString('senderUuid', senderUuid),
+      timestamp,
+    };
+  });
+}
+
+function parseOptionalString(name: string, value: unknown): undefined | string {
+  if (typeof value === 'string') {
+    return value;
+  }
+  if (value === undefined || value === null) {
+    return undefined;
+  }
+  throw new Error(`${name} was not a string`);
+}
+
 export async function runReadOrViewSyncJob({
   attempt,
   isView,
@@ -24,12 +66,7 @@ export async function runReadOrViewSyncJob({
   isView: boolean;
   log: LoggerType;
   maxRetryTime: number;
-  syncs: ReadonlyArray<{
-    messageId?: string;
-    senderE164?: string;
-    senderUuid?: string;
-    timestamp: number;
-  }>;
+  syncs: ReadonlyArray<SyncType>;
   timestamp: number;
 }>): Promise<void> {
   let sendType: SendTypesType;

ts/jobs/readSyncJobQueue.ts

@@ -3,33 +3,30 @@
 
 /* eslint-disable class-methods-use-this */
 
-import * as z from 'zod';
 import * as durations from '../util/durations';
 import type { LoggerType } from '../logging/log';
 import { exponentialBackoffMaxAttempts } from '../util/exponentialBackoff';
-import { runReadOrViewSyncJob } from './helpers/runReadOrViewSyncJob';
+import {
+  SyncType,
+  parseRawSyncDataArray,
+  runReadOrViewSyncJob,
+} from './helpers/readAndViewSyncHelpers';
+import { strictAssert } from '../util/assert';
+import { isRecord } from '../util/isRecord';
 
 import { JobQueue } from './JobQueue';
 import { jobQueueDatabaseStore } from './JobQueueDatabaseStore';
 
 const MAX_RETRY_TIME = durations.DAY;
 
-const readSyncJobDataSchema = z.object({
-  readSyncs: z.array(
-    z.object({
-      messageId: z.string().optional(),
-      senderE164: z.string().optional(),
-      senderUuid: z.string().optional(),
-      timestamp: z.number(),
-    })
-  ),
-});
-
-export type ReadSyncJobData = z.infer<typeof readSyncJobDataSchema>;
+export type ReadSyncJobData = {
+  readSyncs: Array<SyncType>;
+};
 
 export class ReadSyncJobQueue extends JobQueue<ReadSyncJobData> {
   protected parseData(data: unknown): ReadSyncJobData {
-    return readSyncJobDataSchema.parse(data);
+    strictAssert(isRecord(data), 'data is not an object');
+    return { readSyncs: parseRawSyncDataArray(data.readSyncs) };
   }
 
   protected async run(

ts/jobs/viewSyncJobQueue.ts

@@ -3,33 +3,30 @@
 
 /* eslint-disable class-methods-use-this */
 
-import * as z from 'zod';
 import * as durations from '../util/durations';
 import type { LoggerType } from '../logging/log';
 import { exponentialBackoffMaxAttempts } from '../util/exponentialBackoff';
-import { runReadOrViewSyncJob } from './helpers/runReadOrViewSyncJob';
+import {
+  SyncType,
+  parseRawSyncDataArray,
+  runReadOrViewSyncJob,
+} from './helpers/readAndViewSyncHelpers';
+import { strictAssert } from '../util/assert';
+import { isRecord } from '../util/isRecord';
 
 import { JobQueue } from './JobQueue';
 import { jobQueueDatabaseStore } from './JobQueueDatabaseStore';
 
 const MAX_RETRY_TIME = durations.DAY;
 
-const viewSyncJobDataSchema = z.object({
-  viewSyncs: z.array(
-    z.object({
-      messageId: z.string().optional(),
-      senderE164: z.string().optional(),
-      senderUuid: z.string().optional(),
-      timestamp: z.number(),
-    })
-  ),
-});
-
-export type ViewSyncJobData = z.infer<typeof viewSyncJobDataSchema>;
+export type ViewSyncJobData = {
+  viewSyncs: Array<SyncType>;
+};
 
 export class ViewSyncJobQueue extends JobQueue<ViewSyncJobData> {
   protected parseData(data: unknown): ViewSyncJobData {
-    return viewSyncJobDataSchema.parse(data);
+    strictAssert(isRecord(data), 'data is not an object');
+    return { viewSyncs: parseRawSyncDataArray(data.viewSyncs) };
   }
 
   protected async run(

ts/test-node/jobs/helpers/readAndViewSyncHelpers_test.ts

@@ -0,0 +1,102 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+
+import { parseRawSyncDataArray } from '../../../jobs/helpers/readAndViewSyncHelpers';
+
+describe('read and view sync helpers', () => {
+  describe('parseRawSyncDataArray', () => {
+    it('errors if not passed an array', () => {
+      [undefined, { timestamp: 123 }].forEach(input => {
+        assert.throws(() => parseRawSyncDataArray(input));
+      });
+    });
+
+    it('errors if passed an array with any invalid elements', () => {
+      const valid = {
+        messageId: '4a3ad1e1-61a7-464d-9982-f3e8eea81818',
+        senderUuid: '253ce806-7375-4227-82ed-eb8321630133',
+        timestamp: 1234,
+      };
+
+      [undefined, {}, { messageId: -1, timestamp: 4567 }].forEach(invalid => {
+        assert.throws(() => parseRawSyncDataArray([valid, invalid]));
+      });
+    });
+
+    it('does nothing to an empty array', () => {
+      assert.deepEqual(parseRawSyncDataArray([]), []);
+    });
+
+    it('handles a valid array', () => {
+      assert.deepEqual(
+        parseRawSyncDataArray([
+          {
+            senderUuid: 'd9e1e89b-f4a6-4c30-b3ec-8e7a964f94bd',
+            timestamp: 1234,
+          },
+          {
+            messageId: '4a3ad1e1-61a7-464d-9982-f3e8eea81818',
+            senderE164: undefined,
+            senderUuid: '253ce806-7375-4227-82ed-eb8321630133',
+            timestamp: 4567,
+          },
+        ]),
+        [
+          {
+            messageId: undefined,
+            senderE164: undefined,
+            senderUuid: 'd9e1e89b-f4a6-4c30-b3ec-8e7a964f94bd',
+            timestamp: 1234,
+          },
+          {
+            messageId: '4a3ad1e1-61a7-464d-9982-f3e8eea81818',
+            senderE164: undefined,
+            senderUuid: '253ce806-7375-4227-82ed-eb8321630133',
+            timestamp: 4567,
+          },
+        ]
+      );
+    });
+
+    it('turns `null` into `undefined`', () => {
+      assert.deepEqual(
+        parseRawSyncDataArray([
+          {
+            messageId: null,
+            senderUuid: 'd9e1e89b-f4a6-4c30-b3ec-8e7a964f94bd',
+            timestamp: 1234,
+          },
+        ]),
+        [
+          {
+            messageId: undefined,
+            senderE164: undefined,
+            senderUuid: 'd9e1e89b-f4a6-4c30-b3ec-8e7a964f94bd',
+            timestamp: 1234,
+          },
+        ]
+      );
+    });
+
+    it('removes extra properties', () => {
+      assert.deepEqual(
+        parseRawSyncDataArray([
+          {
+            timestamp: 1234,
+            extra: true,
+          },
+        ]),
+        [
+          {
+            messageId: undefined,
+            senderE164: undefined,
+            senderUuid: undefined,
+            timestamp: 1234,
+          },
+        ]
+      );
+    });
+  });
+});