Reject a renumbered multipart total and check abort before the send window
This commit is contained in:
+14
-1
@@ -132,7 +132,20 @@ export class Reassembler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const key = groupKey(pduObj, concat.reference);
|
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 replaced = group.parts.get(concat.part);
|
||||||
const segment = detach(pduObj);
|
const segment = detach(pduObj);
|
||||||
const delta = octetsOf(segment) - (replaced === undefined ? 0 : octetsOf(replaced));
|
const delta = octetsOf(segment) - (replaced === undefined ? 0 : octetsOf(replaced));
|
||||||
|
|||||||
@@ -127,6 +127,11 @@ export class Session extends EventEmitter<SessionEvents> {
|
|||||||
|
|
||||||
if (this.closed) return { err: new Error('Session is closed') };
|
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();
|
await this.window.acquire();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -365,6 +365,17 @@ describe('reassembly bounds', () => {
|
|||||||
assert.equal(reassembler.size, 0);
|
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.
|
// 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', () => {
|
test('drops the oldest incomplete message once the cap is reached', () => {
|
||||||
const reassembler = new Reassembler({ log: silentLog, max: 2, now: () => 0, timeout: 60_000 });
|
const reassembler = new Reassembler({ log: silentLog, max: 2, now: () => 0, timeout: 60_000 });
|
||||||
|
|||||||
@@ -1000,6 +1000,38 @@ describe('robustness', () => {
|
|||||||
assert.deepEqual(seen, []);
|
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.
|
// 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 () => {
|
test('leaves no socket open when coming back up fails', async () => {
|
||||||
const opened: net.Socket[] = [];
|
const opened: net.Socket[] = [];
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ rules there constrain every item below.
|
|||||||
|
|
||||||
## Status
|
## 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.
|
18, 20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0.
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
Reference in New Issue
Block a user