Compile protobufs with no-convert/null-defaults

Co-authored-by: Scott Nonnenberg <scott@signal.org>

package.json

@@ -28,7 +28,7 @@
     "get-expire-time": "node ts/scripts/get-expire-time.js",
     "copy-components": "node ts/scripts/copy.js",
     "sass": "sass stylesheets/manifest.scss:stylesheets/manifest.css stylesheets/manifest_bridge.scss:stylesheets/manifest_bridge.css",
-    "build-module-protobuf": "pbjs --target static-module --force-long --no-typeurl --no-verify --no-create --wrap commonjs --out ts/protobuf/compiled.js protos/*.proto && pbts --out ts/protobuf/compiled.d.ts ts/protobuf/compiled.js",
+    "build-module-protobuf": "pbjs --target static-module --force-long --no-typeurl --no-verify --no-create --no-convert --null-defaults --wrap commonjs --out ts/protobuf/compiled.js protos/*.proto && pbts --out ts/protobuf/compiled.d.ts ts/protobuf/compiled.js",
     "clean-module-protobuf": "rm -f ts/protobuf/compiled.d.ts ts/protobuf/compiled.js",
     "build-protobuf": "yarn build-module-protobuf",
     "clean-protobuf": "yarn clean-module-protobuf",

ts/groups.ts

@@ -1751,7 +1751,7 @@ export async function fetchMembershipProof({
     secretParams,
     request: (sender, options) => sender.getGroupMembershipToken(options),
   });
-  return response.token;
+  return dropNull(response.token);
 }
 
 // Creating a group
