From c7f2141b5b95f1db136219017cff5593af5d20a9 Mon Sep 17 00:00:00 2001
From: Scott Nonnenberg <scott@signal.org>
Date: Wed, 22 Jan 2025 13:55:57 -1000
Subject: [PATCH] parseContact: Be resilient to invalid phone numbers

---
 ts/test-both/util/timestamp_test.ts        |  2 +-
 ts/test-node/types/EmbeddedContact_test.ts | 44 ++++++++++++++++++++++
 ts/types/EmbeddedContact.ts                | 10 +++--
 ts/types/PhoneNumber.ts                    | 16 --------
 4 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/ts/test-both/util/timestamp_test.ts b/ts/test-both/util/timestamp_test.ts
index 0c7a1886ff..45dfa6a312 100644
--- a/ts/test-both/util/timestamp_test.ts
+++ b/ts/test-both/util/timestamp_test.ts
@@ -88,7 +88,7 @@ describe('timestamp', () => {
     });
 
     it('formats month name, day of month, year, and time for other times', () => {
-      const rx = /Apr 20, 2000, \d+:\d+ [A|P]M/;
+      const rx = /Apr (19|20|21), 2000, \d+:\d+ [A|P]M/;
       const datetime = formatDateTimeLong(i18n, new Date(956216013000));
       assert.isTrue(rx.test(datetime));
     });
diff --git a/ts/test-node/types/EmbeddedContact_test.ts b/ts/test-node/types/EmbeddedContact_test.ts
index 56222563fd..baa1235675 100644
--- a/ts/test-node/types/EmbeddedContact_test.ts
+++ b/ts/test-node/types/EmbeddedContact_test.ts
@@ -10,9 +10,11 @@ import type { MessageAttributesType } from '../../model-types.d';
 import type { Avatar, Email, Phone } from '../../types/EmbeddedContact';
 import {
   _validate,
+  ContactFormType,
   embeddedContactSelector,
   getName,
   parseAndWriteAvatar,
+  parsePhoneItem,
 } from '../../types/EmbeddedContact';
 import { fakeAttachment } from '../../test-both/helpers/fakeAttachment';
 import { generateAci } from '../../types/ServiceId';
@@ -632,6 +634,48 @@ describe('Contact', () => {
     });
   });
 
+  describe('parsePhoneItem', () => {
+    it('adds default phone type', () => {
+      const phone: Phone = {
+        value: '+18005550000',
+        // @ts-expect-error Forcing an invalid value here
+        type: null,
+      };
+      const expected = {
+        value: '+18005550000',
+        type: ContactFormType.HOME,
+      };
+      const actual = parsePhoneItem(phone, { regionCode: '805' });
+      assert.deepEqual(actual, expected);
+    });
+
+    it('passes invalid phone numbers through', () => {
+      const phone: Phone = {
+        value: '+1800555u000',
+        type: ContactFormType.WORK,
+      };
+      const expected = {
+        value: '+1800555u000',
+        type: ContactFormType.WORK,
+      };
+      const actual = parsePhoneItem(phone, { regionCode: '805' });
+      assert.deepEqual(actual, expected);
+    });
+
+    it('returns original data if regionCode not provided', () => {
+      const phone: Phone = {
+        value: '+18005550000',
+        type: ContactFormType.MOBILE,
+      };
+      const expected = {
+        value: '+18005550000',
+        type: ContactFormType.MOBILE,
+      };
+      const actual = parsePhoneItem(phone, { regionCode: undefined });
+      assert.deepEqual(actual, expected);
+    });
+  });
+
   describe('_validate', () => {
     it('returns error if contact has no name.displayName or organization', () => {
       const messageId = 'the-message-id';
diff --git a/ts/types/EmbeddedContact.ts b/ts/types/EmbeddedContact.ts
index a6612074a7..ccc5ef32f5 100644
--- a/ts/types/EmbeddedContact.ts
+++ b/ts/types/EmbeddedContact.ts
@@ -10,7 +10,7 @@ import type { ReadonlyMessageAttributesType } from '../model-types.d';
 import { isNotNil } from '../util/isNotNil';
 import {
   format as formatPhoneNumber,
-  parse as parsePhoneNumber,
+  normalize as normalizePhoneNumber,
 } from './PhoneNumber';
 import type {
   AttachmentType,
@@ -317,7 +317,7 @@ export function _validate(
   return undefined;
 }
 
-function parsePhoneItem(
+export function parsePhoneItem(
   item: Phone,
   { regionCode }: { regionCode: string | undefined }
 ): Phone | undefined {
@@ -325,10 +325,14 @@ function parsePhoneItem(
     return undefined;
   }
 
+  const value = regionCode
+    ? normalizePhoneNumber(item.value, { regionCode })
+    : item.value;
+
   return {
     ...item,
     type: item.type || DEFAULT_PHONE_TYPE,
-    value: parsePhoneNumber(item.value, { regionCode }),
+    value: value ?? item.value,
   };
 }
 
diff --git a/ts/types/PhoneNumber.ts b/ts/types/PhoneNumber.ts
index 97bf670196..4f0b017c87 100644
--- a/ts/types/PhoneNumber.ts
+++ b/ts/types/PhoneNumber.ts
@@ -71,22 +71,6 @@ export const format = memoizee(_format, {
   max: 5000,
 });
 
-export function parse(
-  phoneNumber: string,
-  options: {
-    regionCode: string | undefined;
-  }
-): string {
-  const { regionCode } = options;
-  const parsedNumber = instance.parse(phoneNumber, regionCode);
-
-  if (instance.isValidNumber(parsedNumber)) {
-    return instance.format(parsedNumber, PhoneNumberFormat.E164);
-  }
-
-  return phoneNumber;
-}
-
 export function normalize(
   phoneNumber: string,
   options: { regionCode: string }