Read every occurrence of a repeatable TLV, and drop the unread tlvMap #25

Merged
lilleman merged 11 commits from multiple-tlvs into main 2026-09-25 18:54:29 +02:00
4 changed files with 25 additions and 8 deletions
Showing only changes of commit 2476684f24 - Show all commits
-1
View File
@@ -262,7 +262,6 @@ export function objToPdu<C extends CommandName>(obj: PduObjectInput<C>): Result<
); );
} }
function readParams( function readParams(
cmdName: CommandName, cmdName: CommandName,
pdu: Buffer, pdu: Buffer,
+6 -1
View File
@@ -73,6 +73,9 @@ function detach(pduObj: PduObject): PduObject {
return { ...pduObj, params, shortMessageOctets: octets, tlvs }; return { ...pduObj, params, shortMessageOctets: octets, tlvs };
} }
// Roughly the heap a listed occurrence costs beyond its value, so a PDU of empty repeats is not free.
const listedTlvOverhead = 200;
// A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU.
function sizeOf(value: ParamValue): number { function sizeOf(value: ParamValue): number {
if (Buffer.isBuffer(value)) return value.length; if (Buffer.isBuffer(value)) return value.length;
@@ -88,7 +91,9 @@ function octetsOf(pduObj: PduObject): number {
} }
for (const tlv of Object.values(pduObj.tlvs)) { for (const tlv of Object.values(pduObj.tlvs)) {
octets += tlvOctets(tlv.tagValue); const listed = Array.isArray(tlv.tagValue) ? tlv.tagValue.length : 0;
octets += tlvOctets(tlv.tagValue) + listed * listedTlvOverhead;
} }
return octets; return octets;
+11 -6
View File
@@ -1824,9 +1824,8 @@ describe('reassembly bounds', () => {
assert.equal(collectPayload(40).kept, true); assert.equal(collectPayload(40).kept, true);
}); });
// One segment is 36 octets before its two 10-octet callback numbers. test('counts every occurrence of a repeatable TLV against the octet cap, empty ones included', () => {
test('counts every occurrence of a repeatable TLV against the octet cap', () => { function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected {
function collectCallbacks(maxOctets: number): Collected {
const reassembler = new Reassembler({ const reassembler = new Reassembler({
log: silentLog, log: silentLog,
max: 10, max: 10,
@@ -1836,13 +1835,19 @@ describe('reassembly bounds', () => {
timeout: 60_000, timeout: 60_000,
}); });
const carried = segment(9, 1, 2); const carried = segment(9, 1, 2);
const tagValue = [Buffer.alloc(10, 0x31), Buffer.alloc(10, 0x32)];
return collectPdu(reassembler, { ...carried, tlvs: { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } } }); return collectPdu(reassembler, { ...carried, tlvs: { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } } });
} }
assert.equal(collectCallbacks(50).kept, false); const numbers = [Buffer.alloc(10_000, 0x31), Buffer.alloc(10_000, 0x32)];
assert.equal(collectCallbacks(60).kept, true);
assert.equal(collectCallbacks(20_000, numbers).kept, false);
assert.equal(collectCallbacks(30_000, numbers).kept, true);
assert.equal(
collectCallbacks(30_000, Array.from({ length: 10_000 }, () => Buffer.alloc(0))).kept,
false,
'an empty occurrence still holds an object',
);
}); });
// The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one.
+8
View File
@@ -6,6 +6,14 @@ hard rules first — they constrain every item below.
This is a working file that sets its own rules. The documentation conventions in AGENTS.md do not This is a working file that sets its own rules. The documentation conventions in AGENTS.md do not
govern it, and nothing here is a source anything else may cite. govern it, and nothing here is a source anything else may cite.
## Security
- [ ] **Charge a held segment's TLVs for the objects they keep, not only their value octets.** A
peer sending segments that carry thousands of distinct unknown tags with empty values makes
this library hold megabytes of heap per segment that `maxOctets` counts as nothing, up to
255 segments per group. Repeatable tags are already charged per occurrence. From the stability
review of #25.
## Status ## Status
The rewrite is **feature complete and green**: the suite, lint and typecheck are clean on Node 18 The rewrite is **feature complete and green**: the suite, lint and typecheck are clean on Node 18