diff --git a/js/modules/types/contact.js b/js/modules/types/contact.js index 12d45bc7e..a9c8e235f 100644 --- a/js/modules/types/contact.js +++ b/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 }), }); } diff --git a/js/modules/types/message.js b/js/modules/types/message.js index fd339cb6f..ee1ed87db 100644 --- a/js/modules/types/message.js +++ b/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; diff --git a/js/signal.js b/js/signal.js index 468ca32b2..e5b62f875 100644 --- a/js/signal.js +++ b/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) diff --git a/preload.js b/preload.js index a225f9eae..b884864db 100644 --- a/preload.js +++ b/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 diff --git a/test/modules/types/contact_test.js b/test/modules/types/contact_test.js index 5d20274ce..2a8627839 100644 --- a/test/modules/types/contact_test.js +++ b/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() diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index 61535c569..614c6e62d 100644 --- a/test/modules/types/message_test.js +++ b/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); diff --git a/ts/styleguide/StyleGuideUtil.ts b/ts/styleguide/StyleGuideUtil.ts index 432de2cbe..fdc9d628a 100644 --- a/ts/styleguide/StyleGuideUtil.ts +++ b/ts/styleguide/StyleGuideUtil.ts @@ -137,6 +137,7 @@ const Attachments = { parent.Signal = Signal.setup({ Attachments, userDataPath: '/', + getRegionCode: () => parent.storage.get('regionCode'), }); parent.SignalService = SignalService; diff --git a/ts/util/parsePhoneNumber.ts b/ts/util/parsePhoneNumber.ts new file mode 100644 index 000000000..ef80d28b2 --- /dev/null +++ b/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; +}