From 79d3d00646a389bf610a766563670119a824d666 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 13 Feb 2025 16:10:11 +1100 Subject: [PATCH] fix: address PR comments --- ts/receiver/contentMessage.ts | 4 ++- .../messages/outgoing/ContentMessage.ts | 7 ++-- .../ExpirationTimerUpdateMessage.ts | 8 +++++ .../shouldProcessMessage_test.ts | 34 ++++++++++++------- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 01e854590..07756f1e5 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -482,7 +482,9 @@ export async function innerHandleSwarmContentMessage({ const content = SignalService.Content.decode(new Uint8Array(contentDecrypted)); if (!shouldProcessContentMessage(envelope, content, false)) { - window.log.info('innerHandleSwarmContentMessage: dropping invalid content message'); + window.log.info( + `innerHandleSwarmContentMessage: dropping invalid content message ${envelope.timestamp}` + ); await IncomingMessageCache.removeFromCache(envelope); return; } diff --git a/ts/session/messages/outgoing/ContentMessage.ts b/ts/session/messages/outgoing/ContentMessage.ts index cf96fabde..07a71cefe 100644 --- a/ts/session/messages/outgoing/ContentMessage.ts +++ b/ts/session/messages/outgoing/ContentMessage.ts @@ -2,11 +2,12 @@ import { SignalService } from '../../../protobuf'; import { TTL_DEFAULT } from '../../constants'; import { Message } from './Message'; -type InstanceKeys = { +type InstanceFields = { // eslint-disable-next-line @typescript-eslint/ban-types [K in keyof T as T[K] extends Function ? never : K]: T[K]; }; -type ContentFields = Partial, 'sigTimestamp'>>; + +type ContentFields = Partial, 'sigTimestamp'>>; export abstract class ContentMessage extends Message { public plainTextBuffer(): Uint8Array { @@ -23,8 +24,8 @@ export abstract class ContentMessage extends Message { public makeContentProto(extra: T) { return new SignalService.Content({ - sigTimestamp: this.createAtNetworkTimestamp, ...extra, + sigTimestamp: this.createAtNetworkTimestamp, }); } diff --git a/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts b/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts index 48f5e4a0f..77c26bbff 100644 --- a/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts @@ -30,6 +30,14 @@ export class ExpirationTimerUpdateMessage extends DataMessage { this.syncTarget = params.syncTarget ? PubKey.cast(params.syncTarget).key : undefined; } + public contentProto(): SignalService.Content { + // TODO: I am pretty sure we don't need this anymore (super.contentProto does the same in DataMessage) + return new SignalService.Content({ + ...super.contentProto(), + dataMessage: this.dataProto(), + }); + } + public dataProto(): SignalService.DataMessage { const data = new SignalService.DataMessage({}); diff --git a/ts/test/session/unit/receiver/drop_incoming/shouldProcessMessage_test.ts b/ts/test/session/unit/receiver/drop_incoming/shouldProcessMessage_test.ts index 4e6557fc8..25106acc0 100644 --- a/ts/test/session/unit/receiver/drop_incoming/shouldProcessMessage_test.ts +++ b/ts/test/session/unit/receiver/drop_incoming/shouldProcessMessage_test.ts @@ -69,39 +69,44 @@ describe('shouldProcessContentMessage', () => { expect( shouldProcessContentMessage( { timestamp: envelopeTs }, - { sigTimestamp: envelopeTs }, // exact match + { sigTimestamp: envelopeTs }, isCommunity - ) + ), + 'exact match' ).to.eq(true); expect( shouldProcessContentMessage( { timestamp: envelopeTs }, - { sigTimestamp: envelopeTs + 6 * DURATION.HOURS - 1 }, // just below 6h of diff (positive) + { sigTimestamp: envelopeTs + 6 * DURATION.HOURS - 1 }, isCommunity - ) + ), + 'just below 6h of diff (positive)' ).to.eq(true); expect( shouldProcessContentMessage( { timestamp: envelopeTs }, - { sigTimestamp: envelopeTs - 6 * DURATION.HOURS + 1 }, // just below 6h of diff (negative) + { sigTimestamp: envelopeTs - 6 * DURATION.HOURS + 1 }, isCommunity - ) + ), + 'just below 6h of diff (negative)' ).to.eq(true); }); it('if timestamps do not roughly match: return false', async () => { expect( shouldProcessContentMessage( { timestamp: envelopeTs }, - { sigTimestamp: envelopeTs + 6 * DURATION.HOURS + 1 }, // just above 6h of diff + { sigTimestamp: envelopeTs + 6 * DURATION.HOURS + 1 }, isCommunity - ) + ), + 'just above 6h of diff' ).to.eq(false); expect( shouldProcessContentMessage( { timestamp: envelopeTs }, - { sigTimestamp: envelopeTs - 6 * DURATION.HOURS - 1 }, // just above 6h of diff + { sigTimestamp: envelopeTs - 6 * DURATION.HOURS - 1 }, isCommunity - ) + ), + 'just above 6h of diff' ).to.eq(false); }); }); @@ -112,17 +117,20 @@ describe('shouldProcessContentMessage', () => { { timestamp: envelopeTs }, { sigTimestamp: undefined as any }, isCommunity - ) + ), + 'sigTimestamp undefined' ).to.eq(true); expect( - shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity) + shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity), + 'sigTimestamp 0 as number' ).to.eq(true); expect( shouldProcessContentMessage( { timestamp: envelopeTs }, { sigTimestamp: Long.fromNumber(0) as any }, isCommunity - ) + ), + 'sigTimestamp 0 as Long' ).to.eq(true); }); });