fix: address PR comments

pull/3281/head
Audric Ackermann 3 months ago
parent 1766cc0814
commit 79d3d00646
No known key found for this signature in database

@ -482,7 +482,9 @@ export async function innerHandleSwarmContentMessage({
const content = SignalService.Content.decode(new Uint8Array(contentDecrypted)); const content = SignalService.Content.decode(new Uint8Array(contentDecrypted));
if (!shouldProcessContentMessage(envelope, content, false)) { 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); await IncomingMessageCache.removeFromCache(envelope);
return; return;
} }

@ -2,11 +2,12 @@ import { SignalService } from '../../../protobuf';
import { TTL_DEFAULT } from '../../constants'; import { TTL_DEFAULT } from '../../constants';
import { Message } from './Message'; import { Message } from './Message';
type InstanceKeys<T> = { type InstanceFields<T> = {
// eslint-disable-next-line @typescript-eslint/ban-types // eslint-disable-next-line @typescript-eslint/ban-types
[K in keyof T as T[K] extends Function ? never : K]: T[K]; [K in keyof T as T[K] extends Function ? never : K]: T[K];
}; };
type ContentFields = Partial<Omit<InstanceKeys<SignalService.Content>, 'sigTimestamp'>>;
type ContentFields = Partial<Omit<InstanceFields<SignalService.Content>, 'sigTimestamp'>>;
export abstract class ContentMessage extends Message { export abstract class ContentMessage extends Message {
public plainTextBuffer(): Uint8Array { public plainTextBuffer(): Uint8Array {
@ -23,8 +24,8 @@ export abstract class ContentMessage extends Message {
public makeContentProto<T extends ContentFields>(extra: T) { public makeContentProto<T extends ContentFields>(extra: T) {
return new SignalService.Content({ return new SignalService.Content({
sigTimestamp: this.createAtNetworkTimestamp,
...extra, ...extra,
sigTimestamp: this.createAtNetworkTimestamp,
}); });
} }

@ -30,6 +30,14 @@ export class ExpirationTimerUpdateMessage extends DataMessage {
this.syncTarget = params.syncTarget ? PubKey.cast(params.syncTarget).key : undefined; 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 { public dataProto(): SignalService.DataMessage {
const data = new SignalService.DataMessage({}); const data = new SignalService.DataMessage({});

@ -69,39 +69,44 @@ describe('shouldProcessContentMessage', () => {
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: envelopeTs }, // exact match { sigTimestamp: envelopeTs },
isCommunity isCommunity
) ),
'exact match'
).to.eq(true); ).to.eq(true);
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: envelopeTs + 6 * DURATION.HOURS - 1 }, // just below 6h of diff (positive) { sigTimestamp: envelopeTs + 6 * DURATION.HOURS - 1 },
isCommunity isCommunity
) ),
'just below 6h of diff (positive)'
).to.eq(true); ).to.eq(true);
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: envelopeTs - 6 * DURATION.HOURS + 1 }, // just below 6h of diff (negative) { sigTimestamp: envelopeTs - 6 * DURATION.HOURS + 1 },
isCommunity isCommunity
) ),
'just below 6h of diff (negative)'
).to.eq(true); ).to.eq(true);
}); });
it('if timestamps do not roughly match: return false', async () => { it('if timestamps do not roughly match: return false', async () => {
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: envelopeTs + 6 * DURATION.HOURS + 1 }, // just above 6h of diff { sigTimestamp: envelopeTs + 6 * DURATION.HOURS + 1 },
isCommunity isCommunity
) ),
'just above 6h of diff'
).to.eq(false); ).to.eq(false);
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: envelopeTs - 6 * DURATION.HOURS - 1 }, // just above 6h of diff { sigTimestamp: envelopeTs - 6 * DURATION.HOURS - 1 },
isCommunity isCommunity
) ),
'just above 6h of diff'
).to.eq(false); ).to.eq(false);
}); });
}); });
@ -112,17 +117,20 @@ describe('shouldProcessContentMessage', () => {
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: undefined as any }, { sigTimestamp: undefined as any },
isCommunity isCommunity
) ),
'sigTimestamp undefined'
).to.eq(true); ).to.eq(true);
expect( expect(
shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity) shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity),
'sigTimestamp 0 as number'
).to.eq(true); ).to.eq(true);
expect( expect(
shouldProcessContentMessage( shouldProcessContentMessage(
{ timestamp: envelopeTs }, { timestamp: envelopeTs },
{ sigTimestamp: Long.fromNumber(0) as any }, { sigTimestamp: Long.fromNumber(0) as any },
isCommunity isCommunity
) ),
'sigTimestamp 0 as Long'
).to.eq(true); ).to.eq(true);
}); });
}); });

Loading…
Cancel
Save