@@ -3454,9 +3454,13 @@ async function getGroupUpdates({
     if (isChangeSupported) {
       if (!wrappedGroupChange.isTrusted) {
         strictAssert(
-          groupChange.serverSignature && groupChange.actions,
+          groupChange.serverSignature,
           'Server signature must be present in untrusted group change'
         );
+        strictAssert(
+          groupChange.actions,
+          'Actions must be present in untrusted group change'
+        );
         try {
           verifyNotarySignature(
             serverPublicParamsBase64,
@@ -3613,10 +3617,10 @@ async function updateGroupViaPreJoinInfo({
   const newAttributes: ConversationAttributesType = {
     ...group,
     description: decryptGroupDescription(
-      preJoinInfo.descriptionBytes,
+      dropNull(preJoinInfo.descriptionBytes),
       secretParams
     ),
-    name: decryptGroupTitle(preJoinInfo.title, secretParams),
+    name: decryptGroupTitle(dropNull(preJoinInfo.title), secretParams),
     left: true,
     members: group.members || [],
     pendingMembersV2: group.pendingMembersV2 || [],
@@ -3626,7 +3630,7 @@ async function updateGroupViaPreJoinInfo({
         timestamp: Date.now(),
       },
     ],
-    revision: preJoinInfo.version,
+    revision: dropNull(preJoinInfo.version),
 
     temporaryMemberCount: preJoinInfo.memberCount || 1,
   };
@@ -3863,7 +3867,7 @@ async function generateLeftGroupChanges(
         masterKey
       );
 
-      revision = preJoinInfo.version;
+      revision = dropNull(preJoinInfo.version);
     }
   } catch (error) {
     log.warn(

ts/groups/joinViaLink.ts

@@ -29,6 +29,7 @@ import { isAccessControlEnabled } from './util';
 import { isGroupV1 } from '../util/whatTypeOfConversation';
 import { longRunningTaskWrapper } from '../util/longRunningTaskWrapper';
 import { sleep } from '../util/sleep';
+import { dropNull } from '../util/dropNull';
 
 export async function joinViaLink(value: string): Promise<void> {
   let inviteLinkPassword: string;
@@ -116,7 +117,7 @@ export async function joinViaLink(value: string): Promise<void> {
     return;
   }
 
-  if (!isAccessControlEnabled(result.addFromInviteLink)) {
+  if (!isAccessControlEnabled(dropNull(result.addFromInviteLink))) {
     log.error(
       `joinViaLink/${logId}: addFromInviteLink value of ${result.addFromInviteLink} is invalid`
     );
@@ -138,10 +139,10 @@ export async function joinViaLink(value: string): Promise<void> {
     result.addFromInviteLink ===
     Proto.AccessControl.AccessRequired.ADMINISTRATOR;
   const title =
-    decryptGroupTitle(result.title, secretParams) ||
+    decryptGroupTitle(dropNull(result.title), secretParams) ||
     window.i18n('icu:unknownGroup');
   const groupDescription = decryptGroupDescription(
-    result.descriptionBytes,
+    dropNull(result.descriptionBytes),
     secretParams
   );
 
@@ -316,7 +317,7 @@ export async function joinViaLink(value: string): Promise<void> {
                 groupInviteLinkPassword: inviteLinkPassword,
                 left: true,
                 name: title,
-                revision: result.version,
+                revision: dropNull(result.version),
                 temporaryMemberCount: memberCount,
                 timestamp,
               });

ts/services/LinkPreview.ts

@@ -518,8 +518,10 @@ async function getGroupPreview(
   }
 
   const title =
-    window.Signal.Groups.decryptGroupTitle(result.title, secretParams) ||
+    window.Signal.Groups.decryptGroupTitle(
-    window.i18n('icu:unknownGroup');
+      dropNull(result.title),
+      secretParams
+    ) || window.i18n('icu:unknownGroup');
   const description = window.i18n('icu:GroupV2--join--group-metadata--full', {
     memberCount: result?.memberCount ?? 0,
   });

ts/test-both/util/sessionTranslation_test.ts

@@ -4,6 +4,7 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
 import { assert } from 'chai';
+import Long from 'long';
 
 import * as Bytes from '../../Bytes';
 import type { LocalUserDataType } from '../../util/sessionTranslation';
@@ -11,6 +12,38 @@ import { sessionRecordToProtobuf } from '../../util/sessionTranslation';
 
 const getRecordCopy = (record: any): any => JSON.parse(JSON.stringify(record));
 
+function protoToJSON(value: unknown): unknown {
+  if (value == null) {
+    return value;
+  }
+
+  if (typeof value === 'string' || typeof value === 'number') {
+    return value;
+  }
+
+  if (Buffer.isBuffer(value) || value instanceof Uint8Array) {
+    return Buffer.from(value).toString('base64');
+  }
+
+  if (Array.isArray(value)) {
+    return value.map(protoToJSON);
+  }
+
+  if (Long.isLong(value)) {
+    return value.toNumber();
+  }
+
+  if (typeof value === 'object') {
+    const res: Record<string, unknown> = {};
+    for (const key of Object.keys(value)) {
+      res[key] = protoToJSON((value as Record<string, unknown>)[key]);
+    }
+    return res;
+  }
+
+  return value;
+}
+
 describe('sessionTranslation', () => {
   let ourData: LocalUserDataType;
 
@@ -93,6 +126,7 @@ describe('sessionTranslation', () => {
             index: 0,
             key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
           },
+          messageKeys: [],
         },
         receiverChains: [
           {
@@ -121,13 +155,14 @@ describe('sessionTranslation', () => {
         localRegistrationId: 3554,
         aliceBaseKey: 'BVeHv5MAbMgKeaoO/G1CMBdqhC7bo7Mtc4EWxI0oT19N',
       },
+      previousSessions: [],
     };
 
     const recordCopy = getRecordCopy(record);
 
     const actual = sessionRecordToProtobuf(record, ourData);
 
-    assert.deepEqual(expected, actual.toJSON());
+    assert.deepEqual(expected, protoToJSON(actual));
 
     // We want to ensure that conversion doesn't modify incoming data
     assert.deepEqual(record, recordCopy);
@@ -330,6 +365,7 @@ describe('sessionTranslation', () => {
             index: 0,
             key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
           },
+          messageKeys: [],
         },
         receiverChains: [
           {
@@ -436,12 +472,14 @@ describe('sessionTranslation', () => {
             chainKey: {
               index: 3,
             },
+            messageKeys: [],
           },
           {
             senderRatchetKey: 'BRRAnr1NhizgCPPzmYV9qGBpvwCpSQH0Rx+UOtl78wUg',
             chainKey: {
               index: 1,
             },
+            messageKeys: [],
           },
           {
             senderRatchetKey: 'BZvOKPA+kXiCg8TIP/52fu1reCDirC7wb5nyRGce3y4N',
@@ -488,31 +526,35 @@ describe('sessionTranslation', () => {
             chainKey: {
               index: 2,
             },
+            messageKeys: [],
           },
           {
             senderRatchetKey: 'BV7ECvKbwKIAD61BXDYr0xr3JtckuKzR1Hw8cVPWGtlo',
             chainKey: {
               index: 3,
             },
+            messageKeys: [],
           },
           {
             senderRatchetKey: 'BTC7rQqoykGR5Aaix7RkAhI5fSXufc6pVGN9OIC8EW5c',
             chainKey: {
               index: 1,
             },
+            messageKeys: [],
           },
         ],
         remoteRegistrationId: 4243,
         localRegistrationId: 3554,
         aliceBaseKey: 'BVeHv5MAbMgKeaoO/G1CMBdqhC7bo7Mtc4EWxI0oT19N',
       },
+      previousSessions: [],
     };
 
     const recordCopy = getRecordCopy(record);
 
     const actual = sessionRecordToProtobuf(record, ourData);
 
-    assert.deepEqual(expected, actual.toJSON());
+    assert.deepEqual(expected, protoToJSON(actual));
 
     // We want to ensure that conversion doesn't modify incoming data
     assert.deepEqual(record, recordCopy);
@@ -585,6 +627,7 @@ describe('sessionTranslation', () => {
             index: 0,
             key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
           },
+          messageKeys: [],
         },
         receiverChains: [
           {
@@ -618,13 +661,14 @@ describe('sessionTranslation', () => {
         localRegistrationId: 3554,
         aliceBaseKey: 'BVeHv5MAbMgKeaoO/G1CMBdqhC7bo7Mtc4EWxI0oT19N',
       },
+      previousSessions: [],
     };
 
     const recordCopy = getRecordCopy(record);
 
     const actual = sessionRecordToProtobuf(record, ourData);
 
-    assert.deepEqual(expected, actual.toJSON());
+    assert.deepEqual(expected, protoToJSON(actual));
 
     // We want to ensure that conversion doesn't modify incoming data
     assert.deepEqual(record, recordCopy);
@@ -772,6 +816,7 @@ describe('sessionTranslation', () => {
             index: 0,
             key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
           },
+          messageKeys: [],
         },
         receiverChains: [
           {
@@ -815,6 +860,7 @@ describe('sessionTranslation', () => {
               index: 0,
               key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
             },
+            messageKeys: [],
           },
           receiverChains: [
             {
@@ -857,6 +903,7 @@ describe('sessionTranslation', () => {
               index: 0,
               key: '6EI/Nw+vHhCoB499OpM/kLkQJFzrfqoAZ00w1ZgnowU=',
             },
+            messageKeys: [],
           },
           receiverChains: [
             {
@@ -892,7 +939,7 @@ describe('sessionTranslation', () => {
 
     const actual = sessionRecordToProtobuf(record, ourData);
 
-    assert.deepEqual(expected, actual.toJSON());
+    assert.deepEqual(expected, protoToJSON(actual));
 
     // We want to ensure that conversion doesn't modify incoming data
     assert.deepEqual(record, recordCopy);
@@ -950,6 +997,7 @@ describe('sessionTranslation', () => {
           signedPreKeyId: 2995,
         },
         previousCounter: 1,
+        receiverChains: [],
         remoteIdentityPublic: 'BRmB2uSNpwbXZJjisIh1p/VgRctUZSVIoiEm2ThjiHoq',
         remoteRegistrationId: 3188,
         rootKey: 'GzGfNozK5vDKqL4+fdqpiMRIuHNOndM6iMhGubNR1mk=',
@@ -958,19 +1006,21 @@ describe('sessionTranslation', () => {
             index: 1,
             key: 'tl5Eby9q7n8PVeiriKoRjHhu9Y0RxvJ90PMq5MfKwgA=',
           },
+          messageKeys: [],
           senderRatchetKey: 'BRSm55wC8hrG5Rp7l9gxtOhugp5ulcco20upOFCPyyJo',
           senderRatchetKeyPrivate:
             'IC0mCV0kFVAf+Q4cHid5hR7vy+5F0SvpYYaqsSA6d00=',
         },
         sessionVersion: 3,
       },
+      previousSessions: [],
     };
 
     const recordCopy = getRecordCopy(record);
 
     const actual = sessionRecordToProtobuf(record, ourData);
 
-    assert.deepEqual(expected, actual.toJSON());
+    assert.deepEqual(expected, protoToJSON(actual));
 
     // We want to ensure that conversion doesn't modify incoming data
     assert.deepEqual(record, recordCopy);

ts/test-mock/messaging/unknown_contact_test.ts

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type { PrimaryDevice } from '@signalapp/mock-server';
+import { Proto } from '@signalapp/mock-server';
 import createDebug from 'debug';
 import Long from 'long';
 import type { Page } from 'playwright';
@@ -48,6 +49,8 @@ describe('unknown contacts', function (this: Mocha.Suite) {
       callingMessage: {
         offer: {
           callId: new Long(Math.floor(Math.random() * 1e10)),
+          type: Proto.CallingMessage.Offer.Type.OFFER_AUDIO_CALL,
+          opaque: new Uint8Array(0),
         },
       },
     });

ts/textsecure/AccountManager.ts

@@ -50,7 +50,7 @@ import {
 import { normalizeAci } from '../util/normalizeAci';
 import { isMoreRecentThan, isOlderThan } from '../util/timestamp';
 import { ourProfileKeyService } from '../services/ourProfileKey';
-import { assertDev, strictAssert } from '../util/assert';
+import { strictAssert } from '../util/assert';
 import { getRegionCodeForNumber } from '../util/libphonenumberUtil';
 import { isNotNil } from '../util/isNotNil';
 import { missingCaseError } from '../util/missingCaseError';
@@ -258,12 +258,21 @@ export default class AccountManager extends EventTarget {
 
     const bytes = Bytes.fromBase64(base64);
     const proto = Proto.DeviceName.decode(bytes);
-    assertDev(
+    strictAssert(
-      proto.ephemeralPublic && proto.syntheticIv && proto.ciphertext,
+      proto.ephemeralPublic,
-      'Missing required fields in DeviceName'
+      'Missing ephemeralPublic field in DeviceName'
     );
+    strictAssert(proto.syntheticIv, 'Missing syntheticIv field in DeviceName');
+    strictAssert(proto.ciphertext, 'Missing ciphertext field in DeviceName');
 
-    const name = decryptDeviceName(proto, identityKey.privKey);
+    const name = decryptDeviceName(
+      {
+        ephemeralPublic: proto.ephemeralPublic,
+        syntheticIv: proto.syntheticIv,
+        ciphertext: proto.ciphertext,
+      },
+      identityKey.privKey
+    );
 
     return name;
   }

ts/textsecure/MessageReceiver.ts

@@ -398,7 +398,7 @@ export default class MessageReceiver
 
       try {
         const decoded = Proto.Envelope.decode(plaintext);
-        const serverTimestamp = decoded.serverTimestamp?.toNumber();
+        const serverTimestamp = decoded.serverTimestamp?.toNumber() ?? 0;
 
         const ourAci = this.storage.user.getCheckedAci();
 
@@ -412,14 +412,14 @@ export default class MessageReceiver
           messageAgeSec: this.calculateMessageAge(headers, serverTimestamp),
 
           // Proto.Envelope fields
-          type: decoded.type,
+          type: decoded.type ?? Proto.Envelope.Type.UNKNOWN,
           sourceServiceId: decoded.sourceServiceId
             ? normalizeServiceId(
                 decoded.sourceServiceId,
                 'MessageReceiver.handleRequest.sourceServiceId'
               )
             : undefined,
-          sourceDevice: decoded.sourceDevice,
+          sourceDevice: decoded.sourceDevice ?? 1,
           destinationServiceId: decoded.destinationServiceId
             ? normalizeServiceId(
                 decoded.destinationServiceId,
@@ -433,12 +433,12 @@ export default class MessageReceiver
                   'MessageReceiver.handleRequest.updatedPni'
                 )
               : undefined,
-          timestamp: decoded.timestamp?.toNumber(),
+          timestamp: decoded.timestamp?.toNumber() ?? 0,
           content: dropNull(decoded.content),
-          serverGuid: decoded.serverGuid,
+          serverGuid: decoded.serverGuid ?? getGuid(),
           serverTimestamp,
           urgent: isBoolean(decoded.urgent) ? decoded.urgent : true,
-          story: decoded.story,
+          story: decoded.story ?? false,
           reportingToken: decoded.reportingToken?.length
             ? decoded.reportingToken
             : undefined,
@@ -873,7 +873,7 @@ export default class MessageReceiver
         messageAgeSec: item.messageAgeSec || 0,
 
         // Proto.Envelope fields
-        type: decoded.type,
+        type: decoded.type ?? Proto.Envelope.Type.UNKNOWN,
         source: item.source,
         sourceServiceId: normalizeServiceId(
           item.sourceServiceId || decoded.sourceServiceId,
@@ -890,11 +890,11 @@ export default class MessageReceiver
               'CachedEnvelope.updatedPni'
             )
           : undefined,
-        timestamp: decoded.timestamp?.toNumber(),
+        timestamp: decoded.timestamp?.toNumber() ?? 0,
         content: dropNull(decoded.content),
-        serverGuid: decoded.serverGuid,
+        serverGuid: decoded.serverGuid ?? getGuid(),
         serverTimestamp:
-          item.serverTimestamp || decoded.serverTimestamp?.toNumber(),
+          item.serverTimestamp || decoded.serverTimestamp?.toNumber() || 0,
         urgent: isBoolean(item.urgent) ? item.urgent : true,
         story: Boolean(item.story),
         reportingToken: item.reportingToken

ts/textsecure/ProvisioningCipher.ts

@@ -13,6 +13,7 @@ import {
 import { calculateAgreement, createKeyPair, generateKeyPair } from '../Curve';
 import { SignalService as Proto } from '../protobuf';
 import { strictAssert } from '../util/assert';
+import { dropNull } from '../util/dropNull';
 
 type ProvisionDecryptResult = {
   aciKeyPair: KeyPairType;
@@ -34,9 +35,10 @@ class ProvisioningCipherInner {
     provisionEnvelope: Proto.ProvisionEnvelope
   ): Promise<ProvisionDecryptResult> {
     strictAssert(
-      provisionEnvelope.publicKey && provisionEnvelope.body,
+      provisionEnvelope.publicKey,
-      'Missing required fields in ProvisionEnvelope'
+      'Missing publicKey in ProvisionEnvelope'
     );
+    strictAssert(provisionEnvelope.body, 'Missing body in ProvisionEnvelope');
     const masterEphemeral = provisionEnvelope.publicKey;
     const message = provisionEnvelope.body;
     if (new Uint8Array(message)[0] !== 1) {
@@ -78,12 +80,12 @@ class ProvisioningCipherInner {
     const ret: ProvisionDecryptResult = {
       aciKeyPair,
       pniKeyPair,
-      number: provisionMessage.number,
+      number: dropNull(provisionMessage.number),
       aci,
       untaggedPni: pni,
-      provisioningCode: provisionMessage.provisioningCode,
+      provisioningCode: dropNull(provisionMessage.provisioningCode),
-      userAgent: provisionMessage.userAgent,
+      userAgent: dropNull(provisionMessage.userAgent),
-      readReceipts: provisionMessage.readReceipts,
+      readReceipts: provisionMessage.readReceipts ?? false,
     };
     if (Bytes.isNotEmpty(provisionMessage.profileKey)) {
       ret.profileKey = provisionMessage.profileKey;

ts/textsecure/WebAPI.ts

@@ -3390,7 +3390,7 @@ export function initialize({
         return getGroupLog(
           {
             ...options,
-            startVersion: joinedAtVersion,
+            startVersion: joinedAtVersion ?? 0,
           },
           credentials
         );

ts/textsecure/cds/CDSSocketBase.ts

@@ -203,6 +203,9 @@ function decodeSingleResponse(
   resultMap: Map<string, CDSResponseEntryType>,
   response: Proto.CDSClientResponse
 ): void {
+  if (!response.e164PniAciTriples) {
+    return;
+  }
   for (
     let i = 0;
     i < response.e164PniAciTriples.length;

ts/types/Stickers.ts

@@ -1,7 +1,7 @@
 // Copyright 2019 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { isNumber, pick, reject, groupBy, values } from 'lodash';
+import { isNumber, reject, groupBy, values } from 'lodash';
 import pMap from 'p-map';
 import Queue from 'p-queue';
 
@@ -472,7 +472,8 @@ export async function downloadEphemeralPack(
       coverStickerId,
       stickerCount,
       status: 'ephemeral' as const,
-      ...pick(proto, ['title', 'author']),
+      title: proto.title ?? '',
+      author: proto.author ?? '',
     };
     stickerPackAdded(pack);
 
@@ -691,7 +692,8 @@ async function doDownloadStickerPack(
       createdAt: Date.now(),
       stickers: {},
       storageNeedsSync: !fromStorageService,
-      ...pick(proto, ['title', 'author']),
+      title: proto.title ?? '',
+      author: proto.author ?? '',
     };
     await Data.createOrUpdateStickerPack(pack);
     stickerPackAdded(pack);