CHANGELOG.md

@@ -37,6 +37,10 @@
   response command`.
 - `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
+  `broadcast_error_status`, the TLVs SMPP allows more than once in a PDU, read as an array of every
+  occurrence in wire order, even where only one arrived, and `objToPdu()` takes an array for them and
+  refuses a lone value. A PDU carrying two of one used to keep only the last.
 
 ## 0.5.0
 

MIGRATION.md

@@ -70,6 +70,9 @@ have for these:
 - Binary TLVs (`message_payload`, `network_error_code`, `callback_num` and the rest) were parsed into
   a hex string and written back as the ASCII of that string, so every round trip corrupted them.
   They are `Buffer`s in both directions now; drop any hex encoding of your own.
+- `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and
+  `broadcast_error_status` may repeat within a PDU, and each is an array of every occurrence in both
+  directions. 0.4.0 kept only the last one it read.
 - A body carried in the `message_payload` TLV was ignored, so the message arrived empty, and a
   `data_sm` was answered `ESME_RINVCMDID`, so a receipt thrown on one was lost silently. Both reach
   the application now: a receipt as `dlr`, answered for you, and a message as `sms` for you to answer.

README.md

@@ -608,6 +608,9 @@ if (isCommand(pduObj, 'submit_sm')) {
   and hands back the UDH where the PDU carries one.
 - `concatOf(pduObj)`: the `part`, `total` and `reference` a PDU declares and the `spelling` that
   carried them, `'udh'` or `'sar'`, or `undefined` for a whole message.
+- `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and
+  `broadcast_error_status` may repeat in one PDU, so each reads as an array of every occurrence in wire
+  order, and `objToPdu()` writes one TLV per element of the array it takes for them.
 - `messageClassOf(dataCoding)`: `0` for the flash class, `1`, `2` and `3` for the ME-, SIM- and
   TE-specific ones, `undefined` where that `data_coding`'s coding group carries no class.
 

src/defs/commands.ts

