diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f326f..ae4e243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,11 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. +- `maxOctets` charges each held segment 1000 octets beyond its own, 300 more per TLV on it, and 300 + per occurrence of a repeatable one. Segments of empty fields or thousands of empty TLVs used to + count as next to nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you + tuned low**: it now holds several times fewer segments, and an incomplete message evicted over + the cap is lost, since its segments were already answered. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and diff --git a/README.md b/README.md index d93ca5e..ab71fd2 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,7 @@ All optional. Timeouts are milliseconds. | `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. | | `idleTimeout` | `40000` | Drop a peer that has been silent this long. | | `maxReassembly` | `1000` | Incomplete multipart messages held per session. | -| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session. | +| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session: each held segment counts its octets plus 1000, 300 more per TLV on it, and 300 per occurrence of a repeatable one. | | `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. | | `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | | diff --git a/src/reassembly.ts b/src/reassembly.ts index a40eccc..c53d993 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -73,8 +73,9 @@ function detach(pduObj: PduObject): PduObject { 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; +// Measured heap beyond the octets, so a segment of empty fields or empty TLVs is not free. +const segmentObjectOverhead = 1000; +const tlvObjectOverhead = 300; // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. function sizeOf(value: ParamValue): number { @@ -84,7 +85,7 @@ function sizeOf(value: ParamValue): number { } function octetsOf(pduObj: PduObject): number { - let octets = 0; + let octets = segmentObjectOverhead; for (const value of Object.values(pduObj.params)) { octets += sizeOf(value); @@ -93,7 +94,7 @@ function octetsOf(pduObj: PduObject): number { for (const tlv of Object.values(pduObj.tlvs)) { const listed = Array.isArray(tlv.tagValue) ? tlv.tagValue.length : 0; - octets += tlvOctets(tlv.tagValue) + listed * listedTlvOverhead; + octets += tlvOctets(tlv.tagValue) + (1 + listed) * tlvObjectOverhead; } return octets; diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 3456adf..d4b4314 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1805,7 +1805,7 @@ describe('reassembly bounds', () => { assert.deepEqual(lost, [], 'the peer holds the only segment there was, so nothing was lost'); }); - // The two addresses are 22 octets, so only the 14 the TLV carries can overrun a cap of 30. + // The two addresses and the segment's and TLV's objects are 1322 octets, so only the 14 it carries can overrun 1330. test('counts a body carried in message_payload against the octet cap', () => { function collectPayload(maxOctets: number): Collected { const reassembler = new Reassembler({ @@ -1820,12 +1820,12 @@ describe('reassembly bounds', () => { return collectPdu(reassembler, payloadSegment(9, 1, 2)); } - assert.equal(collectPayload(30).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); - assert.equal(collectPayload(40).kept, true); + assert.equal(collectPayload(1330).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); + assert.equal(collectPayload(1340).kept, true); }); - test('counts every occurrence of a repeatable TLV against the octet cap, empty ones included', () => { - function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected { + test('counts the objects a segment and each of its TLVs hold against the octet cap, empty ones included', () => { + function collectTlvs(maxOctets: number, tlvs: PduObject['tlvs']): Collected { const reassembler = new Reassembler({ log: silentLog, max: 10, @@ -1834,11 +1834,17 @@ describe('reassembly bounds', () => { onLost: () => undefined, timeout: 60_000, }); - const carried = segment(9, 1, 2); - return collectPdu(reassembler, { ...carried, tlvs: { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } } }); + return collectPdu(reassembler, { ...segment(9, 1, 2), tlvs }); } + function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected { + return collectTlvs(maxOctets, { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } }); + } + const unknownTags = Object.fromEntries(Array.from({ length: 10_000 }, (_, i) => { + const tagId = 0x4000 + i; + return [String(tagId), { tagId, tagName: undefined, tagValue: Buffer.alloc(0) }]; + })); const numbers = [Buffer.alloc(10_000, 0x31), Buffer.alloc(10_000, 0x32)]; assert.equal(collectCallbacks(20_000, numbers).kept, false); @@ -1848,6 +1854,11 @@ describe('reassembly bounds', () => { false, 'an empty occurrence still holds an object', ); + // The segment itself is 1036 octets, so the tags must be charged 300 each to overrun 3,001,000. + assert.equal(collectTlvs(3_001_000, unknownTags).kept, false, 'an empty tag still holds an object'); + assert.equal(collectTlvs(3_002_000, unknownTags).kept, true); + assert.equal(collectTlvs(1_000, {}).kept, false, 'a segment holds objects beyond its 36 octets'); + assert.equal(collectTlvs(1_036, {}).kept, true); }); // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. @@ -1856,8 +1867,8 @@ describe('reassembly bounds', () => { const reassembler = new Reassembler({ log: silentLog, max: 10, - // One segment is 36 octets, so the second overruns a group already holding the first. - maxOctets: 50, + // One segment is 1036 octets, so the second overruns a group already holding the first. + maxOctets: 1050, now: () => 0, onLost: one => { lost.push(one); }, timeout: 60_000, @@ -1921,9 +1932,9 @@ describe('reassembly bounds', () => { assert.equal(counted.size, 1, 'the second group evicted the first, as a UDH group would'); counted.clear(); - // A sar_* segment is the two 11-octet addresses plus an 8-octet body, with no UDH to carry. - assert.equal(collectSar(capped(20), 3, 1, 2).kept, false); - assert.equal(collectSar(capped(30), 3, 1, 2).kept, true); + // A sar_* segment is the two 11-octet addresses, an 8-octet body, its own object and three TLVs'. + assert.equal(collectSar(capped(1920), 3, 1, 2).kept, false); + assert.equal(collectSar(capped(1930), 3, 1, 2).kept, true); }); // Nothing else says a message the peer has already been answered for was thrown away. @@ -2039,8 +2050,8 @@ describe('reassembly bounds', () => { const reassembler = new Reassembler({ log: silentLog, max: 10, - // One segment is 36 octets: 14 of short_message plus the two 11-octet addresses. - maxOctets: 80, + // One segment is 1036 octets: 14 of short_message, the two 11-octet addresses and its object. + maxOctets: 2100, now: () => 0, onLost: () => undefined, timeout: 60_000, diff --git a/test/session.test.ts b/test/session.test.ts index 5e936ae..9aab5e6 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -910,8 +910,8 @@ describe('receiving', () => { // The refusal a submission gets is the one submit_sm_resp defines, whichever command carried it. test('refuses a data_sm segment a server has no room for with the submit code', async t => { - // The two addresses are 22 octets, so the 6-octet UDH and its text are what overrun 30. - const smpp = await startServer(t, { maxOctets: 30 }); + // The two addresses and the objects are 1322 octets, so the 6-octet UDH and its text are what overrun 1330. + const smpp = await startServer(t, { maxOctets: 1330 }); const { session } = await connect(t, smpp, { bindType: 'transmitter' }); assert.ok(session); diff --git a/todo.md b/todo.md index 7b83a5f..2650bab 100644 --- a/todo.md +++ b/todo.md @@ -8,11 +8,11 @@ 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. +- [ ] **Bound the heap `HeldMessages` keeps, not only its count.** It holds up to 1000 messages the + application has not answered for up to 300 s, each as the PDUs it arrived in, undetached and + uncharged: one 200 KB PDU of 50,000 empty unknown TLVs is 15 MB of heap, and a completed + reassembly is up to `maxOctets`. A peer faster than an application answering asynchronously + holds gigabytes per session. From the stability review of #27. ## Status