From 224acca7d0452a0fed94ee4e4acbee4250cfc294 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 24 Sep 2026 00:37:10 +0200 Subject: [PATCH 1/3] Read and write every copy of a TLV SMPP lets repeat, and drop the dormant tlvMap --- CHANGELOG.md | 5 +++ MIGRATION.md | 3 ++ README.md | 2 + src/defs/commands.ts | 2 - src/defs/tlvs.ts | 88 +++++++++++++++++++++++++------------ src/dlr.ts | 8 ++-- src/index.ts | 2 +- src/pdu.ts | 17 ++++--- src/reassembly.ts | 11 +++-- test/pdu.test.ts | 25 +++++++++++ test/session-extras.test.ts | 2 +- todo.md | 7 --- 12 files changed, 120 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1606b0b..607bd46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,11 @@ 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` read as a list of every copy the peer sent, where only the last was kept, + and are written from a list. A single value for one of them, or a list for any other tag, is + refused. +- `tlvMap` is gone from the `broadcast_sm_resp` command definition. Nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index 4eaf30d..d08866b 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. +- A repeated `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, + `broadcast_area_identifier` or `broadcast_error_status` kept only its last copy. Each of those tags + is a list now, read and written, even where one copy arrives. - 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..cf29688 100644 --- a/README.md +++ b/README.md @@ -608,6 +608,8 @@ 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. +- A tag SMPP lets repeat, marked `multiple` in `tlvs` (`callback_num`, `broadcast_area_identifier` + and three more), is a list of every copy in wire order, and `objToPdu()` takes it as one. - `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..fd789cf 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -1,16 +1,19 @@ -import type { ParamValue, WireType } from './types.ts'; +import type { WireType } from './types.ts'; import type { Result } from '../result.ts'; import { tlv } from './types.ts'; +/** What one TLV on the wire reads as. */ +export type TlvScalar = Buffer | number | string; + export type TlvDefinition = { id: number; multiple?: boolean; tag: string; - type: WireType; + type: WireType; }; /** The constraint keys every definition to its own name, so a `tag` that drifts fails to compile. */ -const tlvSpecs = ( +const tlvSpecs = } }>( definitions: T, ): T => definitions; @@ -98,18 +101,21 @@ for (const definition of Object.values(specs)) { } /** Fallback for tags this table does not know: keep the raw octets. */ -export const tlvDefault: WireType = tlv.buffer; +export const tlvDefault: WireType = tlv.buffer; + +/** A list for a tag its definition marks `multiple`, one copy per list entry, and a single value otherwise. */ +export type TlvValue = TlvScalar | TlvScalar[]; 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 }> { @@ -126,6 +132,45 @@ export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number } return { tagId }; } +function copiesOf(name: string, definition: TlvDefinition | undefined, value: TlvValue): Result<{ copies: TlvScalar[] }> { + const multiple = definition?.multiple === true; + + if (Array.isArray(value) && multiple) return { copies: value }; + + if (!Array.isArray(value) && !multiple) return { copies: [value] }; + + return { + err: new Error(multiple + ? `TLV "${name}": the tag may repeat, so give its value as a list` + : `TLV "${name}": the tag may not repeat, so give its value alone rather than as a list`), + }; +} + +function writeTlv(name: string, tagId: number, type: WireType, value: TlvScalar): Result<{ chunk: Buffer }> { + const sized = type.size(value); + + if (sized.err) { + return { err: new Error(`TLV "${name}": ${sized.err.message}`) }; + } + + 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(tagId, 0); + chunk.writeUInt16BE(sized.size, 2); + + const written = type.write(value, chunk, 4); + + if (written.err) { + return { err: new Error(`TLV "${name}": ${written.err.message}`) }; + } + + return { chunk }; +} + /** Each TLV as its four octet header and the value the tag's own wire type writes. */ export function writeTlvs(inputs: Record | undefined): Result<{ chunks: Buffer[] }> { const chunks: Buffer[] = []; @@ -135,29 +180,18 @@ 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 listed = copiesOf(name, definition, input.tagValue); - if (sized.err) { - return { err: new Error(`TLV "${name}": ${sized.err.message}`) }; + if (listed.err) return { err: listed.err }; + + for (const value of listed.copies) { + const written = writeTlv(name, tag.tagId, definition?.type ?? tlvDefault, value); + + if (written.err) return { err: written.err }; + + chunks.push(written.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 }; diff --git a/src/dlr.ts b/src/dlr.ts index 1ef5b46..500b8ca 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -1,7 +1,7 @@ import type { MessageState } from './defs/constants.ts'; -import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; import type { SmsIdFormat } from './sms-id.ts'; +import type { TlvValue } from './defs/tlvs.ts'; import { consts, constsById, hasUdh, messageTypeOf } from './defs/constants.ts'; import { encodings } from './defs/encodings.ts'; import { messageOctets } from './message-body.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..48138db 100644 --- a/src/index.ts +++ b/src/index.ts @@ -67,7 +67,7 @@ export type { ErrorName } from './defs/errors.ts'; 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 { Tlv, TlvDefinition, TlvName, TlvScalar, TlvValue } from './defs/tlvs.ts'; export type { DestAddress, ParamValue, UnsuccessSme, WireType } from './defs/types.ts'; /** The spec tables, grouped the way `larvitsmpp.defs` was in 0.4.0. */ diff --git a/src/pdu.ts b/src/pdu.ts index 48d686a..27b5aaf 100644 --- a/src/pdu.ts +++ b/src/pdu.ts @@ -3,7 +3,7 @@ import type { ErrorName } from './defs/errors.ts'; import type { ParamValue } 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'; +import type { Tlv, TlvDefinition, TlvInput, TlvScalar, TlvValue } from './defs/tlvs.ts'; import { PduRefusedError, framingRefusal } from './pdu-refusal.ts'; import { cmds, commandNameById, respNameFor } from './defs/commands.ts'; import { hasUdh } from './defs/constants.ts'; @@ -262,6 +262,13 @@ export function objToPdu(obj: PduObjectInput): Result< ); } +/** A repeatable tag gathers every copy the peer sent; any other keeps the last. */ +function tlvValue(definition: TlvDefinition | undefined, earlier: Tlv | undefined, value: TlvScalar): TlvValue { + if (definition?.multiple !== true) return value; + + return [...Array.isArray(earlier?.tagValue) ? earlier.tagValue : [], value]; +} + function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record }> { const tlvs: Record = {}; let offset = start; @@ -279,11 +286,9 @@ 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 name = definition?.tag ?? tagId.toString(); + + tlvs[name] = { tagId, tagName: definition?.tag, tagValue: tlvValue(definition, tlvs[name], read.value) }; offset += 4 + tagLength; } diff --git a/src/reassembly.ts b/src/reassembly.ts index 98a7abc..5073834 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -2,7 +2,7 @@ import type { Concat } from './concat.ts'; import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; import type { SmppLog } from './log.ts'; -import type { Tlv } from './defs/tlvs.ts'; +import type { Tlv, TlvScalar } from './defs/tlvs.ts'; import { ExpiringGroups } from './expiring-groups.ts'; import { decodeMessage } from './message.ts'; import { messageOctets } from './message-body.ts'; @@ -52,6 +52,10 @@ type Group = { total: number; }; +function copied(value: TlvScalar): TlvScalar { + return Buffer.isBuffer(value) ? Buffer.from(value) : value; +} + /** Wire reads hand back views, so retaining one segment would pin the whole PDU it arrived in. */ function detach(pduObj: PduObject): PduObject { const params: Record = {}; @@ -62,9 +66,7 @@ function detach(pduObj: PduObject): PduObject { } for (const [name, tlv] of Object.entries(pduObj.tlvs)) { - tlvs[name] = Buffer.isBuffer(tlv.tagValue) - ? { ...tlv, tagValue: Buffer.from(tlv.tagValue) } - : tlv; + tlvs[name] = { ...tlv, tagValue: Array.isArray(tlv.tagValue) ? tlv.tagValue.map(copied) : copied(tlv.tagValue) }; } // short_message holds the same octets wherever it was not decoded, so one copy covers both. @@ -78,6 +80,7 @@ function detach(pduObj: PduObject): PduObject { // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. function sizeOf(value: unknown): number { if (Buffer.isBuffer(value)) return value.length; + if (Array.isArray(value)) return value.reduce((sum, copy) => sum + sizeOf(copy), 0); return typeof value === 'string' ? value.length : 0; } diff --git a/test/pdu.test.ts b/test/pdu.test.ts index cab91b7..3046978 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -420,6 +420,31 @@ describe('TLVs', () => { assert.deepEqual(rebuilt.tlvs.message_payload?.tagValue, payload); }); + test('reads every copy of a tag SMPP lets repeat, in wire order, and writes them back', () => { + const numbers = [Buffer.from('46701113311', 'latin1'), Buffer.from('46709771337', 'latin1')]; + const pduObj = decode(encode({ + cmdName: 'submit_sm', + params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, + tlvs: { callback_num: { tagValue: numbers }, callback_num_pres_ind: { tagValue: [1] } }, + })); + + assert.deepEqual(pduObj.tlvs.callback_num, { tagId: 0x0381, tagName: 'callback_num', tagValue: numbers }); + assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]); + }); + + test('refuses a single value for a tag SMPP lets repeat, and a list for one it does not', () => { + const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }; + const single = objToPdu({ + cmdName: 'submit_sm', + params, + tlvs: { callback_num: { tagValue: Buffer.from('46701113311', 'latin1') } }, + }); + const list = objToPdu({ cmdName: 'submit_sm', params, tlvs: { source_port: { tagValue: [1234] } } }); + + assert.match(single.err?.message ?? '', /TLV "callback_num": .*list/); + assert.match(list.err?.message ?? '', /TLV "source_port": .*list/); + }); + test('takes the tag id from the record key when the caller gives none', () => { const pduObj = decode(encode({ cmdName: 'deliver_sm', diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 36b0bdb..6ef2f8e 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -2963,7 +2963,7 @@ describe('message id notation', () => { const [dlr, pduObj] = await reported; assert.equal(dlr.smsId, sent.smsIds[0]); - assert.equal(paramText(pduObj.tlvs.receipted_message_id?.tagValue), '1a2b'); + assert.equal(pduObj.tlvs.receipted_message_id?.tagValue, '1a2b'); }); test('leaves the segment ids of a multipart send to merge as they are', async t => { 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. -- 2.52.0 From 9c83dd1d99e87c2b200e144427cb2f9473e07047 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 24 Sep 2026 00:42:03 +0200 Subject: [PATCH 2/3] Refuse an empty list for a repeatable TLV --- CHANGELOG.md | 3 ++- src/defs/tlvs.ts | 6 +++++- test/pdu.test.ts | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 607bd46..d64159f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,8 @@ 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` read as a list of every copy the peer sent, where only the last was kept, - and are written from a list. A single value for one of them, or a list for any other tag, is + and are written from a list. A single value or an empty list for one of them, or a list for any + other tag, is refused. - `tlvMap` is gone from the `broadcast_sm_resp` command definition. Nothing read it. diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index fd789cf..de7ac07 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -135,7 +135,11 @@ export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number } function copiesOf(name: string, definition: TlvDefinition | undefined, value: TlvValue): Result<{ copies: TlvScalar[] }> { const multiple = definition?.multiple === true; - if (Array.isArray(value) && multiple) return { copies: value }; + if (Array.isArray(value) && multiple) { + return value.length > 0 + ? { copies: value } + : { err: new Error(`TLV "${name}": give at least one value, or leave the tag out`) }; + } if (!Array.isArray(value) && !multiple) return { copies: [value] }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 3046978..459b7c9 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -432,16 +432,18 @@ describe('TLVs', () => { assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]); }); - test('refuses a single value for a tag SMPP lets repeat, and a list for one it does not', () => { + test('refuses a single value or an empty list for a tag SMPP lets repeat, and a list for one it does not', () => { const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }; const single = objToPdu({ cmdName: 'submit_sm', params, tlvs: { callback_num: { tagValue: Buffer.from('46701113311', 'latin1') } }, }); + const empty = objToPdu({ cmdName: 'submit_sm', params, tlvs: { callback_num: { tagValue: [] } } }); const list = objToPdu({ cmdName: 'submit_sm', params, tlvs: { source_port: { tagValue: [1234] } } }); assert.match(single.err?.message ?? '', /TLV "callback_num": .*list/); + assert.match(empty.err?.message ?? '', /TLV "callback_num": .*leave the tag out/); assert.match(list.err?.message ?? '', /TLV "source_port": .*list/); }); -- 2.52.0 From 59ff21a3d2f139118dbb381a53968048f5dd5c13 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 24 Sep 2026 00:42:11 +0200 Subject: [PATCH 3/3] File the panel's falsified doc claims and String(thrown) sites --- CHANGELOG.md | 3 +-- todo.md | 9 +++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d64159f..422684e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,7 @@ - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and `broadcast_error_status` read as a list of every copy the peer sent, where only the last was kept, and are written from a list. A single value or an empty list for one of them, or a list for any - other tag, is - refused. + other tag, is refused. - `tlvMap` is gone from the `broadcast_sm_resp` command definition. Nothing read it. ## 0.5.0 diff --git a/todo.md b/todo.md index 45bcbd6..e7fa3a4 100644 --- a/todo.md +++ b/todo.md @@ -378,6 +378,15 @@ and is also what the panel ranked hardest — two methods, one answer. is honest that the run was 2026-09-08. Nothing runs the peers on a schedule or before a tag, so the claim rots silently. Goals 1 and 9. +- [ ] **Correct the four doc claims the 2026-09-24 panel falsified.** `send-sms.ts:317` says this + library's own server waits for every segment before answering, where it answers each on + arrival; AGENTS.md counts nine goals where README lists ten; README's goals point at "the hard + rules below" and "the defect table below", both of which live in AGENTS.md. + +- [ ] **Route the three `new Error(String(thrown))` through `errorFrom()`.** `client.ts:100` and + `reconnect-loop.ts:84,140` convert a thrown value with `String()`, which throws for an object + whose `toString` throws — the case `errorFrom()` exists for, and hard rule 1. + ## Worth doing, not blocking - [ ] **Decide whether `alert_notification` reaches the application as more than `incomingPduObj`.** -- 2.52.0