Parse phone numbers into e164 as part of schema upgrade

js/modules/types/contact.js

@@ -2,6 +2,7 @@ const { omit, compact, map } = require('lodash');
 
 const { toLogFormat } = require('./errors');
 const { SignalService } = require('../../../ts/protobuf');
+const { parsePhoneNumber } = require('../../../ts/util/parsePhoneNumber');
 
 const DEFAULT_PHONE_TYPE = SignalService.DataMessage.Contact.Phone.Type.HOME;
 const DEFAULT_EMAIL_TYPE = SignalService.DataMessage.Contact.Email.Type.HOME;
@@ -12,7 +13,7 @@ exports.parseAndWriteAvatar = upgradeAttachment => async (
   contact,
   context = {}
 ) => {
-  const { message } = context;
+  const { message, regionCode } = context;
   const { avatar } = contact;
 
   // This is to ensure that an omit() call doesn't pull in prototype props/methods
@@ -28,7 +29,7 @@ exports.parseAndWriteAvatar = upgradeAttachment => async (
       : omit(contactShallowCopy, ['avatar']);
 
   // eliminates empty numbers, emails, and addresses; adds type if not provided
-  const parsedContact = parseContact(contactWithUpdatedAvatar);
+  const parsedContact = parseContact(contactWithUpdatedAvatar, { regionCode });
 
   const error = exports._validate(parsedContact, {
     messageId: idForLogging(message),
@@ -43,12 +44,17 @@ exports.parseAndWriteAvatar = upgradeAttachment => async (
   return parsedContact;
 };
 
-function parseContact(contact) {
+function parseContact(contact, options = {}) {
+  const { regionCode } = options;
+
+  const boundParsePhone = phoneNumber =>
+    parsePhoneItem(phoneNumber, { regionCode });
+
   return Object.assign(
     {},
     omit(contact, ['avatar', 'number', 'email', 'address']),
     parseAvatar(contact.avatar),
-    createArrayKey('number', compact(map(contact.number, parsePhoneItem))),
+    createArrayKey('number', compact(map(contact.number, boundParsePhone))),
     createArrayKey('email', compact(map(contact.email, parseEmailItem))),
     createArrayKey('address', compact(map(contact.address, parseAddress)))
   );
@@ -81,13 +87,16 @@ exports._validate = (contact, options = {}) => {
   return null;
 };
 
-function parsePhoneItem(item) {
+function parsePhoneItem(item, options = {}) {
+  const { regionCode } = options;
+
   if (!item.value) {
     return null;
   }
 
   return Object.assign({}, item, {
     type: item.type || DEFAULT_PHONE_TYPE,
+    value: parsePhoneNumber(item.value, { regionCode }),
   });
 }
 

js/modules/types/message.js

@@ -255,10 +255,16 @@ const VERSIONS = [
 exports.CURRENT_SCHEMA_VERSION = VERSIONS.length - 1;
 
 // UpgradeStep
-exports.upgradeSchema = async (rawMessage, { writeNewAttachmentData } = {}) => {
+exports.upgradeSchema = async (
+  rawMessage,
+  { writeNewAttachmentData, getRegionCode } = {}
+) => {
   if (!isFunction(writeNewAttachmentData)) {
     throw new TypeError('`context.writeNewAttachmentData` is required');
   }
+  if (!isFunction(getRegionCode)) {
+    throw new TypeError('`context.getRegionCode` is required');
+  }
 
   let message = rawMessage;
   // eslint-disable-next-line no-restricted-syntax
@@ -266,7 +272,10 @@ exports.upgradeSchema = async (rawMessage, { writeNewAttachmentData } = {}) => {
     // We really do want this intra-loop await because this is a chained async action,
     //   each step dependent on the previous
     // eslint-disable-next-line no-await-in-loop
-    message = await currentVersion(message, { writeNewAttachmentData });
+    message = await currentVersion(message, {
+      writeNewAttachmentData,
+      regionCode: getRegionCode(),
+    });
   }
 
   return message;

js/signal.js

@@ -50,7 +50,7 @@ const { IdleDetector } = require('./modules/idle_detector');
 const MessageDataMigrator = require('./modules/messages_data_migrator');
 
 exports.setup = (options = {}) => {
-  const { Attachments, userDataPath } = options;
+  const { Attachments, userDataPath, getRegionCode } = options;
 
   const Components = {
     ContactDetail,
@@ -84,6 +84,7 @@ exports.setup = (options = {}) => {
     upgradeMessageSchema: message =>
       Message.upgradeSchema(message, {
         writeNewAttachmentData: Attachments.createWriterForNew(attachmentsPath),
+        getRegionCode,
       }),
     writeMessageAttachments: Message.createAttachmentDataWriter(
       Attachments.createWriterForExisting(attachmentsPath)

preload.js

@@ -129,6 +129,7 @@ window.moment.locale(locale);
 window.Signal = Signal.setup({
   Attachments,
   userDataPath: app.getPath('userData'),
+  getRegionCode: () => window.storage.get('regionCode'),
 });
 
 // Pulling these in separately since they access filesystem, electron

test/modules/types/contact_test.js

@@ -36,6 +36,46 @@ describe('Contact', () => {
       assert.deepEqual(result, message.contact[0]);
     });
 
+    it('turns phone numbers to e164 format', async () => {
+      const upgradeAttachment = sinon
+        .stub()
+        .throws(new Error("Shouldn't be called"));
+      const upgradeVersion = Contact.parseAndWriteAvatar(upgradeAttachment);
+
+      const message = {
+        body: 'hey there!',
+        contact: [
+          {
+            name: {
+              displayName: 'Someone Somewhere',
+            },
+            number: [
+              {
+                type: 1,
+                value: '(202) 555-0099',
+              },
+            ],
+          },
+        ],
+      };
+      const expected = {
+        name: {
+          displayName: 'Someone Somewhere',
+        },
+        number: [
+          {
+            type: 1,
+            value: '+12025550099',
+          },
+        ],
+      };
+      const result = await upgradeVersion(message.contact[0], {
+        message,
+        regionCode: 'US',
+      });
+      assert.deepEqual(result, expected);
+    });
+
     it('removes contact avatar if it has no sub-avatar', async () => {
       const upgradeAttachment = sinon
         .stub()

test/modules/types/message_test.js

@@ -277,6 +277,7 @@ describe('Message', () => {
           assert.deepEqual(attachmentData, expectedAttachmentData);
           return 'abc/abcdefg';
         },
+        getRegionCode: () => 'US',
       };
       const actual = await Message.upgradeSchema(input, context);
       assert.deepEqual(actual, expected);

ts/styleguide/StyleGuideUtil.ts

@@ -137,6 +137,7 @@ const Attachments = {
 parent.Signal = Signal.setup({
   Attachments,
   userDataPath: '/',
+  getRegionCode: () => parent.storage.get('regionCode'),
 });
 parent.SignalService = SignalService;
 

ts/util/parsePhoneNumber.ts

@@ -0,0 +1,17 @@
+import { instance, PhoneNumberFormat } from './libphonenumberInstance';
+
+export function parsePhoneNumber(
+  phoneNumber: string,
+  options: {
+    regionCode: string;
+  }
+): string {
+  const { regionCode } = options;
+  const parsedNumber = instance.parse(phoneNumber, regionCode);
+
+  if (instance.isValidNumber(parsedNumber)) {
+    return instance.format(parsedNumber, PhoneNumberFormat.E164);
+  }
+
+  return phoneNumber;
+}