diff --git a/src/reassembly.ts b/src/reassembly.ts index c3b1b92..417f103 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -132,7 +132,20 @@ export class Reassembler { } const key = groupKey(pduObj, concat.reference); - const group = this.groups.get(key) ?? this.open(key, concat.total); + const existing = this.groups.get(key); + + // Parts 1/2 and 2/3 would otherwise complete the stored two-part group as a truncated message. + if (existing && existing.total !== concat.total) { + this.log.warn('reassembler - dropping a segment with an inconsistent UDH total', { + existingTotal: existing.total, + part: concat.part, + total: concat.total, + }); + + return undefined; + } + + const group = existing ?? this.open(key, concat.total); const replaced = group.parts.get(concat.part); const segment = detach(pduObj); const delta = octetsOf(segment) - (replaced === undefined ? 0 : octetsOf(replaced)); diff --git a/src/session.ts b/src/session.ts index 6699d90..3b2c6db 100644 --- a/src/session.ts +++ b/src/session.ts @@ -127,6 +127,11 @@ export class Session extends EventEmitter { if (this.closed) return { err: new Error('Session is closed') }; + // Before the window, or a full window makes an aborted call wait for a slot it will not use. + if (options.signal?.aborted === true) { + return { err: new Error('Aborted before the request was sent') }; + } + await this.window.acquire(); try { diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 52bb76d..0a85a71 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -365,6 +365,17 @@ describe('reassembly bounds', () => { assert.equal(reassembler.size, 0); }); + // Parts 1/2 then 2/3 would otherwise complete the stored two-part group, truncating the message. + test('refuses a segment that renumbers how many parts the message has', () => { + const reassembler = new Reassembler({ log: silentLog, max: 10, now: () => 0, timeout: 60_000 }); + + assert.equal(collect(reassembler, 9, 1, 2), undefined); + assert.equal(collect(reassembler, 9, 2, 3), undefined); + assert.equal(reassembler.size, 1); + + reassembler.clear(); + }); + // 0.4.0 held incomplete groups without limit and swept them only when other traffic arrived. test('drops the oldest incomplete message once the cap is reached', () => { const reassembler = new Reassembler({ log: silentLog, max: 2, now: () => 0, timeout: 60_000 }); diff --git a/test/session.test.ts b/test/session.test.ts index 493c6e6..c5b68a7 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -1000,6 +1000,38 @@ describe('robustness', () => { assert.deepEqual(seen, []); }); + // The guard sits before the send window, or a full window makes the aborted call queue first. + test('does not wait for a send window slot it will never use', async t => { + const smpp = await startServer(); + + t.after(() => smpp.close()); + + // The peer answers nothing, so the one slot stays held for the whole test. + smpp.on('session', session => session.on('sms', () => undefined)); + + const { session } = await connect(smpp, { maxOutstanding: 1, responseTimeout: 5000 }); + + assert.ok(session); + t.after(() => { session.close(); }); + + const held = session.sendSms({ from: '46701113311', message: 'holds the slot', to: '46709771337' }); + const controller = new AbortController(); + + controller.abort(); + + const aborted = await raceWithin(500, session.sendSms({ + from: '46701113311', + message: 'must not queue behind the held one', + to: '46709771337', + }, { signal: controller.signal })); + + assert.notEqual(aborted, false, 'an aborted send should not wait for the window'); + assert.ok(aborted !== false && aborted.err instanceof Error); + + session.close(); + await held; + }); + // A socket the loop opened and never handed over is one leaked per retry, forever. test('leaves no socket open when coming back up fails', async () => { const opened: net.Socket[] = []; diff --git a/todo.md b/todo.md index ddaa1bb..6a30e66 100644 --- a/todo.md +++ b/todo.md @@ -5,7 +5,7 @@ rules there constrain every item below. ## Status -The rewrite is **feature complete and green**: 208 tests, lint and typecheck clean, verified on Node +The rewrite is **feature complete and green**: 210 tests, lint and typecheck clean, verified on Node 18, 20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0. ```bash