@@ -4,7 +4,6 @@ import { buffer, cstring, dest_address_array, int8, unsuccess_sme_array } from '
 type CommandSpec = {
 	id: number;
 	params?: Record<string, WireType>;
-	tlvMap?: Record<string, string>;
 };
 
 const bindParams = {
@@ -59,7 +58,6 @@ const specs = {
 	broadcast_sm_resp: {
 		id: 0x80000111,
 		params: { message_id: cstring },
-		tlvMap: { broadcast_area_identifier: 'failed_broadcast_area_identifier' },
 	},
 	cancel_broadcast_sm: {
 		id: 0x00000113,

src/defs/tlvs.ts

@@ -1,4 +1,4 @@
-import type { ParamValue, WireType } from './types.ts';
+import type { ParamValue, TlvValue, WireType } from './types.ts';
 import type { Result } from '../result.ts';
 import { tlv } from './types.ts';
 
@@ -103,13 +103,13 @@ export const tlvDefault: WireType = tlv.buffer;
 export type Tlv = {
 	tagId: number;
 	tagName: string | undefined;
-	tagValue: ParamValue;
+	tagValue: TlvValue;
 };
 
 export type TlvInput = {
 	/** Resolved from the record key; pass it for a tag the TLV table does not define. */
 	tagId?: number | undefined;
-	tagValue: ParamValue;
+	tagValue: TlvValue;
 };
 
 export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> {
@@ -135,30 +135,50 @@ export function writeTlvs(inputs: Record<string, TlvInput> | undefined): Result<
 
 		if (tag.err) return { err: tag.err };
 
-		const type = tlvsById[tag.tagId]?.type ?? tlvDefault;
-		const sized = type.size(input.tagValue);
+		const definition = tlvsById[tag.tagId];
+		const values = occurrences(input.tagValue, definition?.multiple === true);
 
-		if (sized.err) {
-			return { err: new Error(`TLV "${name}": ${sized.err.message}`) };
+		if (values.err) return { err: new Error(`TLV "${name}": ${values.err.message}`) };
+
+		for (const value of values.values) {
+			const chunk = writeTlv(tag.tagId, definition?.type ?? tlvDefault, value);
+
+			if (chunk.err) return { err: new Error(`TLV "${name}": ${chunk.err.message}`) };
+
+			chunks.push(chunk.chunk);
 		}
-
-		if (sized.size > 0xffff) {
-			return { err: new Error(`TLV "${name}": ${String(sized.size)} octets overflow the two octet length`) };
-		}
-
-		const chunk = Buffer.alloc(sized.size + 4);
-
-		chunk.writeUInt16BE(tag.tagId, 0);
-		chunk.writeUInt16BE(sized.size, 2);
-
-		const written = type.write(input.tagValue, chunk, 4);
-
-		if (written.err) {
-			return { err: new Error(`TLV "${name}": ${written.err.message}`) };
-		}
-
-		chunks.push(chunk);
 	}
 
 	return { chunks };
 }
+
+function occurrences(value: TlvValue, multiple: boolean): Result<{ values: ParamValue[] }> {
+	if (!multiple) {
+		return Array.isArray(value) ? { err: new Error('takes one value, not an array') } : { values: [value] };
+	}
+
+	if (!Array.isArray(value)) return { err: new Error('is repeatable, give an array of its values') };
+
+	if (value.length === 0) return { err: new Error('holds no values, omit it instead') };
+
+	return { values: value };
+}
+
+function writeTlv(tagId: number, type: WireType, value: ParamValue): Result<{ chunk: Buffer }> {
+	const sized = type.size(value);
+
+	if (sized.err) return { err: sized.err };
+
+	if (sized.size > 0xffff) {
+		return { err: new Error(`${String(sized.size)} octets overflow the two octet length`) };
+	}
+
+	const chunk = Buffer.alloc(sized.size + 4);
+
+	chunk.writeUInt16BE(tagId, 0);
+	chunk.writeUInt16BE(sized.size, 2);
+
+	const written = type.write(value, chunk, 4);
+
+	return written.err ? { err: written.err } : { chunk };
+}

src/defs/types.ts

@@ -14,6 +14,9 @@ export type UnsuccessSme = {
 
 export type ParamValue = Buffer | DestAddress[] | UnsuccessSme[] | number | string;
 
+/** A tag defined `multiple` holds every occurrence, in wire order; any other tag holds one value. */
+export type TlvValue = Buffer | Buffer[] | number | number[] | string;
+
 /**
  * One field on the wire. `read` reports how many octets it consumed so callers never have to
  * re-derive a length that could disagree with what was actually written.
@@ -26,7 +29,7 @@ export type WireType<T extends ParamValue = ParamValue> = {
 };
 
 /** Renders a parameter as text without ever falling back to "[object Object]". */
-export function paramText(value: ParamValue | undefined): string {
+export function paramText(value: ParamValue | TlvValue | undefined): string {
 	if (typeof value === 'string') return value;
 	if (typeof value === 'number') return value.toString();
 	if (Buffer.isBuffer(value)) return value.toString('latin1');

src/dlr.ts

@@ -1,5 +1,5 @@
 import type { MessageState } from './defs/constants.ts';
-import type { ParamValue } from './defs/types.ts';
+import type { TlvValue } from './defs/types.ts';
 import type { PduObject } from './pdu.ts';
 import type { SmsIdFormat } from './sms-id.ts';
 import { consts, constsById, hasUdh, messageTypeOf } from './defs/constants.ts';
@@ -150,7 +150,7 @@ const smeMessageTypes: readonly number[] = [
 	consts.ESM_CLASS.USER_ACKNOWLEDGEMENT,
 ];
 
-function nonEmptyText(value: ParamValue | undefined): string | undefined {
+function nonEmptyText(value: TlvValue | undefined): string | undefined {
 	return typeof value === 'string' && value !== '' ? value : undefined;
 }
 
@@ -178,7 +178,7 @@ function receiptBody(pduObj: PduObject): string {
 }
 
 function receiptId(
-	tlvId: ParamValue | undefined,
+	tlvId: TlvValue | undefined,
 	receipt: Receipt | undefined,
 	format: SmsIdFormat,
 ): string | undefined {
@@ -198,7 +198,7 @@ function isMessageState(name: string | undefined): name is MessageState {
 
 /** The state TLV wins where it names a state we know; an unnameable one leaves the body to say. */
 function receiptStatus(
-	tlvState: ParamValue | undefined,
+	tlvState: TlvValue | undefined,
 	receipt: Receipt | undefined,
 ): { statusId: number; statusMsg: MessageState | undefined } {
 	const scraped = receiptStates[receipt?.stat?.toUpperCase() ?? ''];

src/index.ts

@@ -68,7 +68,7 @@ export type { PduObject, PduObjectInput, TlvInput } from './pdu.ts';
 export type { PduHeader } from './pdu-refusal.ts';
 export type { SplitOptions } from './message.ts';
 export type { Tlv, TlvDefinition, TlvName } from './defs/tlvs.ts';
-export type { DestAddress, ParamValue, UnsuccessSme, WireType } from './defs/types.ts';
+export type { DestAddress, ParamValue, TlvValue, UnsuccessSme, WireType } from './defs/types.ts';
 
 /** The spec tables, grouped the way `larvitsmpp.defs` was in 0.4.0. */
 export { defs } from './defs/index.ts';

src/pdu.ts

@@ -1,6 +1,6 @@
 import type { CommandDefinition, CommandName, PduParams, PduParamsInput } from './defs/commands.ts';
 import type { ErrorName } from './defs/errors.ts';
-import type { ParamValue } from './defs/types.ts';
+import type { ParamValue, TlvValue } from './defs/types.ts';
 import type { PduHeader } from './pdu-refusal.ts';
 import type { Result, VoidResult } from './result.ts';
 import type { Tlv, TlvInput } from './defs/tlvs.ts';
@@ -262,11 +262,27 @@ export function objToPdu<C extends CommandName>(obj: PduObjectInput<C>): Result<
 	);
 }
 
-function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record<string, Tlv> }> {
-	const tlvs: Record<string, Tlv> = {};
-	let offset = start;
+type Repeats = { buffers: Map<string, Buffer[]>; numbers: Map<string, number[]> };
 
-	while (offset + 4 <= pdu.length) {
+/** A repeatable tag's occurrences so far, `value` appended; a value no TLV type reads is undefined. */
+function tlvValue(value: ParamValue, key: string, multiple: boolean | undefined, repeats: Repeats): TlvValue | undefined {
+	if (Buffer.isBuffer(value)) return multiple === true ? appended(repeats.buffers, key, value) : value;
+	if (typeof value === 'number') return multiple === true ? appended(repeats.numbers, key, value) : value;
+	if (typeof value === 'string' && multiple !== true) return value;
+
+	return undefined;
+}
+
+function appended<T>(lists: Map<string, T[]>, key: string, value: T): T[] {
+	const list = lists.get(key) ?? [];
+
+	list.push(value);
+	lists.set(key, list);
+
+	return list;
+}
+
+function readTlv(pdu: Buffer, offset: number, repeats: Repeats): Result<{ key: string; octets: number; tlv: Tlv }> {
 	const tagId = pdu.readUInt16BE(offset);
 	const tagLength = pdu.readUInt16BE(offset + 2);
 
@@ -279,13 +295,26 @@ function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: R
 
 	if (read.err) return { err: read.err };
 
-		tlvs[definition?.tag ?? tagId.toString()] = {
-			tagId,
-			tagName: definition?.tag,
-			tagValue: read.value,
-		};
+	const key = definition?.tag ?? tagId.toString();
+	const tagValue = tlvValue(read.value, key, definition?.multiple, repeats);
 
-		offset += 4 + tagLength;
+	if (tagValue === undefined) return { err: new Error(`TLV ${String(tagId)} read as a value no TLV holds`) };
+
+	return { key, octets: 4 + tagLength, tlv: { tagId, tagName: definition?.tag, tagValue } };
+}
+
+function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record<string, Tlv> }> {
+	const repeats: Repeats = { buffers: new Map(), numbers: new Map() };
+	const tlvs: Record<string, Tlv> = {};
+	let offset = start;
+
+	while (offset + 4 <= pdu.length) {
+		const read = readTlv(pdu, offset, repeats);
+
+		if (read.err) return { err: read.err };
+
+		tlvs[read.key] = read.tlv;
+		offset += read.octets;
 	}
 
 	return { offset, tlvs };

test/pdu.test.ts

@@ -463,6 +463,48 @@ describe('TLVs', () => {
 		assert.ok(err instanceof Error);
 	});
 
+	test('keeps every occurrence of a repeatable TLV, in wire order', () => {
+		const first = Buffer.from('0146709771337', 'hex');
+		const second = Buffer.from('0146701113311', 'hex');
+		const pduObj = decode(encode({
+			cmdName: 'submit_sm',
+			params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' },
+			tlvs: {
+				callback_num: { tagValue: [first, second] },
+				callback_num_pres_ind: { tagValue: [1] },
+			},
+		}));
+
+		assert.deepEqual(pduObj.tlvs.callback_num?.tagValue, [first, second]);
+		assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]);
+	});
+
+	test('reads the failed areas of a broadcast_sm_resp as broadcast_area_identifier', () => {
+		const areas = [Buffer.from('0001', 'hex'), Buffer.from('0002', 'hex')];
+		const pduObj = decode(encode({
+			cmdName: 'broadcast_sm_resp',
+			params: { message_id: '01a0d051-b588-76eb-a5c5-a8cb8b854e68' },
+			tlvs: { failed_broadcast_area_identifier: { tagValue: areas } },
+		}));
+
+		assert.deepEqual(pduObj.tlvs.broadcast_area_identifier?.tagValue, areas);
+	});
+
+	test('refuses a repeatable TLV given one value, and a lone TLV given several', () => {
+		const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' };
+
+		for (const tlvs of [
+			{ callback_num: { tagValue: Buffer.from('01', 'hex') } },
+			{ callback_num: { tagValue: [] } },
+			{ source_port: { tagValue: [1234, 1235] } },
+		]) {
+			const { buffer, err } = objToPdu({ cmdName: 'submit_sm', params, tlvs });
+
+			assert.equal(buffer, undefined);
+			assert.ok(err instanceof Error);
+		}
+	});
+
 	test('round-trips a receipt with message_state and receipted_message_id', () => {
 		const receipt = 'id:450 sub:001 dlvrd:1 submit date:1504031342 done date:1504031342 stat:DELIVRD err:0 text:xxx';
 		const pduObj = decode(encode({

todo.md

@@ -189,13 +189,6 @@ and is also what the panel ranked hardest — two methods, one answer.
 
 ### Correctness, ahead of everything below
 
-- [ ] **Read `multiple` in `parseTlvs()` and `writeTlvs()`, or delete it and `tlvMap`.** Five TLVs
-      declare `multiple: true` (`callback_num`, `callback_num_atag`, `callback_num_pres_ind`,
-      `broadcast_area_identifier`, `broadcast_error_status`) and nothing reads it; `parseTlvs()` keys
-      by tag name, so a peer sending two `callback_num` TLVs silently keeps the last. `tlvMap` on
-      `broadcast_sm_resp` is declared, set once and read nowhere. This is the "Dormant filters" row
-      of the 0.4.0 defect table in a new spelling — metadata that reads as a guarantee.
-
 - [ ] **Test that a multipart send which errors never fires `messageDlr`.** Goal 2 now says so and
       README promises it; `session-extras.test.ts` covers a drop *after* the send, not one during it.