From 10877c9f5f9a7a3a8a55d382e78326e4d75792c9 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 24 Sep 2026 00:13:38 +0200 Subject: [PATCH] Read every occurrence of a repeatable TLV, and drop the unread tlvMap --- CHANGELOG.md | 4 +++ MIGRATION.md | 3 ++ README.md | 3 ++ src/defs/commands.ts | 2 -- src/defs/tlvs.ts | 68 ++++++++++++++++++++++++++++---------------- src/defs/types.ts | 5 +++- src/dlr.ts | 8 +++--- src/index.ts | 2 +- src/pdu.ts | 63 +++++++++++++++++++++++++++++----------- test/pdu.test.ts | 42 +++++++++++++++++++++++++++ todo.md | 7 ----- 11 files changed, 151 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1606b0b..847371f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/MIGRATION.md b/MIGRATION.md index 4eaf30d..35b8b09 100644 --- a/MIGRATION.md +++ b/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. diff --git a/README.md b/README.md index fb5ce7f..2c5873e 100644 --- a/README.md +++ b/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. diff --git a/src/defs/commands.ts b/src/defs/commands.ts index 900a789..13ebf82 100644 --- a/src/defs/commands.ts +++ b/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; - tlvMap?: Record; }; 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, diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 79ff8c2..cae3d75 100644 --- a/src/defs/tlvs.ts +++ b/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 | 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 }; +} diff --git a/src/defs/types.ts b/src/defs/types.ts index cd601d8..93abf84 100644 --- a/src/defs/types.ts +++ b/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 = { }; /** 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'); diff --git a/src/dlr.ts b/src/dlr.ts index 1ef5b46..27ab4dd 100644 --- a/src/dlr.ts +++ b/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() ?? '']; diff --git a/src/index.ts b/src/index.ts index eea6081..d24f9d0 100644 --- a/src/index.ts +++ b/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'; diff --git a/src/pdu.ts b/src/pdu.ts index 48d686a..48dbcf8 100644 --- a/src/pdu.ts +++ b/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,30 +262,59 @@ export function objToPdu(obj: PduObjectInput): Result< ); } +type Repeats = { buffers: Map; numbers: Map }; + +/** 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(lists: Map, 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); + + if (offset + 4 + tagLength > pdu.length) { + return { err: new Error(`TLV ${String(tagId)} runs past the end of the PDU`) }; + } + + const definition = tlvsById[tagId]; + const read = (definition?.type ?? tlvDefault).read(pdu, offset + 4, tagLength); + + if (read.err) return { err: read.err }; + + const key = definition?.tag ?? tagId.toString(); + const tagValue = tlvValue(read.value, key, definition?.multiple, repeats); + + 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 }> { + const repeats: Repeats = { buffers: new Map(), numbers: new Map() }; const tlvs: Record = {}; let offset = start; while (offset + 4 <= pdu.length) { - const tagId = pdu.readUInt16BE(offset); - const tagLength = pdu.readUInt16BE(offset + 2); - - if (offset + 4 + tagLength > pdu.length) { - return { err: new Error(`TLV ${String(tagId)} runs past the end of the PDU`) }; - } - - const definition = tlvsById[tagId]; - const read = (definition?.type ?? tlvDefault).read(pdu, offset + 4, tagLength); + const read = readTlv(pdu, offset, repeats); if (read.err) return { err: read.err }; - tlvs[definition?.tag ?? tagId.toString()] = { - tagId, - tagName: definition?.tag, - tagValue: read.value, - }; - - offset += 4 + tagLength; + tlvs[read.key] = read.tlv; + offset += read.octets; } return { offset, tlvs }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index cab91b7..c2de212 100644 --- a/test/pdu.test.ts +++ b/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({ diff --git a/todo.md b/todo.md index 1276023..45bcbd6 100644 --- a/todo.md +++ b/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.