From 01fd927070852085ebb0014785592318f6edb431 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 08:55:25 +1000 Subject: [PATCH 01/10] match envelope type with ios protos --- protos/SignalService.proto | 4 ++-- ts/receiver/closedGroups.ts | 2 +- ts/receiver/contentMessage.ts | 6 +++--- ts/session/crypto/MessageEncrypter.ts | 8 ++++---- ts/session/sending/MessageSender.ts | 2 +- .../session/unit/crypto/MessageEncrypter_test.ts | 10 ++++------ .../session/unit/sending/MessageSender_test.ts | 16 ++++++++-------- ts/test/test-utils/utils/envelope.ts | 4 ++-- 8 files changed, 25 insertions(+), 27 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 72459280b..187cff541 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -7,8 +7,8 @@ option java_outer_classname = "SignalServiceProtos"; message Envelope { enum Type { - UNIDENTIFIED_SENDER = 6; - CLOSED_GROUP_CIPHERTEXT = 7; + SESSION_MESSAGE = 6; + CLOSED_GROUP_MESSAGE = 7; } // @required diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 9c942a979..a7f2468f6 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -65,7 +65,7 @@ export async function handleClosedGroupControlMessage( // We drop New closed group message from our other devices, as they will come as ConfigurationMessage instead if (type === Type.ENCRYPTION_KEY_PAIR) { const isComingFromGroupPubkey = - envelope.type === SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT; + envelope.type === SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE; await handleClosedGroupEncryptionKeyPair(envelope, groupUpdate, isComingFromGroupPubkey); return; } diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 5937fd0bc..c2c73ecbb 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -228,10 +228,10 @@ async function doDecrypt( } switch (envelope.type) { - // Only UNIDENTIFIED_SENDER and CLOSED_GROUP_CIPHERTEXT are supported - case SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT: + // Only SESSION_MESSAGE and CLOSED_GROUP_MESSAGE are supported + case SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE: return decryptForClosedGroup(envelope, ciphertext); - case SignalService.Envelope.Type.UNIDENTIFIED_SENDER: { + case SignalService.Envelope.Type.SESSION_MESSAGE: { return decryptUnidentifiedSender(envelope, ciphertext); } default: diff --git a/ts/session/crypto/MessageEncrypter.ts b/ts/session/crypto/MessageEncrypter.ts index 8c5c15b80..958fea91d 100644 --- a/ts/session/crypto/MessageEncrypter.ts +++ b/ts/session/crypto/MessageEncrypter.ts @@ -26,7 +26,7 @@ export async function encrypt( plainTextBuffer: Uint8Array, encryptionType: EncryptionType ): Promise { - const { CLOSED_GROUP_CIPHERTEXT, UNIDENTIFIED_SENDER } = SignalService.Envelope.Type; + const { CLOSED_GROUP_MESSAGE, SESSION_MESSAGE } = SignalService.Envelope.Type; if (encryptionType !== EncryptionType.ClosedGroup && encryptionType !== EncryptionType.Fallback) { throw new Error(`Invalid encryption type:${encryptionType}`); } @@ -35,7 +35,7 @@ export async function encrypt( if (encryptForClosedGroup) { // window?.log?.info( - // 'Encrypting message with SessionProtocol and envelope type is CLOSED_GROUP_CIPHERTEXT' + // 'Encrypting message with SessionProtocol and envelope type is CLOSED_GROUP_MESSAGE' // ); const hexEncryptionKeyPair = await getLatestClosedGroupEncryptionKeyPair(device.key); if (!hexEncryptionKeyPair) { @@ -52,13 +52,13 @@ export async function encrypt( ); return { - envelopeType: CLOSED_GROUP_CIPHERTEXT, + envelopeType: CLOSED_GROUP_MESSAGE, cipherText: cipherTextClosedGroup, }; } const cipherText = await exports.encryptUsingSessionProtocol(device, plainText); - return { envelopeType: UNIDENTIFIED_SENDER, cipherText }; + return { envelopeType: SESSION_MESSAGE, cipherText }; } export async function encryptUsingSessionProtocol( diff --git a/ts/session/sending/MessageSender.ts b/ts/session/sending/MessageSender.ts index 6dd77bdaf..200f44dfe 100644 --- a/ts/session/sending/MessageSender.ts +++ b/ts/session/sending/MessageSender.ts @@ -59,7 +59,7 @@ async function buildEnvelope( ): Promise { let source: string | undefined; - if (type === SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT) { + if (type === SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE) { source = sskSource; } diff --git a/ts/test/session/unit/crypto/MessageEncrypter_test.ts b/ts/test/session/unit/crypto/MessageEncrypter_test.ts index 73400c67e..7de5e5490 100644 --- a/ts/test/session/unit/crypto/MessageEncrypter_test.ts +++ b/ts/test/session/unit/crypto/MessageEncrypter_test.ts @@ -125,7 +125,7 @@ describe('MessageEncrypter', () => { describe('EncryptionType', () => { describe('ClosedGroup', () => { - it('should return a CLOSED_GROUP_CIPHERTEXT envelope type for ClosedGroup', async () => { + it('should return a CLOSED_GROUP_MESSAGE envelope type for ClosedGroup', async () => { const hexKeyPair = { publicHex: `05${ourUserEd25516Keypair.pubKey}`, privateHex: '0123456789abcdef', @@ -142,10 +142,10 @@ describe('MessageEncrypter', () => { ); chai .expect(result.envelopeType) - .to.deep.equal(SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT); + .to.deep.equal(SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE); }); - it('should return a UNIDENTIFIED_SENDER envelope type for Fallback', async () => { + it('should return a SESSION_MESSAGE envelope type for Fallback', async () => { const data = crypto.randomBytes(10); const result = await MessageEncrypter.encrypt( @@ -153,9 +153,7 @@ describe('MessageEncrypter', () => { data, EncryptionType.Fallback ); - chai - .expect(result.envelopeType) - .to.deep.equal(SignalService.Envelope.Type.UNIDENTIFIED_SENDER); + chai.expect(result.envelopeType).to.deep.equal(SignalService.Envelope.Type.SESSION_MESSAGE); }); it('should throw an error for anything else than Fallback or ClosedGroup', () => { diff --git a/ts/test/session/unit/sending/MessageSender_test.ts b/ts/test/session/unit/sending/MessageSender_test.ts index c156c1e07..f48560f7d 100644 --- a/ts/test/session/unit/sending/MessageSender_test.ts +++ b/ts/test/session/unit/sending/MessageSender_test.ts @@ -29,7 +29,7 @@ describe('MessageSender', () => { lokiMessageAPISendStub = sandbox.stub(LokiMessageApi, 'sendMessage').resolves(); encryptStub = sandbox.stub(MessageEncrypter, 'encrypt').resolves({ - envelopeType: SignalService.Envelope.Type.UNIDENTIFIED_SENDER, + envelopeType: SignalService.Envelope.Type.SESSION_MESSAGE, cipherText: crypto.randomBytes(10), }); @@ -77,7 +77,7 @@ describe('MessageSender', () => { }); describe('logic', () => { - let messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.UNIDENTIFIED_SENDER; + let messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.SESSION_MESSAGE; beforeEach(() => { encryptStub.callsFake(async (_device, plainTextBuffer, _type) => ({ @@ -107,7 +107,7 @@ describe('MessageSender', () => { }); it('should correctly build the envelope', async () => { - messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.UNIDENTIFIED_SENDER; + messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.SESSION_MESSAGE; // This test assumes the encryption stub returns the plainText passed into it. const device = TestUtils.generateFakePubKey().key; @@ -137,15 +137,15 @@ describe('MessageSender', () => { const envelope = SignalService.Envelope.decode( webSocketMessage.request?.body as Uint8Array ); - expect(envelope.type).to.equal(SignalService.Envelope.Type.UNIDENTIFIED_SENDER); + expect(envelope.type).to.equal(SignalService.Envelope.Type.SESSION_MESSAGE); expect(envelope.source).to.equal(''); expect(toNumber(envelope.timestamp)).to.equal(timestamp); expect(envelope.content).to.deep.equal(plainTextBuffer); }); - describe('UNIDENTIFIED_SENDER', () => { + describe('SESSION_MESSAGE', () => { it('should set the envelope source to be empty', async () => { - messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.UNIDENTIFIED_SENDER; + messageEncyrptReturnEnvelopeType = SignalService.Envelope.Type.SESSION_MESSAGE; // This test assumes the encryption stub returns the plainText passed into it. const device = TestUtils.generateFakePubKey().key; @@ -175,10 +175,10 @@ describe('MessageSender', () => { const envelope = SignalService.Envelope.decode( webSocketMessage.request?.body as Uint8Array ); - expect(envelope.type).to.equal(SignalService.Envelope.Type.UNIDENTIFIED_SENDER); + expect(envelope.type).to.equal(SignalService.Envelope.Type.SESSION_MESSAGE); expect(envelope.source).to.equal( '', - 'envelope source should be empty in UNIDENTIFIED_SENDER' + 'envelope source should be empty in SESSION_MESSAGE' ); }); }); diff --git a/ts/test/test-utils/utils/envelope.ts b/ts/test/test-utils/utils/envelope.ts index 393729e47..fff07c88f 100644 --- a/ts/test/test-utils/utils/envelope.ts +++ b/ts/test/test-utils/utils/envelope.ts @@ -10,7 +10,7 @@ export function generateEnvelopePlusClosedGroup(groupId: string, sender: string) receivedAt: Date.now(), timestamp: Date.now() - 2000, id: uuid(), - type: SignalService.Envelope.Type.CLOSED_GROUP_CIPHERTEXT, + type: SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE, source: groupId, content: new Uint8Array(), toJSON: () => ['fake'], @@ -24,7 +24,7 @@ export function generateEnvelopePlus(sender: string): EnvelopePlus { receivedAt: Date.now(), timestamp: Date.now() - 2000, id: uuid(), - type: SignalService.Envelope.Type.UNIDENTIFIED_SENDER, + type: SignalService.Envelope.Type.SESSION_MESSAGE, source: sender, senderIdentity: sender, content: new Uint8Array(), From f32919985da3aa1e1c0a19085c0c1fef1057522c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 08:57:54 +1000 Subject: [PATCH 02/10] remove Contact in proto (unused) --- protos/SignalService.proto | 71 -------------------------------------- 1 file changed, 71 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 187cff541..786e4d30c 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -82,77 +82,6 @@ message DataMessage { repeated QuotedAttachment attachments = 4; } - message Contact { - - message Name { - optional string givenName = 1; - optional string familyName = 2; - optional string prefix = 3; - optional string suffix = 4; - optional string middleName = 5; - optional string displayName = 6; - } - - message Phone { - - enum Type { - HOME = 1; - MOBILE = 2; - WORK = 3; - CUSTOM = 4; - } - - optional string value = 1; - optional Type type = 2; - optional string label = 3; - } - - message Email { - - enum Type { - HOME = 1; - MOBILE = 2; - WORK = 3; - CUSTOM = 4; - } - - optional string value = 1; - optional Type type = 2; - optional string label = 3; - } - - message PostalAddress { - - enum Type { - HOME = 1; - WORK = 2; - CUSTOM = 3; - } - - optional Type type = 1; - optional string label = 2; - optional string street = 3; - optional string pobox = 4; - optional string neighborhood = 5; - optional string city = 6; - optional string region = 7; - optional string postcode = 8; - optional string country = 9; - } - - message Avatar { - optional AttachmentPointer avatar = 1; - optional bool isProfile = 2; - } - - optional Name name = 1; - repeated Phone number = 3; - repeated Email email = 4; - repeated PostalAddress address = 5; - optional Avatar avatar = 6; - optional string organization = 7; - } - message Preview { // @required optional string url = 1; From c0907829191543507567eeb44e67bf73db99ccfe Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 09:00:54 +1000 Subject: [PATCH 03/10] remove UPDATE type of closed group control message --- protos/SignalService.proto | 1 - ts/receiver/closedGroups.ts | 6 ------ 2 files changed, 7 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 786e4d30c..88e861568 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -99,7 +99,6 @@ message DataMessage { enum Type { NEW = 1; // publicKey, name, encryptionKeyPair, members, admins, expireTimer - UPDATE = 2; // name, members ENCRYPTION_KEY_PAIR = 3; // publicKey, wrappers NAME_CHANGE = 4; // name MEMBERS_ADDED = 5; // members diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index a7f2468f6..aa42254d5 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -56,12 +56,6 @@ export async function handleClosedGroupControlMessage( return; } - if (type === Type.UPDATE) { - window?.log?.error('ClosedGroup: Got a non explicit group update. dropping it ', type); - await removeFromCache(envelope); - return; - } - // We drop New closed group message from our other devices, as they will come as ConfigurationMessage instead if (type === Type.ENCRYPTION_KEY_PAIR) { const isComingFromGroupPubkey = From 173b49723baa594d246c10d820d18da985257a3d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 09:08:20 +1000 Subject: [PATCH 04/10] rename groupInvitation in proto and remove more stuff unused --- protos/SignalService.proto | 47 ++----------------- ts/receiver/closedGroups.ts | 2 - ts/receiver/contentMessage.ts | 3 +- ts/receiver/dataMessage.ts | 5 +- .../visibleMessage/GroupInvitationMessage.ts | 8 ++-- .../messages/GroupInvitationMessage_test.ts | 4 +- 6 files changed, 14 insertions(+), 55 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 88e861568..a1f83950c 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -94,6 +94,10 @@ message DataMessage { optional string profilePicture = 2; } + message OpenGroupInvitation { + optional string url = 1; + optional string name = 3; + } message ClosedGroupControlMessage { @@ -127,10 +131,6 @@ message DataMessage { optional uint32 expireTimer = 8; } - message GroupInvitation { - optional string serverAddress = 1; - optional string serverName = 3; - } optional string body = 1; repeated AttachmentPointer attachments = 2; @@ -140,10 +140,9 @@ message DataMessage { optional bytes profileKey = 6; optional uint64 timestamp = 7; optional Quote quote = 8; - repeated Contact contact = 9; repeated Preview preview = 10; optional LokiProfile profile = 101; - optional GroupInvitation groupInvitation = 102; + optional OpenGroupInvitation openGroupInvitation = 102; optional ClosedGroupControlMessage closedGroupControlMessage = 104; optional string syncTarget = 105; } @@ -226,39 +225,3 @@ message GroupContext { repeated string admins = 6; } -message ContactDetails { - - message Avatar { - optional string contentType = 1; - optional uint32 length = 2; - } - - // @required - optional string number = 1; - optional string name = 2; - optional Avatar avatar = 3; - optional string color = 4; - optional bytes profileKey = 6; - optional bool blocked = 7; - optional uint32 expireTimer = 8; - optional string nickname = 101; -} - -message GroupDetails { - - message Avatar { - optional string contentType = 1; - optional uint32 length = 2; - } - - // @required - optional bytes id = 1; - optional string name = 2; - repeated string members = 3; - optional Avatar avatar = 4; - optional bool active = 5 [default = true]; - optional uint32 expireTimer = 6; - optional string color = 7; - optional bool blocked = 8; - repeated string admins = 9; -} diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index aa42254d5..a90ded05b 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -186,8 +186,6 @@ export async function handleNewClosedGroup( await removeFromCache(envelope); return; } - // FIXME maybe we should handle an expiretimer here too? And on ClosedGroup updates? - const maybeConvo = ConversationController.getInstance().get(groupId); const expireTimer = groupUpdate.expireTimer; diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index c2c73ecbb..a1f669008 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -297,10 +297,9 @@ function shouldDropBlockedUserMessage(content: SignalService.Content): boolean { const data = content.dataMessage; const isControlDataMessageOnly = !data.body && - !data.contact?.length && !data.preview?.length && !data.attachments?.length && - !data.groupInvitation && + !data.openGroupInvitation && !data.quote; return !isControlDataMessageOnly; diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index 7bb4f6830..062ce4615 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -249,7 +249,7 @@ export async function processDecrypted( } export function isMessageEmpty(message: SignalService.DataMessage) { - const { flags, body, attachments, group, quote, contact, preview, groupInvitation } = message; + const { flags, body, attachments, group, quote, preview, openGroupInvitation } = message; return ( !flags && @@ -258,9 +258,8 @@ export function isMessageEmpty(message: SignalService.DataMessage) { _.isEmpty(attachments) && _.isEmpty(group) && _.isEmpty(quote) && - _.isEmpty(contact) && _.isEmpty(preview) && - _.isEmpty(groupInvitation) + _.isEmpty(openGroupInvitation) ); } diff --git a/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts b/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts index 2ff3bef36..1140fe72f 100644 --- a/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts @@ -24,13 +24,13 @@ export class GroupInvitationMessage extends DataMessage { } public dataProto(): SignalService.DataMessage { - const groupInvitation = new SignalService.DataMessage.GroupInvitation({ - serverAddress: this.serverAddress, - serverName: this.serverName, + const openGroupInvitation = new SignalService.DataMessage.OpenGroupInvitation({ + url: this.serverAddress, + name: this.serverName, }); return new SignalService.DataMessage({ - groupInvitation, + openGroupInvitation, expireTimer: this.expireTimer, }); } diff --git a/ts/test/session/unit/messages/GroupInvitationMessage_test.ts b/ts/test/session/unit/messages/GroupInvitationMessage_test.ts index 13c83c963..fb4689180 100644 --- a/ts/test/session/unit/messages/GroupInvitationMessage_test.ts +++ b/ts/test/session/unit/messages/GroupInvitationMessage_test.ts @@ -23,8 +23,8 @@ describe('GroupInvitationMessage', () => { const plainText = message.plainTextBuffer(); const decoded = SignalService.Content.decode(plainText); - expect(decoded.dataMessage?.groupInvitation).to.have.property('serverAddress', serverAddress); - expect(decoded.dataMessage?.groupInvitation).to.have.property('serverName', serverName); + expect(decoded.dataMessage?.openGroupInvitation).to.have.property('url', serverAddress); + expect(decoded.dataMessage?.openGroupInvitation).to.have.property('name', serverName); }); it('correct ttl', () => { From c1225b3a74dfdb116067b8fca9761e77fbb67362 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 09:58:10 +1000 Subject: [PATCH 05/10] made some timestamp required in the protobuf --- protos/SignalService.proto | 15 ++--- ts/opengroup/opengroupV2/ApiUtil.ts | 2 +- .../outgoing/visibleMessage/VisibleMessage.ts | 16 +++-- ts/session/utils/Attachments.ts | 51 +++++++++------- ts/session/utils/AttachmentsV2.ts | 60 ++++++++----------- 5 files changed, 73 insertions(+), 71 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index a1f83950c..dac96f3ba 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -1,8 +1,5 @@ -// Source: https://github.com/signalapp/libsignal-service-java/blob/4684a49b2ed8f32be619e0d0eea423626b6cb2cb/protobuf/SignalService.proto package signalservice; -option java_package = "org.whispersystems.signalservice.internal.push"; -option java_outer_classname = "SignalServiceProtos"; message Envelope { @@ -15,7 +12,7 @@ message Envelope { required Type type = 1; optional string source = 2; // @required - optional uint64 timestamp = 5; + required uint64 timestamp = 5; optional bytes content = 8; } @@ -25,9 +22,9 @@ message TypingMessage { STOPPED = 1; } // @required - optional uint64 timestamp = 1; + required uint64 timestamp = 1; // @required - optional Action action = 2; + required Action action = 2; } @@ -75,16 +72,16 @@ message DataMessage { } // @required - optional uint64 id = 1; + required uint64 id = 1; // @required - optional string author = 2; + required string author = 2; optional string text = 3; repeated QuotedAttachment attachments = 4; } message Preview { // @required - optional string url = 1; + required string url = 1; optional string title = 2; optional AttachmentPointer image = 3; } diff --git a/ts/opengroup/opengroupV2/ApiUtil.ts b/ts/opengroup/opengroupV2/ApiUtil.ts index 301741321..4672447fe 100644 --- a/ts/opengroup/opengroupV2/ApiUtil.ts +++ b/ts/opengroup/opengroupV2/ApiUtil.ts @@ -56,7 +56,7 @@ export const parseMessages = async ( const opengroupv2Message = OpenGroupMessageV2.fromJson(r); if ( !opengroupv2Message?.serverId || - !opengroupv2Message.sentTimestamp || + !opengroupv2Message.sentTimestamp || // this is our serverTimestamp !opengroupv2Message.base64EncodedData || !opengroupv2Message.base64EncodedSignature ) { diff --git a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts index e77a0e069..e90fae4b4 100644 --- a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts @@ -5,10 +5,9 @@ import { DataMessage } from '..'; import { Constants } from '../../..'; import { SignalService } from '../../../../protobuf'; import { LokiProfile } from '../../../../types/Message'; -import { ExpirationTimerUpdateMessage } from '../controlMessage/ExpirationTimerUpdateMessage'; import { MessageParams } from '../Message'; -export interface AttachmentPointer { +interface AttachmentPointerCommon { id?: number; contentType?: string; key?: Uint8Array; @@ -20,11 +19,18 @@ export interface AttachmentPointer { width?: number; height?: number; caption?: string; +} + +export interface AttachmentPointer extends AttachmentPointerCommon { url?: string; } +export interface AttachmentPointerWithUrl extends AttachmentPointerCommon { + url: string; +} + export interface Preview { - url?: string; + url: string; title?: string; image?: AttachmentPointer; } @@ -36,8 +42,8 @@ export interface QuotedAttachment { } export interface Quote { - id?: number; - author?: string; + id: number; + author: string; text?: string; attachments?: Array; } diff --git a/ts/session/utils/Attachments.ts b/ts/session/utils/Attachments.ts index acbaf27a0..c9751ad0b 100644 --- a/ts/session/utils/Attachments.ts +++ b/ts/session/utils/Attachments.ts @@ -3,12 +3,14 @@ import { Attachment } from '../../types/Attachment'; import { AttachmentPointer, + AttachmentPointerWithUrl, Preview, Quote, QuotedAttachment, } from '../messages/outgoing/visibleMessage/VisibleMessage'; import { FSv2 } from '../../fileserver'; import { addAttachmentPadding } from '../crypto/BufferPadding'; +import _ from 'lodash'; interface UploadParams { attachment: Attachment; @@ -17,21 +19,21 @@ interface UploadParams { shouldPad?: boolean; } -interface RawPreview { +export interface RawPreview { url?: string; title?: string; image: Attachment; } -interface RawQuoteAttachment { +export interface RawQuoteAttachment { contentType?: string; fileName?: string; thumbnail?: Attachment; } -interface RawQuote { - id?: number; - author?: string; +export interface RawQuote { + id: number; + author: string; text?: string; attachments?: Array; } @@ -40,7 +42,7 @@ interface RawQuote { export class AttachmentFsV2Utils { private constructor() {} - public static async uploadToFsV2(params: UploadParams): Promise { + public static async uploadToFsV2(params: UploadParams): Promise { const { attachment, isRaw = false, shouldPad = false } = params; if (typeof attachment !== 'object' || attachment == null) { throw new Error('Invalid attachment passed.'); @@ -84,15 +86,17 @@ export class AttachmentFsV2Utils { if (FSv2.useFileServerAPIV2Sending) { const uploadToV2Result = await FSv2.uploadFileToFsV2(attachmentData); if (uploadToV2Result) { - pointer.id = uploadToV2Result.fileId; - pointer.url = uploadToV2Result.fileUrl; - } else { - window?.log?.warn('upload to file server v2 failed'); + const pointerWithUrl: AttachmentPointerWithUrl = { + ...pointer, + id: uploadToV2Result.fileId, + url: uploadToV2Result.fileUrl, + }; + return pointerWithUrl; } - return pointer; - } else { - throw new Error('Only v2 fileserver upload is supported'); + window?.log?.warn('upload to file server v2 failed'); + throw new Error(`upload to file server v2 of ${attachment.fileName} failed`); } + throw new Error('Only v2 fileserver upload is supported'); } public static async uploadAttachmentsToFsV2( @@ -111,19 +115,22 @@ export class AttachmentFsV2Utils { public static async uploadLinkPreviewsToFsV2( previews: Array ): Promise> { - const promises = (previews || []).map(async item => { + const promises = (previews || []).map(async preview => { // some links does not have an image associated, and it makes the whole message fail to send - if (!item.image) { - return item; + if (!preview.image) { + window.log.warn('tried to upload file to fsv2 without image.. skipping'); + return undefined; } + const image = await this.uploadToFsV2({ + attachment: preview.image, + }); return { - ...item, - image: await this.uploadToFsV2({ - attachment: item.image, - }), - }; + ...preview, + image, + url: preview.url || image.url, + } as Preview; }); - return Promise.all(promises); + return _.compact(await Promise.all(promises)); } public static async uploadQuoteThumbnailsToFsV2(quote?: RawQuote): Promise { diff --git a/ts/session/utils/AttachmentsV2.ts b/ts/session/utils/AttachmentsV2.ts index 477aa0ebf..abd13c9ce 100644 --- a/ts/session/utils/AttachmentsV2.ts +++ b/ts/session/utils/AttachmentsV2.ts @@ -1,41 +1,24 @@ -import * as crypto from 'crypto'; import { Attachment } from '../../types/Attachment'; import { OpenGroupRequestCommonType } from '../../opengroup/opengroupV2/ApiUtil'; import { AttachmentPointer, + AttachmentPointerWithUrl, Preview, Quote, QuotedAttachment, } from '../messages/outgoing/visibleMessage/VisibleMessage'; import { uploadFileOpenGroupV2 } from '../../opengroup/opengroupV2/OpenGroupAPIV2'; import { addAttachmentPadding } from '../crypto/BufferPadding'; +import { RawPreview, RawQuote } from './Attachments'; +import _ from 'lodash'; interface UploadParamsV2 { attachment: Attachment; openGroup: OpenGroupRequestCommonType; } -interface RawPreview { - url?: string; - title?: string; - image: Attachment; -} - -interface RawQuoteAttachment { - contentType?: string; - fileName?: string; - thumbnail?: Attachment; -} - -interface RawQuote { - id?: number; - author?: string; - text?: string; - attachments?: Array; -} - -export async function uploadV2(params: UploadParamsV2): Promise { +export async function uploadV2(params: UploadParamsV2): Promise { const { attachment, openGroup } = params; if (typeof attachment !== 'object' || attachment == null) { throw new Error('Invalid attachment passed.'); @@ -62,10 +45,15 @@ export async function uploadV2(params: UploadParamsV2): Promise, openGroup: OpenGroupRequestCommonType ): Promise> { - const promises = (previews || []).map(async item => { + const promises = (previews || []).map(async preview => { // some links does not have an image associated, and it makes the whole message fail to send - if (!item.image) { - return item; + if (!preview.image) { + window.log.warn('tried to upload file to opengroupv2 without image.. skipping'); + + return undefined; } + const image = await exports.uploadV2({ + attachment: preview.image, + openGroup, + }); return { - ...item, - image: await exports.uploadV2({ - attachment: item.image, - openGroup, - }), + ...preview, + image, + url: preview.url || (image.url as string), }; }); - return Promise.all(promises); + return _.compact(await Promise.all(promises)); } export async function uploadQuoteThumbnailsV2( @@ -121,7 +113,7 @@ export async function uploadQuoteThumbnailsV2( return { ...attachment, thumbnail, - } as QuotedAttachment; + }; }); const attachments = await Promise.all(promises); From 03fe67b97410d259f88672794e7dd6393e43ac4c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 09:58:30 +1000 Subject: [PATCH 06/10] remove unused contact field on message --- js/modules/types/contact.js | 98 ------------------------------------- js/modules/types/message.js | 2 +- protos/SignalService.proto | 6 ++- ts/models/message.ts | 4 -- ts/models/messageType.ts | 1 - ts/receiver/attachments.ts | 38 -------------- ts/receiver/queuedJob.ts | 1 - 7 files changed, 5 insertions(+), 145 deletions(-) delete mode 100644 js/modules/types/contact.js diff --git a/js/modules/types/contact.js b/js/modules/types/contact.js deleted file mode 100644 index 2eda53ead..000000000 --- a/js/modules/types/contact.js +++ /dev/null @@ -1,98 +0,0 @@ -const { omit, compact, map } = require('lodash'); - -const { toLogFormat } = require('./errors'); -const { SignalService } = require('../../../ts/protobuf'); - -const DEFAULT_PHONE_TYPE = SignalService.DataMessage.Contact.Phone.Type.HOME; - -exports.parseAndWriteAvatar = upgradeAttachment => async (contact, context = {}) => { - const { message, logger } = context; - const { avatar } = contact; - - // This is to ensure that an omit() call doesn't pull in prototype props/methods - const contactShallowCopy = Object.assign({}, contact); - - const contactWithUpdatedAvatar = - avatar && avatar.avatar - ? Object.assign({}, contactShallowCopy, { - avatar: Object.assign({}, avatar, { - avatar: await upgradeAttachment(avatar.avatar, context), - }), - }) - : omit(contactShallowCopy, ['avatar']); - - // eliminates empty numbers, emails, and addresses; adds type if not provided - const parsedContact = parseContact(contactWithUpdatedAvatar); - - const error = exports._validate(parsedContact, { - messageId: idForLogging(message), - }); - if (error) { - logger.error('Contact.parseAndWriteAvatar: contact was malformed.', toLogFormat(error)); - } - - return parsedContact; -}; - -function parseContact(contact) { - const boundParsePhone = phoneNumber => parsePhoneItem(phoneNumber); - - return Object.assign( - {}, - omit(contact, ['avatar', 'number', 'email', 'address']), - parseAvatar(contact.avatar), - createArrayKey('number', compact(map(contact.number, boundParsePhone))) - ); -} - -function idForLogging(message) { - return `${message.source}.${message.sourceDevice} ${message.sent_at}`; -} - -exports._validate = (contact, options = {}) => { - const { messageId } = options; - const { name, number, organization } = contact; - - if ((!name || !name.displayName) && !organization) { - return new Error(`Message ${messageId}: Contact had neither 'displayName' nor 'organization'`); - } - - if (!number || !number.length) { - return new Error(`Message ${messageId}: Contact had no included numbers`); - } - - return null; -}; - -function parsePhoneItem(item) { - if (!item.value) { - return null; - } - - return Object.assign({}, item, { - type: item.type || DEFAULT_PHONE_TYPE, - value: item.value, - }); -} - -function parseAvatar(avatar) { - if (!avatar) { - return null; - } - - return { - avatar: Object.assign({}, avatar, { - isProfile: avatar.isProfile || false, - }), - }; -} - -function createArrayKey(key, array) { - if (!array || !array.length) { - return null; - } - - return { - [key]: array, - }; -} diff --git a/js/modules/types/message.js b/js/modules/types/message.js index d06cc7e89..44968644d 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -255,7 +255,7 @@ const toVersion5 = exports._withSchemaVersion({ }); const toVersion6 = exports._withSchemaVersion({ schemaVersion: 6, - upgrade: exports._mapContact(Contact.parseAndWriteAvatar(Attachment.migrateDataToFileSystem)), + upgrade: () => {}, }); // IMPORTANT: We’ve updated our definition of `initializeAttachmentMetadata`, so // we need to run it again on existing items that have previously been incorrectly diff --git a/protos/SignalService.proto b/protos/SignalService.proto index dac96f3ba..b85e6bde3 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -92,8 +92,10 @@ message DataMessage { } message OpenGroupInvitation { - optional string url = 1; - optional string name = 3; + // @required + required string url = 1; + // @required + required string name = 3; } message ClosedGroupControlMessage { diff --git a/ts/models/message.ts b/ts/models/message.ts index b54081e0b..4d6e9cae9 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -259,10 +259,6 @@ export class MessageModel extends Backbone.Model { window.Whisper.ExpirationTimerOptions.getAbbreviated(expireTimerUpdate.expireTimer || 0) ); } - const contacts = this.get('contact'); - if (contacts && contacts.length) { - return window.Signal.Types.Contact.getName(contacts[0]); - } return ''; } diff --git a/ts/models/messageType.ts b/ts/models/messageType.ts index 4f723d051..de6dd7184 100644 --- a/ts/models/messageType.ts +++ b/ts/models/messageType.ts @@ -32,7 +32,6 @@ export interface MessageAttributes { group_update?: any; groupInvitation?: any; attachments?: any; - contact?: any; conversationId: string; errors?: any; flags?: number; diff --git a/ts/receiver/attachments.ts b/ts/receiver/attachments.ts index 1822c6361..f19c080a9 100644 --- a/ts/receiver/attachments.ts +++ b/ts/receiver/attachments.ts @@ -181,42 +181,6 @@ async function processPreviews(message: MessageModel, convo: ConversationModel): return addedCount; } -async function processAvatars(message: MessageModel, convo: ConversationModel): Promise { - let addedCount = 0; - const isOpenGroupV2 = convo.isOpenGroupV2(); - - const contacts = message.get('contact') || []; - - const contact = await Promise.all( - contacts.map(async (item: any, index: any) => { - if (!item.avatar || !item.avatar.avatar) { - return item; - } - - addedCount += 1; - - const avatarJob = await AttachmentDownloads.addJob(item.avatar.avatar, { - messaeId: message.id, - type: 'contact', - index, - isOpenGroupV2, - }); - - return { - ...item, - avatar: { - ...item.avatar, - avatar: avatarJob, - }, - }; - }) - ); - - message.set({ contact }); - - return addedCount; -} - async function processQuoteAttachments( message: MessageModel, convo: ConversationModel @@ -292,8 +256,6 @@ export async function queueAttachmentDownloads( count += await processPreviews(message, conversation); - count += await processAvatars(message, conversation); - count += await processQuoteAttachments(message, conversation); // I don 't think we rely on this for anything diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index 5941a1ac9..7e2f228cd 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -331,7 +331,6 @@ async function handleRegularMessage( schemaVersion: dataMessage.schemaVersion, attachments: dataMessage.attachments, body: dataMessage.body, - contact: dataMessage.contact, conversationId: conversation.id, decrypted_at: now, errors: [], From 5bf844241bf535516c33ca515d0c7f4ac8c61ea9 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 10:27:00 +1000 Subject: [PATCH 07/10] fix tests --- js/modules/types/message.js | 32 +--------- test/backup_test.js | 33 +--------- test/modules/types/message_test.js | 96 ------------------------------ ts/receiver/dataMessage.ts | 16 ----- ts/session/snode_api/onions.ts | 7 +-- ts/types/Message.ts | 8 --- ts/util/accountManager.ts | 2 +- 7 files changed, 5 insertions(+), 189 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 44968644d..a9803bf5c 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -6,7 +6,6 @@ const SchemaVersion = require('./schema_version'); const { initializeAttachmentMetadata, } = require('../../../ts/types/message/initializeAttachmentMetadata'); -const Contact = require('./contact'); const GROUP = 'group'; const PRIVATE = 'private'; @@ -158,17 +157,6 @@ exports._mapAttachments = upgradeAttachment => async (message, context) => { return Object.assign({}, message, { attachments }); }; -// Public API -// _mapContact :: (Contact -> Promise Contact) -> -// (Message, Context) -> -// Promise Message -exports._mapContact = upgradeContact => async (message, context) => { - const contextWithMessage = Object.assign({}, context, { message }); - const upgradeWithContext = contact => upgradeContact(contact, contextWithMessage); - const contact = await Promise.all((message.contact || []).map(upgradeWithContext)); - return Object.assign({}, message, { contact }); -}; - // _mapQuotedAttachments :: (QuotedAttachment -> Promise QuotedAttachment) -> // (Message, Context) -> // Promise Message @@ -255,7 +243,7 @@ const toVersion5 = exports._withSchemaVersion({ }); const toVersion6 = exports._withSchemaVersion({ schemaVersion: 6, - upgrade: () => {}, + upgrade: message => message, }); // IMPORTANT: We’ve updated our definition of `initializeAttachmentMetadata`, so // we need to run it again on existing items that have previously been incorrectly @@ -343,8 +331,8 @@ exports.upgradeSchema = async ( if (maxVersion < index) { break; } - const currentVersion = VERSIONS[index]; + // 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 @@ -614,21 +602,6 @@ exports.createAttachmentDataWriter = ({ writeExistingAttachmentData, logger }) = return omit(thumbnail, ['data']); }); - const writeContactAvatar = async messageContact => { - const { avatar } = messageContact; - if (avatar && !avatar.avatar) { - return omit(messageContact, ['avatar']); - } - - await writeExistingAttachmentData(avatar.avatar); - - return Object.assign({}, messageContact, { - avatar: Object.assign({}, avatar, { - avatar: omit(avatar.avatar, ['data']), - }), - }); - }; - const writePreviewImage = async item => { const { image } = item; if (!image) { @@ -646,7 +619,6 @@ exports.createAttachmentDataWriter = ({ writeExistingAttachmentData, logger }) = {}, await writeThumbnails(message, { logger }), { - contact: await Promise.all((contact || []).map(writeContactAvatar)), preview: await Promise.all((preview || []).map(writePreviewImage)), attachments: await Promise.all( (attachments || []).map(async attachment => { diff --git a/test/backup_test.js b/test/backup_test.js index 90f79fc4f..526d73ac2 100644 --- a/test/backup_test.js +++ b/test/backup_test.js @@ -214,7 +214,6 @@ describe('Backup', () => { const OUR_NUMBER = '+12025550000'; const CONTACT_ONE_NUMBER = '+12025550001'; - const CONTACT_TWO_NUMBER = '+12025550002'; const toArrayBuffer = nodeBuffer => nodeBuffer.buffer.slice( @@ -310,17 +309,6 @@ describe('Backup', () => { }); return Object.assign({}, await loadThumbnails(message), { - contact: await Promise.all( - (message.contact || []).map(async contact => { - return contact && contact.avatar && contact.avatar.avatar - ? Object.assign({}, contact, { - avatar: Object.assign({}, contact.avatar, { - avatar: await wrappedLoadAttachment(contact.avatar.avatar), - }), - }) - : contact; - }) - ), attachments: await Promise.all( (message.attachments || []).map(async attachment => { await wrappedLoadAttachment(attachment); @@ -401,26 +389,7 @@ describe('Backup', () => { }, ], }, - contact: [ - { - name: { - displayName: 'Someone Somewhere', - }, - number: [ - { - value: CONTACT_TWO_NUMBER, - type: 1, - }, - ], - avatar: { - isProfile: false, - avatar: { - contentType: 'image/png', - data: FIXTURES.png, - }, - }, - }, - ], + preview: [ { url: 'https://www.instagram.com/p/BsOGulcndj-/', diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index c42bfe966..83c92ee15 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -69,7 +69,6 @@ describe('Message', () => { path: 'ab/abcdefghi', }, ], - contact: [], preview: [], }; @@ -114,55 +113,6 @@ describe('Message', () => { }, ], }, - contact: [], - preview: [], - }; - - const writeExistingAttachmentData = attachment => { - assert.equal(attachment.path, 'ab/abcdefghi'); - assert.deepEqual(attachment.data, stringToArrayBuffer('It’s easy if you try')); - }; - - const actual = await Message.createAttachmentDataWriter({ - writeExistingAttachmentData, - logger, - })(input); - assert.deepEqual(actual, expected); - }); - - it('should process contact avatars', async () => { - const input = { - body: 'Imagine there is no heaven…', - schemaVersion: 4, - attachments: [], - contact: [ - { - name: 'john', - avatar: { - isProfile: false, - avatar: { - path: 'ab/abcdefghi', - data: stringToArrayBuffer('It’s easy if you try'), - }, - }, - }, - ], - }; - const expected = { - body: 'Imagine there is no heaven…', - schemaVersion: 4, - attachments: [], - contact: [ - { - name: 'john', - avatar: { - isProfile: false, - avatar: { - path: 'ab/abcdefghi', - }, - }, - }, - ], preview: [], }; @@ -277,7 +227,6 @@ describe('Message', () => { hasVisualMediaAttachments: undefined, hasFileAttachments: undefined, schemaVersion: Message.CURRENT_SCHEMA_VERSION, - contact: [], }; const expectedAttachmentData = stringToArrayBuffer('It’s easy if you try'); @@ -629,49 +578,4 @@ describe('Message', () => { assert.deepEqual(result, expected); }); }); - - describe('_mapContact', () => { - it('handles message with no contact field', async () => { - const upgradeContact = sinon.stub().throws(new Error("Shouldn't be called")); - const upgradeVersion = Message._mapContact(upgradeContact); - - const message = { - body: 'hey there!', - }; - const expected = { - body: 'hey there!', - contact: [], - }; - const result = await upgradeVersion(message); - assert.deepEqual(result, expected); - }); - - it('handles one contact', async () => { - const upgradeContact = contact => Promise.resolve(contact); - const upgradeVersion = Message._mapContact(upgradeContact); - - const message = { - body: 'hey there!', - contact: [ - { - name: { - displayName: 'Someone somewhere', - }, - }, - ], - }; - const expected = { - body: 'hey there!', - contact: [ - { - name: { - displayName: 'Someone somewhere', - }, - }, - ], - }; - const result = await upgradeVersion(message); - assert.deepEqual(result, expected); - }); - }); }); diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index 062ce4615..c8637ade0 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -140,22 +140,6 @@ function cleanAttachments(decrypted: any) { }; }); - decrypted.contact = (decrypted.contact || []).map((item: any) => { - const { avatar } = item; - - if (!avatar || !avatar.avatar) { - return item; - } - - return { - ...item, - avatar: { - ...item.avatar, - avatar: cleanAttachment(item.avatar.avatar), - }, - }; - }); - if (quote) { if (quote.id) { quote.id = _.toNumber(quote.id); diff --git a/ts/session/snode_api/onions.ts b/ts/session/snode_api/onions.ts index ea7634af9..f4b566e48 100644 --- a/ts/session/snode_api/onions.ts +++ b/ts/session/snode_api/onions.ts @@ -239,18 +239,13 @@ async function processOnionRequestErrorAtDestination({ if (statusCode === 200) { return; } - window?.log?.info('processOnionRequestErrorAtDestination. statusCode ok:', statusCode); + window?.log?.info('processOnionRequestErrorAtDestination. statusCode nok:', statusCode); process406Error(statusCode); await process421Error(statusCode, body, associatedWith, destinationEd25519); processOxenServerError(statusCode, body); if (destinationEd25519) { await processAnyOtherErrorAtDestination(statusCode, body, destinationEd25519, associatedWith); - } else { - console.warn( - 'processOnionRequestErrorAtDestination: destinationEd25519 unset. was it an open group call?', - statusCode - ); } } diff --git a/ts/types/Message.ts b/ts/types/Message.ts index 89d49492f..82d41b78a 100644 --- a/ts/types/Message.ts +++ b/ts/types/Message.ts @@ -1,5 +1,4 @@ import { Attachment } from './Attachment'; -import { Contact } from './Contact'; import { IndexableBoolean, IndexablePresence } from './IndexedDB'; export type Message = UserMessage; @@ -23,7 +22,6 @@ export type IncomingMessage = Readonly< sourceDevice?: number; } & SharedMessageProperties & MessageSchemaVersion5 & - MessageSchemaVersion6 & ExpirationTimerUpdate >; @@ -51,12 +49,6 @@ type MessageSchemaVersion5 = Partial< }> >; -type MessageSchemaVersion6 = Partial< - Readonly<{ - contact: Array; - }> ->; - export type LokiProfile = { displayName: string; avatarPointer: string; diff --git a/ts/util/accountManager.ts b/ts/util/accountManager.ts index 1128cf4a8..21427f744 100644 --- a/ts/util/accountManager.ts +++ b/ts/util/accountManager.ts @@ -178,7 +178,7 @@ async function bouncyDeleteAccount(reason?: string) { } export async function deleteAccount(reason?: string) { - return _.debounce(() => bouncyDeleteAccount(reason), 200); + return bouncyDeleteAccount(reason); } async function createAccount(identityKeyPair: any) { From 3c80869418725e59d1aa02b0d29bbab603bdc66e Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 11:41:11 +1000 Subject: [PATCH 08/10] improve bad path handling when snode not in path --- ts/session/onions/onionPath.ts | 30 +++++++++++++++++++----------- ts/session/snode_api/onions.ts | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/ts/session/onions/onionPath.ts b/ts/session/onions/onionPath.ts index 676d08dde..841ab1535 100644 --- a/ts/session/onions/onionPath.ts +++ b/ts/session/onions/onionPath.ts @@ -5,6 +5,7 @@ import { default as insecureNodeFetch } from 'node-fetch'; import { UserUtils } from '../utils'; import { incrementBadSnodeCountOrDrop, snodeHttpsAgent } from '../snode_api/onions'; import { allowOnlyOneAtATime } from '../utils/Promise'; +import pRetry from 'p-retry'; const desiredGuardCount = 3; const minimumGuardCount = 2; @@ -91,8 +92,7 @@ export async function dropSnodeFromPath(snodeEd25519: string) { window?.log?.warn( `Could not drop ${ed25519Str(snodeEd25519)} from path index: ${pathWithSnodeIndex}` ); - - return; + throw new Error(`Could not drop snode ${ed25519Str(snodeEd25519)} from path: not in any paths`); } window?.log?.info( `dropping snode ${ed25519Str(snodeEd25519)} from path index: ${pathWithSnodeIndex}` @@ -156,20 +156,28 @@ export async function getOnionPath(toExclude?: Snode): Promise> { /** * If we don't know which nodes is causing trouble, increment the issue with this full path. */ -export async function incrementBadPathCountOrDrop(guardNodeEd25519: string) { - const pathIndex = onionPaths.findIndex(p => p[0].pubkey_ed25519 === guardNodeEd25519); - window?.log?.info( - `\t\tincrementBadPathCountOrDrop starting with guard ${ed25519Str(guardNodeEd25519)}` +export async function incrementBadPathCountOrDrop(sndeEd25519: string) { + const pathWithSnodeIndex = onionPaths.findIndex(path => + path.some(snode => snode.pubkey_ed25519 === sndeEd25519) ); - if (pathIndex === -1) { - (window?.log?.info || console.warn)('Did not find path with this guard node'); - return; + if (pathWithSnodeIndex === -1) { + (window?.log?.info || console.warn)('Did not find any path containing this snode'); + // this can only be bad. throw an abortError so we use another path if needed + throw new pRetry.AbortError( + 'incrementBadPathCountOrDrop: Did not find any path containing this snode' + ); } - const pathWithIssues = onionPaths[pathIndex]; + const guardNodeEd25519 = onionPaths[pathWithSnodeIndex][0].pubkey_ed25519; + + window?.log?.info( + `\t\tincrementBadPathCountOrDrop starting with guard ${ed25519Str(guardNodeEd25519)}` + ); + + const pathWithIssues = onionPaths[pathWithSnodeIndex]; - window?.log?.info('handling bad path for path index', pathIndex); + window?.log?.info('handling bad path for path index', pathWithSnodeIndex); const oldPathFailureCount = pathFailureCount[guardNodeEd25519] || 0; // tslint:disable: prefer-for-of diff --git a/ts/session/snode_api/onions.ts b/ts/session/snode_api/onions.ts index f4b566e48..c22d65bf1 100644 --- a/ts/session/snode_api/onions.ts +++ b/ts/session/snode_api/onions.ts @@ -591,7 +591,7 @@ export async function incrementBadSnodeCountOrDrop({ await OnionPaths.dropSnodeFromPath(snodeEd25519); } catch (e) { window?.log?.warn( - 'dropSnodeFromPath, got error while patchingup... incrementing the whole path as bad', + 'dropSnodeFromPath, got error while patching up... incrementing the whole path as bad', e ); // if dropSnodeFromPath throws, it means there is an issue patching up the path, increment the whole path issues count From b403b8922482d6e68803458ef7af1156a562c541 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 12:26:48 +1000 Subject: [PATCH 09/10] fix bug with quote empty profileName --- ts/models/message.ts | 5 +++-- ts/session/snode_api/onions.ts | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ts/models/message.ts b/ts/models/message.ts index 4d6e9cae9..00a31d22a 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -639,9 +639,10 @@ export class MessageModel extends Backbone.Model { } const { author, id, referencedMessageNotFound } = quote; - const contact = author && ConversationController.getInstance().get(author); + const contact: ConversationModel = author && ConversationController.getInstance().get(author); + + const authorName = contact ? contact.getContactProfileNameOrShortenedPubKey() : null; - const authorName = contact ? contact.getName() : null; const isFromMe = contact ? contact.id === UserUtils.getOurPubKeyStrFromCache() : false; const onClick = noClick ? null diff --git a/ts/session/snode_api/onions.ts b/ts/session/snode_api/onions.ts index c22d65bf1..8c2459b3f 100644 --- a/ts/session/snode_api/onions.ts +++ b/ts/session/snode_api/onions.ts @@ -535,7 +535,6 @@ async function handle421InvalidSwarm({ await dropSnodeFromSwarmIfNeeded(associatedWith, snodeEd25519); } catch (e) { if (e.message !== exceptionMessage) { - console.warn('dropSnodeFromSwarmIfNeeded', snodeEd25519); window?.log?.warn( 'Got error while parsing 421 result. Dropping this snode from the swarm of this pubkey', e From c5287158c4b8db829c5aa63df7f305273f0c78d0 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 10 Jun 2021 12:27:03 +1000 Subject: [PATCH 10/10] merge protobuf attachment and preview types --- protos/SignalService.proto | 10 ++-- .../outgoing/visibleMessage/VisibleMessage.ts | 57 ++++++++++++------- ts/session/utils/Attachments.ts | 20 +++++-- ts/session/utils/AttachmentsV2.ts | 8 ++- ts/session/utils/syncUtils.ts | 6 +- .../session/unit/messages/ChatMessage_test.ts | 12 ++-- 6 files changed, 71 insertions(+), 42 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index b85e6bde3..ac8536809 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -157,8 +157,10 @@ message ConfigurationMessage { } message Contact { - optional bytes publicKey = 1; - optional string name = 2; + // @required + required bytes publicKey = 1; + // @required + required string name = 2; optional string profilePicture = 3; optional bytes profileKey = 4; } @@ -179,7 +181,7 @@ message ReceiptMessage { } // @required - optional Type type = 1; + required Type type = 1; repeated uint64 timestamp = 2; } @@ -190,7 +192,7 @@ message AttachmentPointer { } // @required - optional fixed64 id = 1; + required fixed64 id = 1; optional string contentType = 2; optional bytes key = 3; optional uint32 size = 4; diff --git a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts index e90fae4b4..21d3004ce 100644 --- a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts @@ -8,7 +8,6 @@ import { LokiProfile } from '../../../../types/Message'; import { MessageParams } from '../Message'; interface AttachmentPointerCommon { - id?: number; contentType?: string; key?: Uint8Array; size?: number; @@ -23,10 +22,12 @@ interface AttachmentPointerCommon { export interface AttachmentPointer extends AttachmentPointerCommon { url?: string; + id?: number; } export interface AttachmentPointerWithUrl extends AttachmentPointerCommon { url: string; + id: number; } export interface Preview { @@ -35,39 +36,53 @@ export interface Preview { image?: AttachmentPointer; } -export interface QuotedAttachment { +export interface PreviewWithAttachmentUrl { + url: string; + id: number; + title?: string; + image?: AttachmentPointerWithUrl; +} + +interface QuotedAttachmentCommon { contentType?: string; fileName?: string; +} + +export interface QuotedAttachment extends QuotedAttachmentCommon { thumbnail?: AttachmentPointer; } +export interface QuotedAttachmentWithUrl extends QuotedAttachmentCommon { + thumbnail?: AttachmentPointerWithUrl | QuotedAttachment; +} + export interface Quote { id: number; author: string; text?: string; - attachments?: Array; + attachments?: Array; } export interface VisibleMessageParams extends MessageParams { - attachments?: Array; + attachments?: Array; body?: string; quote?: Quote; expireTimer?: number; lokiProfile?: LokiProfile; - preview?: Array; + preview?: Array; syncTarget?: string; // null means it is not a synced message } export class VisibleMessage extends DataMessage { public readonly expireTimer?: number; - private readonly attachments?: Array; + private readonly attachments?: Array; private readonly body?: string; private readonly quote?: Quote; private readonly profileKey?: Uint8Array; private readonly displayName?: string; private readonly avatarPointer?: string; - private readonly preview?: Array; + private readonly preview?: Array; /// In the case of a sync message, the public key of the person the message was targeted at. /// - Note: `null or undefined` if this isn't a sync message. @@ -141,22 +156,20 @@ export class VisibleMessage extends DataMessage { dataMessage.quote.author = this.quote.author; dataMessage.quote.text = this.quote.text; if (this.quote.attachments) { - dataMessage.quote.attachments = this.quote.attachments.map( - (attachment: QuotedAttachment) => { - const quotedAttachment = new SignalService.DataMessage.Quote.QuotedAttachment(); - if (attachment.contentType) { - quotedAttachment.contentType = attachment.contentType; - } - if (attachment.fileName) { - quotedAttachment.fileName = attachment.fileName; - } - if (attachment.thumbnail) { - quotedAttachment.thumbnail = attachment.thumbnail; - } - - return quotedAttachment; + dataMessage.quote.attachments = this.quote.attachments.map(attachment => { + const quotedAttachment = new SignalService.DataMessage.Quote.QuotedAttachment(); + if (attachment.contentType) { + quotedAttachment.contentType = attachment.contentType; } - ); + if (attachment.fileName) { + quotedAttachment.fileName = attachment.fileName; + } + if (attachment.thumbnail && (attachment.thumbnail as any).id) { + quotedAttachment.thumbnail = attachment.thumbnail as any; // be sure to keep the typescript guard on id above + } + + return quotedAttachment; + }); } } diff --git a/ts/session/utils/Attachments.ts b/ts/session/utils/Attachments.ts index c9751ad0b..225d06536 100644 --- a/ts/session/utils/Attachments.ts +++ b/ts/session/utils/Attachments.ts @@ -5,8 +5,10 @@ import { AttachmentPointer, AttachmentPointerWithUrl, Preview, + PreviewWithAttachmentUrl, Quote, QuotedAttachment, + QuotedAttachmentWithUrl, } from '../messages/outgoing/visibleMessage/VisibleMessage'; import { FSv2 } from '../../fileserver'; import { addAttachmentPadding } from '../crypto/BufferPadding'; @@ -101,7 +103,7 @@ export class AttachmentFsV2Utils { public static async uploadAttachmentsToFsV2( attachments: Array - ): Promise> { + ): Promise> { const promises = (attachments || []).map(async attachment => this.uploadToFsV2({ attachment, @@ -114,7 +116,7 @@ export class AttachmentFsV2Utils { public static async uploadLinkPreviewsToFsV2( previews: Array - ): Promise> { + ): Promise> { const promises = (previews || []).map(async preview => { // some links does not have an image associated, and it makes the whole message fail to send if (!preview.image) { @@ -127,8 +129,9 @@ export class AttachmentFsV2Utils { return { ...preview, image, - url: preview.url || image.url, - } as Preview; + url: image.url, + id: image.id, + }; }); return _.compact(await Promise.all(promises)); } @@ -145,13 +148,18 @@ export class AttachmentFsV2Utils { attachment: attachment.thumbnail, }); } + if (!thumbnail) { + return attachment; + } return { ...attachment, thumbnail, - } as QuotedAttachment; + url: thumbnail.url, + id: thumbnail.id, + } as QuotedAttachmentWithUrl; }); - const attachments = await Promise.all(promises); + const attachments = _.compact(await Promise.all(promises)); return { ...quote, diff --git a/ts/session/utils/AttachmentsV2.ts b/ts/session/utils/AttachmentsV2.ts index abd13c9ce..d1783658c 100644 --- a/ts/session/utils/AttachmentsV2.ts +++ b/ts/session/utils/AttachmentsV2.ts @@ -5,6 +5,7 @@ import { AttachmentPointer, AttachmentPointerWithUrl, Preview, + PreviewWithAttachmentUrl, Quote, QuotedAttachment, } from '../messages/outgoing/visibleMessage/VisibleMessage'; @@ -59,7 +60,7 @@ export async function uploadV2(params: UploadParamsV2): Promise, openGroup: OpenGroupRequestCommonType -): Promise> { +): Promise> { const promises = (attachments || []).map(async attachment => exports.uploadV2({ attachment, @@ -73,7 +74,7 @@ export async function uploadAttachmentsV2( export async function uploadLinkPreviewsV2( previews: Array, openGroup: OpenGroupRequestCommonType -): Promise> { +): Promise> { const promises = (previews || []).map(async preview => { // some links does not have an image associated, and it makes the whole message fail to send if (!preview.image) { @@ -89,6 +90,7 @@ export async function uploadLinkPreviewsV2( ...preview, image, url: preview.url || (image.url as string), + id: image.id as number, }; }); return _.compact(await Promise.all(promises)); @@ -103,7 +105,7 @@ export async function uploadQuoteThumbnailsV2( } const promises = (quote.attachments ?? []).map(async attachment => { - let thumbnail: AttachmentPointer | undefined; + let thumbnail: PreviewWithAttachmentUrl | undefined; if (attachment.thumbnail) { thumbnail = await exports.uploadV2({ attachment: attachment.thumbnail, diff --git a/ts/session/utils/syncUtils.ts b/ts/session/utils/syncUtils.ts index 74c8c7d80..14b6daf93 100644 --- a/ts/session/utils/syncUtils.ts +++ b/ts/session/utils/syncUtils.ts @@ -20,7 +20,9 @@ import { SignalService } from '../../protobuf'; import _ from 'lodash'; import { AttachmentPointer, + AttachmentPointerWithUrl, Preview, + PreviewWithAttachmentUrl, Quote, VisibleMessage, } from '../messages/outgoing/visibleMessage/VisibleMessage'; @@ -237,9 +239,9 @@ const buildSyncVisibleMessage = ( key, digest, }; - }) as Array; + }) as Array; const quote = (dataMessage.quote as Quote) || undefined; - const preview = (dataMessage.preview as Array) || []; + const preview = (dataMessage.preview as Array) || []; const expireTimer = dataMessage.expireTimer; return new VisibleMessage({ diff --git a/ts/test/session/unit/messages/ChatMessage_test.ts b/ts/test/session/unit/messages/ChatMessage_test.ts index bd99449fa..e2b3c4b5e 100644 --- a/ts/test/session/unit/messages/ChatMessage_test.ts +++ b/ts/test/session/unit/messages/ChatMessage_test.ts @@ -6,7 +6,9 @@ import { toNumber } from 'lodash'; import { Constants } from '../../../../session'; import { AttachmentPointer, + AttachmentPointerWithUrl, Preview, + PreviewWithAttachmentUrl, Quote, VisibleMessage, } from '../../../../session/messages/outgoing/visibleMessage/VisibleMessage'; @@ -84,10 +86,10 @@ describe('VisibleMessage', () => { }); it('can create message with a preview', () => { - let preview: Preview; + let preview: PreviewWithAttachmentUrl; - preview = { url: 'url', title: 'title' }; - const previews = new Array(); + preview = { url: 'url', title: 'title', id: 1234 }; + const previews = new Array(); previews.push(preview); const message = new VisibleMessage({ @@ -106,10 +108,10 @@ describe('VisibleMessage', () => { }); it('can create message with an AttachmentPointer', () => { - let attachment: AttachmentPointer; + let attachment: AttachmentPointerWithUrl; attachment = { url: 'url', contentType: 'contentType', id: 1234 }; - const attachments = new Array(); + const attachments = new Array(); attachments.push(attachment); const message = new VisibleMessage({