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

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 | |
 

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;

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,

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);

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