From 75d47945227699d3d31055a8b11f006ae45c3214 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:12:09 +0200 Subject: [PATCH 01/14] Type each known TLV's value by its tag, on read and on write --- CHANGELOG.md | 5 ++ README.md | 5 ++ src/defs/tlvs.ts | 94 ++++++++++++++++++++++++++++--------- src/index.ts | 4 +- src/pdu.ts | 12 ++--- src/retained-pdu.ts | 6 +-- src/server.ts | 4 +- src/session.ts | 4 +- src/sms.ts | 4 +- test/pdu.test.ts | 36 +++++++++++--- test/session-extras.test.ts | 2 +- todo.md | 8 ---- 12 files changed, 131 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a0dbda..f286cb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,11 @@ `Buffer.isBuffer()` or `typeof` check written for 0.5.0 now reads it as absent. Read `tagValue[0]` for the first occurrence. `objToPdu()`, `session.send()` and `session.sendReturn()` take `{ tagValue: [value] }` for them and refuse a lone value before anything goes out. +- `pduObj.tlvs` and every `tlvs` input are typed per tag, as `Tlvs` and `TlvInputs`: + `receipted_message_id` reads as a `string`, `callback_num` as a `Buffer[]`, and a value of the wrong + type for its tag fails to compile. An unknown tag reads as a `Buffer` under its decimal id. Code + that narrowed `tagValue` with `typeof` still compiles; a `Record` annotation does not, + so use `Tlvs`. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/README.md b/README.md index aee9db5..12673d4 100644 --- a/README.md +++ b/README.md @@ -613,6 +613,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. +- `pduObj.tlvs` is typed per tag, as `Tlvs`: `receipted_message_id` a string, `message_state` a + number, `message_payload` a `Buffer`. A tag the table does not define is a `Buffer` keyed by its + decimal id, `tlvs['5142']`. - `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: `number[]` for `callback_num_pres_ind` and `broadcast_error_status`, `Buffer[]` for the rest. @@ -627,6 +630,8 @@ if (isCommand(pduObj, 'submit_sm')) { naming the character, its code point and where it is. - Every text field is latin1: addresses, `system_id`, `message_id`, `service_type` and the C-Octet String TLVs. A character past `U+00FF` is refused, as is a `U+0000` in a C-Octet String. +- `tlvs` is typed per tag, as `TlvInputs`, so `{ message_state: { tagValue: 'ENROUTE' } }` fails to + compile. A tag the table does not define takes a `tagId`. - The five repeatable TLVs take an array, written as one TLV per element; a lone value or an empty array is refused. - A `Buffer` goes out exactly as given under any `data_coding`: binary payloads, hand-built user diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 7959625..da2472f 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -79,15 +79,18 @@ const specs = tlvSpecs({ its_session_info: { id: 0x1383, tag: 'its_session_info', type: tlv.buffer }, }); -export type TlvName = keyof typeof specs; +type Specs = typeof specs; -export const tlvs: Record & Record = { - ...specs, - // Alternate spellings; the definition behind each keeps its canonical name. +export type TlvName = keyof Specs; + +// Alternate spellings; the definition behind each keeps its canonical name. +const alternates = { alert_on_msg_delivery: specs.alert_on_message_delivery, failed_broadcast_area_identifier: specs.broadcast_area_identifier, }; +export const tlvs: Record & Record = { ...specs, ...alternates }; + export const tlvsById: Record = {}; for (const definition of Object.values(specs)) { @@ -97,11 +100,28 @@ 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 type Tlv = { - tagId: number; - tagName: string | undefined; - tagValue: TlvValue; -}; +type Repeated = Specs[K] extends { multiple: true } ? V[] : V; + +type WireValue = Specs[K]['type']['default']; + +export type TlvReadValue = Repeated>; + +/** A lone text field also takes a number, and a lone octet field text, which goes out as latin1. */ +export type TlvWriteValue = Specs[K] extends { multiple: true } ? TlvReadValue + : WireValue extends number ? number + : WireValue extends string ? number | string + : Buffer | number | string; + +type OneTlv = { tagId: number; tagName: K; tagValue: TlvReadValue }; + +export type Tlv = { [N in K]: OneTlv }[K]; + +/** A tag the TLV table does not define, keyed by its decimal id. */ +export type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; + +type KnownTlvs = { [K in TlvName]?: OneTlv }; + +export type Tlvs = KnownTlvs & Record<`${number}`, UnknownTlv>; export type TlvInput = { /** Resolved from the record key; pass it for a tag the TLV table does not define. */ @@ -109,6 +129,14 @@ export type TlvInput = { tagValue: TlvValue; }; +type Alternate = keyof typeof alternates; + +type Canonical = K extends Alternate ? (typeof alternates)[K]['tag'] : K; + +export type TlvInputs = { + [K in Alternate | TlvName]?: { tagId?: number | undefined; tagValue: TlvWriteValue> }; +} & Record; + export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { const tagId = input.tagId ?? tlvs[name]?.id; @@ -198,10 +226,38 @@ function readTlv(pdu: Buffer, offset: number): Result<{ octets: number; occurren return { occurrence: { definition, tagId, value: read.value }, octets: 4 + tagLength }; } +function isTlvName(name: string): name is TlvName { + return Object.hasOwn(specs, name); +} + +function readsAs(definition: TlvDefinition, value: unknown): boolean { + const kind = definition.type.default; + const fits = (one: unknown): boolean => typeof one === typeof kind && Buffer.isBuffer(one) === Buffer.isBuffer(kind); + + return definition.multiple === true ? Array.isArray(value) && value.every(fits) : fits(value); +} + +function isTlvShape(tlv: unknown): tlv is { tagId: number; tagName: unknown; tagValue: unknown } { + return typeof tlv === 'object' && tlv !== null && 'tagId' in tlv && typeof tlv.tagId === 'number' + && 'tagName' in tlv && 'tagValue' in tlv; +} + +function isTlv(key: string, tlv: unknown): boolean { + if (!isTlvShape(tlv)) return false; + if (tlv.tagName === undefined) return /^\d+$/.test(key) && Buffer.isBuffer(tlv.tagValue); + + return tlv.tagName === key && isTlvName(key) && readsAs(specs[key], tlv.tagValue); +} + +/** Every entry keyed by its tag name, or an unknown tag by its decimal id, holding what its table type reads. */ +export function isTlvs(record: Record): record is Tlvs { + return Object.entries(record).every(([key, tlv]) => isTlv(key, tlv)); +} + /** Keyed by tag name, a repeatable tag listing every occurrence in wire order and any other keeping its last. */ -function keyedTlvs(occurrences: Occurrence[]): Record { +function keyedTlvs(occurrences: Occurrence[]): Result<{ tlvs: Tlvs }> { const repeated = new Map(); - const tlvs: Record = {}; + const tlvs: Record = {}; for (const { definition, tagId, value } of occurrences) { const key = definition?.tag ?? tagId.toString(); @@ -217,19 +273,13 @@ function keyedTlvs(occurrences: Occurrence[]): Record { } for (const [key, { tagId, values }] of repeated) { - const buffers = values.filter(value => Buffer.isBuffer(value)); - - tlvs[key] = { - tagId, - tagName: key, - tagValue: buffers.length === values.length ? buffers : values.filter(value => typeof value === 'number'), - }; + tlvs[key] = { tagId, tagName: key, tagValue: values }; } - return tlvs; + return isTlvs(tlvs) ? { tlvs } : { err: new Error('A TLV did not read as its table type') }; } -export function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record }> { +export function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Tlvs }> { const occurrences: Occurrence[] = []; let offset = start; @@ -242,5 +292,7 @@ export function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; offset += read.octets; } - return { offset, tlvs: keyedTlvs(occurrences) }; + const keyed = keyedTlvs(occurrences); + + return keyed.err ? { err: keyed.err } : { offset, tlvs: keyed.tlvs }; } diff --git a/src/index.ts b/src/index.ts index d24f9d0..96cf427 100644 --- a/src/index.ts +++ b/src/index.ts @@ -64,10 +64,10 @@ export type { CommandName, PduParams, PduParamsInput } from './defs/commands.ts' export type { ConstGroup, MessageState, SubmitMessagingMode } from './defs/constants.ts'; export type { Encoding, EncodingName, Unencodable } from './defs/encodings.ts'; export type { ErrorName } from './defs/errors.ts'; -export type { PduObject, PduObjectInput, TlvInput } from './pdu.ts'; +export type { PduObject, PduObjectInput, TlvInput, TlvInputs } 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, TlvReadValue, TlvWriteValue, Tlvs, UnknownTlv } from './defs/tlvs.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. */ diff --git a/src/pdu.ts b/src/pdu.ts index 8ba806f..be649db 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 { TlvInput, TlvInputs, Tlvs } 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'; @@ -23,7 +23,7 @@ export type PduObjectInput = { cmdStatus?: ErrorName; params?: PduParamsInput; seqNr?: number; - tlvs?: Record | undefined; + tlvs?: TlvInputs | undefined; }; /** @@ -43,10 +43,10 @@ export type PduObject = { * the peer put in `message_payload` is not here; `messageOctets()` is what reads either. */ shortMessageOctets: Buffer | undefined; - tlvs: Record; + tlvs: Tlvs; }; -export type { TlvInput }; +export type { TlvInput, TlvInputs }; const respBit = 0x80000000; @@ -293,7 +293,7 @@ function readOptionalParams( pdu: Buffer, start: number, afterShortMessage: boolean, -): Result<{ tlvs: Record }> { +): Result<{ tlvs: Tlvs }> { const plain = parseTlvs(pdu, start); if (!plain.err && plain.offset === pdu.length) return { tlvs: plain.tlvs }; @@ -418,7 +418,7 @@ export function pduReturn( pdu: Buffer | PduObject, status: ErrorName = 'ESME_ROK', params: Record = {}, - tlvs?: Record, + tlvs?: TlvInputs, ): Result<{ buffer: Buffer }> { if (Buffer.isBuffer(pdu)) { const parsed = pduToObj(pdu); diff --git a/src/retained-pdu.ts b/src/retained-pdu.ts index 641731a..e42647d 100644 --- a/src/retained-pdu.ts +++ b/src/retained-pdu.ts @@ -1,12 +1,12 @@ import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; -import type { Tlv } from './defs/tlvs.ts'; import { detachedTlv, tlvOctets } from './defs/types.ts'; +import { isTlvs } from './defs/tlvs.ts'; /** Wire reads hand back views, so retaining one PDU would pin the whole chunk it arrived in. */ export function detach(pduObj: PduObject): PduObject { const params: Record = {}; - const tlvs: Record = {}; + const tlvs: Record = {}; for (const [name, value] of Object.entries(pduObj.params)) { params[name] = Buffer.isBuffer(value) ? Buffer.from(value) : value; @@ -21,7 +21,7 @@ export function detach(pduObj: PduObject): PduObject { ? params.short_message : pduObj.shortMessageOctets && Buffer.from(pduObj.shortMessageOctets); - return { ...pduObj, params, shortMessageOctets: octets, tlvs }; + return { ...pduObj, params, shortMessageOctets: octets, tlvs: isTlvs(tlvs) ? tlvs : pduObj.tlvs }; } // Measured heap beyond the octets, so a PDU of empty fields or empty TLVs is not free. diff --git a/src/server.ts b/src/server.ts index 7cad123..0728637 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,5 +1,5 @@ import type { CloseOptions, OnRequest } from './session-options.ts'; -import type { PduObject, TlvInput } from './pdu.ts'; +import type { PduObject, TlvInputs } from './pdu.ts'; import type { Result, VoidResult } from './result.ts'; import type { Server as NetServer, Socket } from 'node:net'; import type { Server as TlsServer, TlsOptions } from 'node:tls'; @@ -162,7 +162,7 @@ async function authenticate( } /** An ESME reads a missing sc_interface_version as this SMSC having none. */ -function bindRespTlvs(session: Session, options: ServerOptions): Record | undefined { +function bindRespTlvs(session: Session, options: ServerOptions): TlvInputs | undefined { if (!session.acceptsOptionalParams()) return undefined; return { diff --git a/src/session.ts b/src/session.ts index 16b9775..71370c9 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,7 +1,7 @@ import type { ErrorName } from './defs/errors.ts'; import type { MessageDlr } from './dlr-merger.ts'; import type { ParamValue } from './defs/types.ts'; -import type { PduObject, PduObjectInput, TlvInput } from './pdu.ts'; +import type { PduObject, PduObjectInput, TlvInputs } from './pdu.ts'; import type { PduRefusedError } from './pdu-refusal.ts'; import type { BindType, CloseOptions, LinkEnd, ReconnectOptions, SendOptions, SessionEvents, SessionOptions } from './session-options.ts'; import type { Result, VoidResult } from './result.ts'; @@ -175,7 +175,7 @@ export class Session extends EventEmitter { pdu: PduObject, status: ErrorName = 'ESME_ROK', params: Record = {}, - tlvs?: Record, + tlvs?: TlvInputs, ): Promise { return Promise.resolve( this.answer(pduReturn(pdu, status, params, tlvs), pdu.cmdName, pdu.seqNr), diff --git a/src/sms.ts b/src/sms.ts index 4635421..dc57106 100644 --- a/src/sms.ts +++ b/src/sms.ts @@ -1,6 +1,6 @@ import type { ErrorName } from './defs/errors.ts'; import type { MessageState } from './defs/constants.ts'; -import type { PduObject, PduObjectInput, TlvInput } from './pdu.ts'; +import type { PduObject, PduObjectInput, TlvInputs } from './pdu.ts'; import type { Result, VoidResult } from './result.ts'; import type { Session } from './session.ts'; import { UnansweredError } from './unanswered-error.ts'; @@ -179,7 +179,7 @@ function receiptText(sms: Sms, smsId: string, status: MessageState): string { ].join(' '); } -function receiptTlvs(smsId: string, status: MessageState): Record { +function receiptTlvs(smsId: string, status: MessageState): TlvInputs { return { message_state: { tagValue: consts.MESSAGE_STATE[status] }, receipted_message_id: { tagValue: smsId }, diff --git a/test/pdu.test.ts b/test/pdu.test.ts index c2de212..1d9b433 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -475,6 +475,10 @@ describe('TLVs', () => { }, })); + const firstRead: Buffer | undefined = pduObj.tlvs.callback_num?.tagValue[0]; + const presentation: number | undefined = pduObj.tlvs.callback_num_pres_ind?.tagValue[0]; + + assert.deepEqual([firstRead, presentation], [first, 1]); assert.deepEqual(pduObj.tlvs.callback_num?.tagValue, [first, second]); assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]); }); @@ -493,18 +497,38 @@ describe('TLVs', () => { 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] } }, + for (const { buffer, err } of [ + // @ts-expect-error callback_num repeats, so it takes an array + objToPdu({ cmdName: 'submit_sm', params, tlvs: { callback_num: { tagValue: Buffer.from('01', 'hex') } } }), + objToPdu({ cmdName: 'submit_sm', params, tlvs: { callback_num: { tagValue: [] } } }), + // @ts-expect-error source_port does not repeat + objToPdu({ cmdName: 'submit_sm', params, tlvs: { source_port: { tagValue: [1234, 1235] } } }), ]) { - const { buffer, err } = objToPdu({ cmdName: 'submit_sm', params, tlvs }); - assert.equal(buffer, undefined); assert.ok(err instanceof Error); } }); + test('types each known TLV by its tag, and an unknown one as octets', () => { + const pduObj = decode(encode({ + cmdName: 'deliver_sm', + params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, + tlvs: { + 5142: { tagId: 5142, tagValue: Buffer.from('01', 'hex') }, + message_state: { tagValue: 2 }, + receipted_message_id: { tagValue: '0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15' }, + }, + })); + const id: string | undefined = pduObj.tlvs.receipted_message_id?.tagValue; + const state: number | undefined = pduObj.tlvs.message_state?.tagValue; + const unknown: Buffer | undefined = pduObj.tlvs['5142']?.tagValue; + + assert.deepEqual([id, state, unknown], ['0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15', 2, Buffer.from('01', 'hex')]); + + // @ts-expect-error message_state is an integer + assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { message_state: { tagValue: 'ENROUTE' } } }).err); + }); + 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/test/session-extras.test.ts b/test/session-extras.test.ts index fa52f1a..43c29ae 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1808,7 +1808,7 @@ describe('where a segment says it is concatenated', () => { }); test('reads no concatenation where a sar_* TLV is missing', () => { - const lone = { + const lone: PduObject = { ...sarSegment(5, 1, 2), tlvs: { sar_msg_ref_num: { tagId: 0x020c, tagName: 'sar_msg_ref_num', tagValue: 5 } }, }; diff --git a/todo.md b/todo.md index ee74247..2a11169 100644 --- a/todo.md +++ b/todo.md @@ -189,14 +189,6 @@ and is also what the panel ranked hardest — two methods, one answer. ### Correctness, ahead of everything below -- [ ] **Type each known TLV's value by its tag, on read and on write, before 0.6.0 is cut.** Every - `tagValue` is `TlvValue`, so `{ callback_num: { tagValue: buf } }` compiles and is refused - only at runtime, and reading `receipted_message_id` has to narrow out arrays it can never - hold. Derive the types from the specs' own wire types and `multiple` flag. It has to land in - the same minor as the arrays, or narrowing the types is a second break. A test compiles the - CHANGELOG's `tagValue[0]` advice, which the union does not type-check today. From the - product-owner review of #25. - - [ ] **Settle what a repeated tag not marked `multiple` reads as, and pin it in a test.** A vendor tag or a known single-value tag a peer sends twice keeps the last occurrence and drops the rest silently, which goal 3 argues against; listing it would change every such tag's shape. -- 2.52.0 From 939eda7269fc200d0e30d17c0e723246b811722e Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:18:08 +0200 Subject: [PATCH 02/14] Key a TLV input like the read side, drop tagId, copy TLV octets at parse, and export only Tlvs, TlvInputs and Tlv --- CHANGELOG.md | 10 ++++-- MIGRATION.md | 3 ++ README.md | 5 +-- src/defs/tlvs.ts | 63 +++++++++++++++++----------------- src/defs/types.ts | 10 ------ src/index.ts | 4 +-- src/pdu.ts | 47 +++++++++---------------- src/retained-pdu.ts | 10 ++---- test/dlr.test.ts | 4 +-- test/interop.test.ts | 4 +-- test/operator-receipts.test.ts | 6 ++-- test/pdu.test.ts | 63 ++++++++++++++++++++++------------ test/unsendable.test.ts | 27 +++------------ 13 files changed, 118 insertions(+), 138 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f286cb2..4b74c7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,9 +60,13 @@ `{ tagValue: [value] }` for them and refuse a lone value before anything goes out. - `pduObj.tlvs` and every `tlvs` input are typed per tag, as `Tlvs` and `TlvInputs`: `receipted_message_id` reads as a `string`, `callback_num` as a `Buffer[]`, and a value of the wrong - type for its tag fails to compile. An unknown tag reads as a `Buffer` under its decimal id. Code - that narrowed `tagValue` with `typeof` still compiles; a `Record` annotation does not, - so use `Tlvs`. + type for its tag fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote + `Record` or `Record`; `TlvInput` is gone. + + **A TLV input is keyed by its name, or by its decimal id where the table names none, and `tagId` + is refused.** Write `{ 5142: { tagValue } }` for a vendor tag, not `{ vendor: { tagId: 5142, … } }`. + A name and a `tagId` could disagree, and `{ message_state: { tagId: 5, … } }` went out as tag 5. + A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index 35b8b09..b21f6af 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -30,6 +30,9 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.MESSAGING_MODE`**, which also names `SMSC_DEFAULT`. They are bits 1-0 of `esm_class`, not whole values of it. Read them from the new group, or pass `messagingMode` to `sendSms()`. A stale `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. +- **A TLV is keyed by its name, or by its decimal id where the table names none**, and `tagId` on + an input is refused: `{ 5142: { tagValue } }`, not `{ vendor: { tagId: 5142, tagValue } }`. + Unknown tags read back under the same decimal key. - **The `error` event is `sessionError`**, and `serverError` on the server handle. - **`log`** takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods instead of a `larvitutils` one, and is silent by default: [README](README.md#logging). diff --git a/README.md b/README.md index 12673d4..38aefcb 100644 --- a/README.md +++ b/README.md @@ -630,8 +630,9 @@ if (isCommand(pduObj, 'submit_sm')) { naming the character, its code point and where it is. - Every text field is latin1: addresses, `system_id`, `message_id`, `service_type` and the C-Octet String TLVs. A character past `U+00FF` is refused, as is a `U+0000` in a C-Octet String. -- `tlvs` is typed per tag, as `TlvInputs`, so `{ message_state: { tagValue: 'ENROUTE' } }` fails to - compile. A tag the table does not define takes a `tagId`. +- `tlvs` is keyed and typed like `pduObj.tlvs`, as `TlvInputs`: `{ message_state: { tagValue: 2 } }`, + or `{ 5142: { tagValue: octets } }` for a tag the table does not define. Any other key, a decimal + id the table names, and a `tagId` are refused. - The five repeatable TLVs take an array, written as one TLV per element; a lone value or an empty array is refused. - A `Buffer` goes out exactly as given under any `data_coding`: binary payloads, hand-built user diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index da2472f..6417009 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -104,55 +104,54 @@ type Repeated = Specs[K] extends { multiple: true } ? V[] type WireValue = Specs[K]['type']['default']; -export type TlvReadValue = Repeated>; +type ReadValue = Repeated>; /** A lone text field also takes a number, and a lone octet field text, which goes out as latin1. */ -export type TlvWriteValue = Specs[K] extends { multiple: true } ? TlvReadValue +type WriteValue = Specs[K] extends { multiple: true } ? ReadValue : WireValue extends number ? number : WireValue extends string ? number | string : Buffer | number | string; -type OneTlv = { tagId: number; tagName: K; tagValue: TlvReadValue }; +type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; -export type Tlv = { [N in K]: OneTlv }[K]; +/** A tag the TLV table does not define. */ +type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; -/** A tag the TLV table does not define, keyed by its decimal id. */ -export type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; +/** Keyed by tag name, or by its decimal id where the table defines no name. */ +export type Tlvs = { [K in TlvName]?: KnownTlv } & Record<`${number}`, UnknownTlv>; -type KnownTlvs = { [K in TlvName]?: OneTlv }; - -export type Tlvs = KnownTlvs & Record<`${number}`, UnknownTlv>; - -export type TlvInput = { - /** Resolved from the record key; pass it for a tag the TLV table does not define. */ - tagId?: number | undefined; - tagValue: TlvValue; -}; +export type Tlv = { [K in TlvName]: KnownTlv }[TlvName] | UnknownTlv; type Alternate = keyof typeof alternates; type Canonical = K extends Alternate ? (typeof alternates)[K]['tag'] : K; -export type TlvInputs = { - [K in Alternate | TlvName]?: { tagId?: number | undefined; tagValue: TlvWriteValue> }; -} & Record; +/** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ +export type TlvInputs = { [K in Alternate | TlvName]?: { tagValue: WriteValue> } } + & Record<`${number}`, { tagValue: Buffer | number | string }>; -export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { - const tagId = input.tagId ?? tlvs[name]?.id; - - if (tagId === undefined) { - return { err: new Error(`TLV "${name}": unknown tag name, give it a tagId`) }; +function tagIdOf(name: string, input: object): Result<{ tagId: number }> { + if ('tagId' in input) { + return { err: new Error(`TLV "${name}": key it by its name, or a tag the table does not define by its decimal id, instead of giving a tagId`) }; } - if (!Number.isInteger(tagId) || tagId < 0 || tagId > 0xFFFF) { - return { err: new Error(`TLV "${name}": tagId ${String(tagId)} out of range 0-65535`) }; + if (Object.hasOwn(tlvs, name)) return { tagId: tlvs[name]?.id ?? 0 }; + + if (!/^(0|[1-9]\d*)$/.test(name)) { + return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; } - return { tagId }; + const tagId = Number(name); + + if (tagId > 0xFFFF) return { err: new Error(`TLV "${name}": tag id out of range 0-65535`) }; + + const known = tlvsById[tagId]; + + return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; } /** 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[] }> { +export function writeTlvs(inputs: TlvInputs | undefined): Result<{ chunks: Buffer[] }> { const chunks: Buffer[] = []; for (const [name, input] of Object.entries(inputs ?? {})) { @@ -223,7 +222,10 @@ function readTlv(pdu: Buffer, offset: number): Result<{ octets: number; occurren if (read.err) return { err: read.err }; - return { occurrence: { definition, tagId, value: read.value }, octets: 4 + tagLength }; + // Copied, so holding a TLV pins no more than its own octets. + const value = Buffer.isBuffer(read.value) ? Buffer.from(read.value) : read.value; + + return { occurrence: { definition, tagId, value }, octets: 4 + tagLength }; } function isTlvName(name: string): name is TlvName { @@ -249,8 +251,7 @@ function isTlv(key: string, tlv: unknown): boolean { return tlv.tagName === key && isTlvName(key) && readsAs(specs[key], tlv.tagValue); } -/** Every entry keyed by its tag name, or an unknown tag by its decimal id, holding what its table type reads. */ -export function isTlvs(record: Record): record is Tlvs { +function isTlvs(record: Record): record is Tlvs { return Object.entries(record).every(([key, tlv]) => isTlv(key, tlv)); } @@ -276,7 +277,7 @@ function keyedTlvs(occurrences: Occurrence[]): Result<{ tlvs: Tlvs }> { tlvs[key] = { tagId, tagName: key, tagValue: values }; } - return isTlvs(tlvs) ? { tlvs } : { err: new Error('A TLV did not read as its table type') }; + return isTlvs(tlvs) ? { tlvs } : { err: new Error('A TLV did not read as its table type, a defect in this library') }; } export function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Tlvs }> { diff --git a/src/defs/types.ts b/src/defs/types.ts index 7f3acf5..6b70ab1 100644 --- a/src/defs/types.ts +++ b/src/defs/types.ts @@ -32,16 +32,6 @@ export function tlvOctets(value: TlvValue): number { return octets; } -/** A value holding no view into the PDU it was read from. */ -export function detachedTlv(value: TlvValue): TlvValue { - if (Buffer.isBuffer(value)) return Buffer.from(value); - if (!Array.isArray(value)) return value; - - const buffers = value.filter(one => Buffer.isBuffer(one)); - - return buffers.length === value.length ? buffers.map(one => Buffer.from(one)) : value; -} - /** * 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. diff --git a/src/index.ts b/src/index.ts index 96cf427..837f3f0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -64,10 +64,10 @@ export type { CommandName, PduParams, PduParamsInput } from './defs/commands.ts' export type { ConstGroup, MessageState, SubmitMessagingMode } from './defs/constants.ts'; export type { Encoding, EncodingName, Unencodable } from './defs/encodings.ts'; export type { ErrorName } from './defs/errors.ts'; -export type { PduObject, PduObjectInput, TlvInput, TlvInputs } from './pdu.ts'; +export type { PduObject, PduObjectInput, TlvInputs } from './pdu.ts'; export type { PduHeader } from './pdu-refusal.ts'; export type { SplitOptions } from './message.ts'; -export type { Tlv, TlvDefinition, TlvName, TlvReadValue, TlvWriteValue, Tlvs, UnknownTlv } from './defs/tlvs.ts'; +export type { Tlv, TlvDefinition, TlvName, Tlvs } from './defs/tlvs.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. */ diff --git a/src/pdu.ts b/src/pdu.ts index be649db..fd0651a 100644 --- a/src/pdu.ts +++ b/src/pdu.ts @@ -3,14 +3,14 @@ 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 { TlvInput, TlvInputs, Tlvs } from './defs/tlvs.ts'; +import type { TlvInputs, Tlvs } 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'; import { decodeMessage, encodeBody } from './message.ts'; import { errorNameById, errors, isErrorName } from './defs/errors.ts'; import { paramNumber, valueText } from './defs/types.ts'; -import { parseTlvs, tagIdOf, tlvs, writeTlvs } from './defs/tlvs.ts'; +import { parseTlvs, writeTlvs } from './defs/tlvs.ts'; /** The highest sequence number this library hands out; SMPP 3.4 4.7.1 reserves 0x7fffffff. */ export const maxSeqNr = 2147483646; @@ -46,7 +46,7 @@ export type PduObject = { tlvs: Tlvs; }; -export type { TlvInput, TlvInputs }; +export type { TlvInputs }; const respBit = 0x80000000; @@ -68,30 +68,13 @@ export function isCommand( type ResolvedBody = { params: Record; - tlvs: Record | undefined; + tlvs: TlvInputs | undefined; }; function codingOf(params: Record): number | undefined { return typeof params.data_coding === 'number' ? params.data_coding : undefined; } -type CarriedBody = { name: string; text: string; tlv: TlvInput }; - -/** Every entry carrying body text, under whatever names their tagIds are keyed to. */ -function carriedBodies(input: Record | undefined): CarriedBody[] { - const carried: CarriedBody[] = []; - - for (const [name, tlv] of Object.entries(input ?? {})) { - const tag = tagIdOf(name, tlv); - - if (!tag.err && tag.tagId === tlvs.message_payload.id && typeof tlv.tagValue === 'string') { - carried.push({ name, text: tlv.tagValue, tlv }); - } - } - - return carried; -} - /** messageOctets() reads short_message wherever it holds an octet, and the TLV only where it does not. */ function carriesOctets(value: ParamValue | undefined): boolean { return Buffer.isBuffer(value) && value.length > 0; @@ -105,19 +88,21 @@ function writtenBody(definition: CommandDefinition, value: ParamValue | undefine /** Encoded in place, settling data_coding where `settles` says no mandatory field will carry it. */ function resolveCarried( resolved: ResolvedBody, - inputs: Record | undefined, + inputs: TlvInputs | undefined, dataCoding: number | undefined, settles: boolean, ): VoidResult { - for (const carried of carriedBodies(inputs)) { - const encoded = encodeBody(carried.text, dataCoding); + const text = inputs?.message_payload?.tagValue; - if (encoded.err) return { err: new Error(`TLV "${carried.name}": ${encoded.err.message}`) }; + if (typeof text !== 'string') return {}; - if (settles) resolved.params.data_coding = encoded.dataCoding; + const encoded = encodeBody(text, dataCoding); - resolved.tlvs = { ...resolved.tlvs, [carried.name]: { ...carried.tlv, tagValue: encoded.buffer } }; - } + if (encoded.err) return { err: new Error(`TLV "message_payload": ${encoded.err.message}`) }; + + if (settles) resolved.params.data_coding = encoded.dataCoding; + + resolved.tlvs = { ...resolved.tlvs, message_payload: { tagValue: encoded.buffer } }; return {}; } @@ -125,7 +110,7 @@ function resolveCarried( /** data_coding names the alphabet of the body, and short_message settles it where it carries octets. */ function resolveBody( params: Record, - tlvs: Record | undefined, + tlvs: TlvInputs | undefined, definition: CommandDefinition, ): Result { const message = writtenBody(definition, params.short_message); @@ -197,7 +182,7 @@ function buildBody( cmdName: CommandName, cmdStatus: ErrorName, params: Record, - tlvs: Record | undefined, + tlvs: TlvInputs | undefined, ): Result<{ body: Buffer }> { if (errors[cmdStatus] !== 0 && definition.id >= respBit) return { body: Buffer.alloc(0) }; @@ -221,7 +206,7 @@ function buildPdu( cmdStatus: ErrorName, seqNr: number, params: Record, - tlvs: Record | undefined, + tlvs: TlvInputs | undefined, ): Result<{ buffer: Buffer }> { const definition = cmds[cmdName]; diff --git a/src/retained-pdu.ts b/src/retained-pdu.ts index e42647d..50e429c 100644 --- a/src/retained-pdu.ts +++ b/src/retained-pdu.ts @@ -1,27 +1,21 @@ import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; -import { detachedTlv, tlvOctets } from './defs/types.ts'; -import { isTlvs } from './defs/tlvs.ts'; +import { tlvOctets } from './defs/types.ts'; /** Wire reads hand back views, so retaining one PDU would pin the whole chunk it arrived in. */ export function detach(pduObj: PduObject): PduObject { const params: Record = {}; - const tlvs: Record = {}; for (const [name, value] of Object.entries(pduObj.params)) { params[name] = Buffer.isBuffer(value) ? Buffer.from(value) : value; } - for (const [name, tlv] of Object.entries(pduObj.tlvs)) { - tlvs[name] = { ...tlv, tagValue: detachedTlv(tlv.tagValue) }; - } - // short_message holds the same octets wherever it was not decoded, so one copy covers both. const octets = Buffer.isBuffer(params.short_message) ? params.short_message : pduObj.shortMessageOctets && Buffer.from(pduObj.shortMessageOctets); - return { ...pduObj, params, shortMessageOctets: octets, tlvs: isTlvs(tlvs) ? tlvs : pduObj.tlvs }; + return { ...pduObj, params, shortMessageOctets: octets }; } // Measured heap beyond the octets, so a PDU of empty fields or empty TLVs is not free. diff --git a/test/dlr.test.ts b/test/dlr.test.ts index 8e3e9a7..fa6ec84 100644 --- a/test/dlr.test.ts +++ b/test/dlr.test.ts @@ -4,7 +4,7 @@ import { consts } from '../src/defs/constants.ts'; import { dlrFromPdu, parseReceipt, receiptCodes } from '../src/dlr.ts'; import { encodeMessage } from '../src/message.ts'; import { objToPdu, pduToObj } from '../src/pdu.ts'; -import type { PduObject, TlvInput } from '../src/pdu.ts'; +import type { PduObject, TlvInputs } from '../src/pdu.ts'; const receiptText = 'id:0195f0c7 sub:001 dlvrd:001 submit date:2508251430 done date:2508251431 stat:DELIVRD err:000 text:hello there'; @@ -14,7 +14,7 @@ const textReceipt = `id:${textReceiptId} sub:001 dlvrd:001 submit date:250905143 function deliverSm( message: Buffer | string, - tlvs?: Record, + tlvs?: TlvInputs, esmClass: number = consts.ESM_CLASS.MC_DELIVERY_RECEIPT, dataCoding = 0, ): PduObject { diff --git a/test/interop.test.ts b/test/interop.test.ts index dbefb68..2e5d905 100644 --- a/test/interop.test.ts +++ b/test/interop.test.ts @@ -95,8 +95,8 @@ describe('our encoder against the reference parser', () => { }, seqNr: 77, tlvs: { - message_state: { tagId: 0x0427, tagValue: 2 }, - receipted_message_id: { tagId: 0x001E, tagValue: 'abc123' }, + message_state: { tagValue: 2 }, + receipted_message_id: { tagValue: 'abc123' }, }, })); diff --git a/test/operator-receipts.test.ts b/test/operator-receipts.test.ts index 21eec8c..36dd655 100644 --- a/test/operator-receipts.test.ts +++ b/test/operator-receipts.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import test, { describe } from 'node:test'; import type { Dlr, Receipt } from '../src/dlr.ts'; import type { MessageDlr } from '../src/session.ts'; -import type { PduObject, TlvInput } from '../src/pdu.ts'; +import type { PduObject, TlvInputs } from '../src/pdu.ts'; import { bindToSmsc, dummySmsc } from './dummy-smsc.ts'; import { consts } from '../src/defs/constants.ts'; import { dlrFromPdu, parseReceipt, receiptCodes, transientStates } from '../src/dlr.ts'; @@ -16,7 +16,7 @@ import { objToPdu, pduToObj } from '../src/pdu.ts'; function deliverSm( body: string, - tlvs?: Record, + tlvs?: TlvInputs, esmClass: number = consts.ESM_CLASS.MC_DELIVERY_RECEIPT, ): PduObject { const { buffer } = objToPdu({ @@ -53,7 +53,7 @@ type ReceiptFixture = { name: string; receipt: Receipt; source: string; - tlvs?: Record; + tlvs?: TlvInputs; }; const fixtures: readonly ReceiptFixture[] = [ diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 1d9b433..5e6388d 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -3,6 +3,7 @@ import test, { describe } from 'node:test'; import { PduRefusedError, refusalAnswer } from '../src/pdu-refusal.ts'; import { isCommand, isResp, objToPdu, pduReturn, pduToObj } from '../src/pdu.ts'; import { paramText } from '../src/defs/types.ts'; +import { tlvsById } from '../src/defs/tlvs.ts'; function encode(...args: Parameters): Buffer { const { buffer, err } = objToPdu(...args); @@ -377,7 +378,7 @@ describe('TLVs', () => { }, seqNr: 393, tlvs: { - 5142: { tagId: 5142, tagValue: Buffer.from('blajfoo', 'ascii') }, + 5142: { tagValue: Buffer.from('blajfoo', 'ascii') }, receipted_message_id: { tagValue: '293f293' }, }, })); @@ -432,25 +433,22 @@ describe('TLVs', () => { assert.deepEqual(pduObj.tlvs.source_port, { tagId: 0x020A, tagName: 'source_port', tagValue: 1234 }); }); - test('refuses an unknown tag name rather than putting a wrong tag on the wire', () => { - const { buffer, err } = objToPdu({ - cmdName: 'deliver_sm', - params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, - tlvs: { nils: { tagValue: 'blajfoo' } }, - }); + test('refuses a TLV keyed any way but by its name, or by its decimal id where the table names none', () => { + const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }; + const refusals = [ + // @ts-expect-error nils is no tag name + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { nils: { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5142, tagValue: 'blajfoo' } } }), reason: /instead of giving a tagId/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 1063: { tagValue: 2 } } }), reason: /message_state/ }, + ]; - assert.equal(buffer, undefined); - assert.ok(err instanceof Error); - }); - - test('refuses a tag id that does not fit the two octet field', () => { - const { err } = objToPdu({ - cmdName: 'deliver_sm', - params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, - tlvs: { nils: { tagId: 0x10000, tagValue: 'blajfoo' } }, - }); - - assert.ok(err instanceof Error); + for (const { built: { buffer, err }, reason } of refusals) { + assert.equal(buffer, undefined); + assert.ok(err instanceof Error); + assert.match(err.message, reason); + } }); test('refuses a TLV too long for the two octet length field', () => { @@ -509,12 +507,33 @@ describe('TLVs', () => { } }); + test('reads every tag in the table as the type its entry declares', () => { + const bare = encode({ cmdName: 'deliver_sm', params: { destination_addr: '46709771337', source_addr: '46701113311' } }); + + for (const { id, tag, type } of Object.values(tlvsById)) { + const sized = type.size(type.default); + + assert.ok(sized.size !== undefined); + + const tlv = Buffer.alloc(4 + sized.size); + + tlv.writeUInt16BE(id, 0); + tlv.writeUInt16BE(sized.size, 2); + assert.equal(type.write(type.default, tlv, 4).err, undefined); + + const pdu = Buffer.concat([bare, tlv]); + + pdu.writeUInt32BE(pdu.length, 0); + assert.ok(Object.hasOwn(decode(pdu).tlvs, tag), tag); + } + }); + test('types each known TLV by its tag, and an unknown one as octets', () => { const pduObj = decode(encode({ cmdName: 'deliver_sm', params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, tlvs: { - 5142: { tagId: 5142, tagValue: Buffer.from('01', 'hex') }, + 5142: { tagValue: Buffer.from('01', 'hex') }, message_state: { tagValue: 2 }, receipted_message_id: { tagValue: '0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15' }, }, @@ -541,8 +560,8 @@ describe('TLVs', () => { }, seqNr: 323, tlvs: { - message_state: { tagId: 1063, tagValue: 2 }, - receipted_message_id: { tagId: 30, tagValue: 450 }, + message_state: { tagValue: 2 }, + receipted_message_id: { tagValue: 450 }, }, })); diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts index f01aef4..3108910 100644 --- a/test/unsendable.test.ts +++ b/test/unsendable.test.ts @@ -173,18 +173,17 @@ describe('a body the PDU\'s own data_coding cannot carry', () => { assert.match(built.err.message, /U\+3042/); }); - test('refuses the body TLV under whatever name the caller keyed its tagId to', () => { + test('refuses the body TLV keyed by its id, so only message_payload is written as text', () => { const built = objToPdu({ cmdName: 'data_sm', - params: { data_coding: 0x03, destination_addr: to, source_addr: from }, - tlvs: { body: { tagId: 0x0424, tagValue: 'あいう' } }, + params: { data_coding: 0x08, destination_addr: to, source_addr: from }, + tlvs: { 1060: { tagValue: 'あいう' }, message_payload: { tagValue: 'あいう' } }, }); assert.ok(built.err instanceof Error); assert.equal(built.buffer, undefined); - assert.match(built.err.message, /"body"/); - assert.match(built.err.message, /LATIN1/); - assert.match(built.err.message, /U\+3042/); + assert.match(built.err.message, /"1060"/); + assert.match(built.err.message, /message_payload/); }); test('leaves data_coding to short_message wherever it carries octets, as messageOctets() reads it', () => { @@ -205,22 +204,6 @@ describe('a body the PDU\'s own data_coding cannot carry', () => { assert.deepEqual(messageOctets(pduObj), short); }); - test('encodes every entry carrying the body tag, so a second one cannot go out truncated', () => { - const built = objToPdu({ - cmdName: 'data_sm', - params: { data_coding: 0x08, destination_addr: to, source_addr: from }, - tlvs: { alias: { tagId: 0x0424, tagValue: 'あいう' }, message_payload: { tagValue: 'あいう' } }, - }); - - assert.equal(built.err, undefined); - assert.ok(built.buffer); - - const hex = built.buffer.toString('hex'); - - assert.equal(hex.split('304230443046').length - 1, 2, 'both entries carry the UCS2 octets'); - assert.ok(!hex.includes('424446'), 'no entry goes out as the low octets of its code points'); - }); - test('leaves the alphabet to the body TLV wherever short_message carries no octets', () => { for (const short of [undefined, '', Buffer.alloc(0)]) { const built = objToPdu({ -- 2.52.0 From c4b0437323c861a4934dd6b1d39aaf9565d96632 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:18:49 +0200 Subject: [PATCH 03/14] Read a named tag's definition once in tagIdOf() --- src/defs/tlvs.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 6417009..c0eb97b 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -135,7 +135,9 @@ function tagIdOf(name: string, input: object): Result<{ tagId: number }> { return { err: new Error(`TLV "${name}": key it by its name, or a tag the table does not define by its decimal id, instead of giving a tagId`) }; } - if (Object.hasOwn(tlvs, name)) return { tagId: tlvs[name]?.id ?? 0 }; + const named = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + + if (named) return { tagId: named.id }; if (!/^(0|[1-9]\d*)$/.test(name)) { return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; -- 2.52.0 From 138464ad0f5a8821085ffe50045283e9f4157ee1 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:25:49 +0200 Subject: [PATCH 04/14] Refuse a TLV input that is no { tagValue }, and the alternate tag names, keep numbers out of octet TLVs, and pin that a TLV is copied off its chunk --- CHANGELOG.md | 4 +++- MIGRATION.md | 3 ++- docs/decisions.md | 4 +--- src/defs/tlvs.ts | 40 +++++++++++++++++++++++++++------------- test/pdu.test.ts | 31 +++++++++++++++++++++---------- 5 files changed, 54 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b74c7c..009b357 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,7 +66,9 @@ **A TLV input is keyed by its name, or by its decimal id where the table names none, and `tagId` is refused.** Write `{ 5142: { tagValue } }` for a vendor tag, not `{ vendor: { tagId: 5142, … } }`. A name and a `tagId` could disagree, and `{ message_state: { tagId: 5, … } }` went out as tag 5. - A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name. + A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and + so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of + `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index b21f6af..3d01adc 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,7 +32,8 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. - **A TLV is keyed by its name, or by its decimal id where the table names none**, and `tagId` on an input is refused: `{ 5142: { tagValue } }`, not `{ vendor: { tagId: 5142, tagValue } }`. - Unknown tags read back under the same decimal key. + Write `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back + under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`. - **The `error` event is `sessionError`**, and `serverError` on the server handle. - **`log`** takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods instead of a `larvitutils` one, and is silent by default: [README](README.md#logging). diff --git a/docs/decisions.md b/docs/decisions.md index b77ffaa..c48d82c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -434,9 +434,7 @@ rule and an index of the titles below. `writeParams()` ignores it, so an empty one, an absent one and one the wire cannot carry are the same input rather than three. A `data_coding` on a command that declares no such field is honoured the other way round, since it is `replace_sm`'s only way to name the alphabet its octets are in. - Every entry carrying the payload tag is resolved, by tag id rather - than by record key, since `tagIdOf()` lets a caller name it anything and a spelling that escaped - the guard would be a second spelling that disagrees about correctness. Rejected: refusing a string + Rejected: refusing a string `message_payload` outright and demanding octets, which contradicts `short_message` on the same PDU. Rejected: guarding every string-valued field against `data_coding`, which says nothing about them — a text field on the wire has an diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index c0eb97b..b3c6dcb 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -110,7 +110,7 @@ type ReadValue = Repeated>; type WriteValue = Specs[K] extends { multiple: true } ? ReadValue : WireValue extends number ? number : WireValue extends string ? number | string - : Buffer | number | string; + : Buffer | string; type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; @@ -122,22 +122,34 @@ export type Tlvs = { [K in TlvName]?: KnownTlv } & Record<`${number}`, Unknow export type Tlv = { [K in TlvName]: KnownTlv }[TlvName] | UnknownTlv; -type Alternate = keyof typeof alternates; - -type Canonical = K extends Alternate ? (typeof alternates)[K]['tag'] : K; - /** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ -export type TlvInputs = { [K in Alternate | TlvName]?: { tagValue: WriteValue> } } +export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } & Record<`${number}`, { tagValue: Buffer | number | string }>; -function tagIdOf(name: string, input: object): Result<{ tagId: number }> { +function isTlvInput(input: unknown): input is { tagValue: TlvValue } { + if (typeof input !== 'object' || input === null || !('tagValue' in input)) return false; + + const value = input.tagValue; + + if (!Array.isArray(value)) return Buffer.isBuffer(value) || typeof value === 'number' || typeof value === 'string'; + + return value.every(one => Buffer.isBuffer(one)) || value.every(one => typeof one === 'number'); +} + +function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { + if (!isTlvInput(input)) { + return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; + } + if ('tagId' in input) { return { err: new Error(`TLV "${name}": key it by its name, or a tag the table does not define by its decimal id, instead of giving a tagId`) }; } - const named = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + if (isTlvName(name)) return { tagId: specs[name].id, tagValue: input.tagValue }; - if (named) return { tagId: named.id }; + const alternate = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + + if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate.tag}, the name it reads back under`) }; if (!/^(0|[1-9]\d*)$/.test(name)) { return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; @@ -149,20 +161,22 @@ function tagIdOf(name: string, input: object): Result<{ tagId: number }> { const known = tlvsById[tagId]; - return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; + return known + ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } + : { tagId, tagValue: input.tagValue }; } /** Each TLV as its four octet header and the value the tag's own wire type writes. */ export function writeTlvs(inputs: TlvInputs | undefined): Result<{ chunks: Buffer[] }> { const chunks: Buffer[] = []; - for (const [name, input] of Object.entries(inputs ?? {})) { - const tag = tagIdOf(name, input); + for (const [name, input] of Object.entries(inputs ?? {})) { + const tag = entryOf(name, input); if (tag.err) return { err: tag.err }; const definition = tlvsById[tag.tagId]; - const values = occurrences(input.tagValue, definition?.multiple === true); + const values = occurrences(tag.tagValue, definition?.multiple === true); if (values.err) return { err: new Error(`TLV "${name}": ${values.err.message}`) }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 5e6388d..3fb637a 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -392,7 +392,7 @@ describe('TLVs', () => { assert.deepEqual(unknown.tagValue, Buffer.from('blajfoo', 'ascii')); }); - test('keeps a binary TLV byte for byte through pduToObj and back', () => { + test('keeps a binary TLV byte for byte through pduToObj and back, copied off the chunk it arrived in', () => { const payload = Buffer.from('deadbeef00ff', 'hex'); const params = { destination_addr: '46709771337', @@ -400,16 +400,21 @@ describe('TLVs', () => { short_message: 'binary payload follows', source_addr: '46701113311', }; - const parsed = decode(encode({ + const pdu = encode({ cmdName: 'deliver_sm', params, seqNr: 7, tlvs: { message_payload: { tagValue: payload } }, - })); - const carried = parsed.tlvs.message_payload; + }); + const chunk = Buffer.alloc(64 * 1024); + + pdu.copy(chunk, 100); + + const carried = decode(chunk.subarray(100, 100 + pdu.length)).tlvs.message_payload; assert.ok(carried); assert.deepEqual(carried.tagValue, payload); + assert.notEqual(carried.tagValue.buffer, chunk.buffer, 'a TLV holds its own octets, not the chunk it arrived in'); const rebuilt = decode(encode({ cmdName: 'deliver_sm', @@ -442,6 +447,8 @@ describe('TLVs', () => { { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 1063: { tagValue: 2 } } }), reason: /message_state/ }, + // @ts-expect-error the value goes in a { tagValue } wrapper + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: 2 } }), reason: /give it as \{ tagValue \}/ }, ]; for (const { built: { buffer, err }, reason } of refusals) { @@ -481,15 +488,17 @@ describe('TLVs', () => { assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]); }); - test('reads the failed areas of a broadcast_sm_resp as broadcast_area_identifier', () => { + test('writes and 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 } }, - })); + const params = { message_id: '01a0d051-b588-76eb-a5c5-a8cb8b854e68' }; + const pduObj = decode(encode({ cmdName: 'broadcast_sm_resp', params, tlvs: { broadcast_area_identifier: { tagValue: areas } } })); assert.deepEqual(pduObj.tlvs.broadcast_area_identifier?.tagValue, areas); + + // @ts-expect-error the alternate spelling is read back under the name, so only the name is written + const { err } = objToPdu({ cmdName: 'broadcast_sm_resp', params, tlvs: { failed_broadcast_area_identifier: { tagValue: areas } } }); + + assert.match(err?.message ?? '', /key it broadcast_area_identifier/); }); test('refuses a repeatable TLV given one value, and a lone TLV given several', () => { @@ -544,6 +553,8 @@ describe('TLVs', () => { assert.deepEqual([id, state, unknown], ['0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15', 2, Buffer.from('01', 'hex')]); + // @ts-expect-error an octet field takes no number, which would go out as its digits + assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { network_error_code: { tagValue: 5 } } })); // @ts-expect-error message_state is an integer assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { message_state: { tagValue: 'ENROUTE' } } }).err); }); -- 2.52.0 From 58e622143bc76f4ad41e93fb570231acadcaa8f3 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:27:25 +0200 Subject: [PATCH 05/14] Drop an assertion that always passes, and reflow a decision --- docs/decisions.md | 9 ++++----- test/pdu.test.ts | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index c48d82c..80c122f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -434,11 +434,10 @@ rule and an index of the titles below. `writeParams()` ignores it, so an empty one, an absent one and one the wire cannot carry are the same input rather than three. A `data_coding` on a command that declares no such field is honoured the other way round, since it is `replace_sm`'s only way to name the alphabet its octets are in. - Rejected: refusing a string - `message_payload` outright and demanding - octets, which contradicts `short_message` on the same PDU. Rejected: guarding every string-valued - field against `data_coding`, which says nothing about them — a text field on the wire has an - alphabet of its own. + Rejected: refusing a string `message_payload` outright and demanding octets, which contradicts + `short_message` on the same PDU. Rejected: guarding every string-valued field against + `data_coding`, which says nothing about them — a text field on the wire has an alphabet of its + own. - **A GSM 03.38 message declares `data_coding` 0x00, and an inbound 0x01 is still read as GSM.** Maintainer's call, 2026-09-09: `dataCodingFor()` and `encodeBody()` both resolved an alphabet diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 3fb637a..52d2723 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -554,7 +554,7 @@ describe('TLVs', () => { assert.deepEqual([id, state, unknown], ['0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15', 2, Buffer.from('01', 'hex')]); // @ts-expect-error an octet field takes no number, which would go out as its digits - assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { network_error_code: { tagValue: 5 } } })); + objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { network_error_code: { tagValue: 5 } } }); // @ts-expect-error message_state is an integer assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { message_state: { tagValue: 'ENROUTE' } } }).err); }); -- 2.52.0 From cda8e9b11293ed760894b63bef06cadb4925035c Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:29:31 +0200 Subject: [PATCH 06/14] Relay a parsed PDU's TLVs as they are, refuse only a tagId that disagrees with its key, and refuse a number for an octet TLV --- CHANGELOG.md | 8 +++++--- MIGRATION.md | 5 +++-- README.md | 7 ++++--- src/defs/tlvs.ts | 43 +++++++++++++++++++++++++++---------------- src/retained-pdu.ts | 2 ++ test/pdu.test.ts | 20 +++++++++++++++++++- 6 files changed, 60 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 009b357..334e5b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,9 +63,11 @@ type for its tag fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote `Record` or `Record`; `TlvInput` is gone. - **A TLV input is keyed by its name, or by its decimal id where the table names none, and `tagId` - is refused.** Write `{ 5142: { tagValue } }` for a vendor tag, not `{ vendor: { tagId: 5142, … } }`. - A name and a `tagId` could disagree, and `{ message_state: { tagId: 5, … } }` went out as tag 5. + **A TLV input is keyed by its name, or by its decimal id where the table names none, and a `tagId` + that disagrees with its key is refused.** Write `{ 5142: { tagValue } }` for a vendor tag, not + `{ vendor: { tagId: 5142, … } }`; `{ message_state: { tagId: 5, … } }` used to go out as tag 5. A + parsed PDU's `tlvs` still relay as they are. A number for an octet TLV, vendor tags included, is + refused, where it went out as its ASCII digits. A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. diff --git a/MIGRATION.md b/MIGRATION.md index 3d01adc..77d4253 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -30,8 +30,9 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.MESSAGING_MODE`**, which also names `SMSC_DEFAULT`. They are bits 1-0 of `esm_class`, not whole values of it. Read them from the new group, or pass `messagingMode` to `sendSms()`. A stale `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. -- **A TLV is keyed by its name, or by its decimal id where the table names none**, and `tagId` on - an input is refused: `{ 5142: { tagValue } }`, not `{ vendor: { tagId: 5142, tagValue } }`. +- **A TLV is keyed by its name, or by its decimal id where the table names none**, and a `tagId` + disagreeing with its key is refused: `{ 5142: { tagValue } }`, not + `{ vendor: { tagId: 5142, tagValue } }`. A number for an octet TLV is refused; give a Buffer. Write `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`. - **The `error` event is `sessionError`**, and `serverError` on the server handle. diff --git a/README.md b/README.md index 38aefcb..864aa30 100644 --- a/README.md +++ b/README.md @@ -630,9 +630,10 @@ if (isCommand(pduObj, 'submit_sm')) { naming the character, its code point and where it is. - Every text field is latin1: addresses, `system_id`, `message_id`, `service_type` and the C-Octet String TLVs. A character past `U+00FF` is refused, as is a `U+0000` in a C-Octet String. -- `tlvs` is keyed and typed like `pduObj.tlvs`, as `TlvInputs`: `{ message_state: { tagValue: 2 } }`, - or `{ 5142: { tagValue: octets } }` for a tag the table does not define. Any other key, a decimal - id the table names, and a `tagId` are refused. +- `tlvs` is keyed and typed like `pduObj.tlvs`, as `TlvInputs`, so a parsed PDU's `tlvs` relay as + they are: `{ message_state: { tagValue: 2 } }`, or `{ 5142: { tagValue: octets } }` for a tag the + table does not define. Any other key, a decimal id the table names, a `tagId` disagreeing with its + key, and a number for an octet TLV are refused. - The five repeatable TLVs take an array, written as one TLV per element; a lone value or an empty array is refused. - A `Buffer` goes out exactly as given under any `data_coding`: binary payloads, hand-built user diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index b3c6dcb..74a557a 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -106,7 +106,7 @@ type WireValue = Specs[K]['type']['default']; type ReadValue = Repeated>; -/** A lone text field also takes a number, and a lone octet field text, which goes out as latin1. */ +/** A lone text field also takes a number, and an octet field text, which goes out as latin1. */ type WriteValue = Specs[K] extends { multiple: true } ? ReadValue : WireValue extends number ? number : WireValue extends string ? number | string @@ -118,13 +118,13 @@ type KnownTlv = { tagId: number; tagName: K; tagValue: ReadVa type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; /** Keyed by tag name, or by its decimal id where the table defines no name. */ -export type Tlvs = { [K in TlvName]?: KnownTlv } & Record<`${number}`, UnknownTlv>; +export type Tlvs = { [K in TlvName]?: KnownTlv } & Partial>; export type Tlv = { [K in TlvName]: KnownTlv }[TlvName] | UnknownTlv; /** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } - & Record<`${number}`, { tagValue: Buffer | number | string }>; + & Partial>; function isTlvInput(input: unknown): input is { tagValue: TlvValue } { if (typeof input !== 'object' || input === null || !('tagValue' in input)) return false; @@ -136,16 +136,8 @@ function isTlvInput(input: unknown): input is { tagValue: TlvValue } { return value.every(one => Buffer.isBuffer(one)) || value.every(one => typeof one === 'number'); } -function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { - if (!isTlvInput(input)) { - return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; - } - - if ('tagId' in input) { - return { err: new Error(`TLV "${name}": key it by its name, or a tag the table does not define by its decimal id, instead of giving a tagId`) }; - } - - if (isTlvName(name)) return { tagId: specs[name].id, tagValue: input.tagValue }; +function keyedTagId(name: string): Result<{ tagId: number }> { + if (isTlvName(name)) return { tagId: specs[name].id }; const alternate = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; @@ -161,9 +153,24 @@ function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue const known = tlvsById[tagId]; - return known - ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } - : { tagId, tagValue: input.tagValue }; + return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; +} + +/** The key names the tag; a `tagId` beside it, as a parsed TLV carries, has to agree. */ +function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { + if (!isTlvInput(input)) { + return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; + } + + const keyed = keyedTagId(name); + + if (keyed.err) return { err: keyed.err }; + + if ('tagId' in input && input.tagId !== keyed.tagId) { + return { err: new Error(`TLV "${name}": tagId ${String(input.tagId)} is not the tag its key names, ${String(keyed.tagId)}; drop the tagId`) }; + } + + return { tagId: keyed.tagId, tagValue: input.tagValue }; } /** Each TLV as its four octet header and the value the tag's own wire type writes. */ @@ -205,6 +212,10 @@ function occurrences(value: TlvValue, multiple: boolean): Result<{ values: Param } function writeTlv(tagId: number, type: WireType, value: ParamValue): Result<{ chunk: Buffer }> { + if (type === tlvDefault && typeof value === 'number') { + return { err: new Error('holds octets, which a number would write as its digits; give a Buffer or a string') }; + } + const sized = type.size(value); if (sized.err) return { err: sized.err }; diff --git a/src/retained-pdu.ts b/src/retained-pdu.ts index 50e429c..c46e2c9 100644 --- a/src/retained-pdu.ts +++ b/src/retained-pdu.ts @@ -38,6 +38,8 @@ export function retainedOctets(pduObj: PduObject): number { } for (const tlv of Object.values(pduObj.tlvs)) { + if (tlv === undefined) continue; + const listed = Array.isArray(tlv.tagValue) ? tlv.tagValue.length : 0; octets += tlvOctets(tlv.tagValue) + (1 + listed) * tlvObjectOverhead; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 52d2723..dc373d3 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -443,7 +443,10 @@ describe('TLVs', () => { const refusals = [ // @ts-expect-error nils is no tag name { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { nils: { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, - { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5142, tagValue: 'blajfoo' } } }), reason: /instead of giving a tagId/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5143, tagValue: 'blajfoo' } } }), reason: /tagId 5143/ }, + // @ts-expect-error the key names the tag + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: 5, tagValue: 2 } } }), reason: /tagId 5/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagValue: 300 } } }), reason: /Buffer/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 1063: { tagValue: 2 } } }), reason: /message_state/ }, @@ -537,6 +540,21 @@ describe('TLVs', () => { } }); + test('relays a parsed PDU\'s TLVs back out as they arrived', () => { + const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }; + const parsed = decode(encode({ + cmdName: 'deliver_sm', + params, + tlvs: { + 5142: { tagValue: Buffer.from('01', 'hex') }, + callback_num: { tagValue: [Buffer.from('0146709771337', 'hex')] }, + receipted_message_id: { tagValue: '0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15' }, + }, + })); + + assert.deepEqual(decode(encode({ cmdName: 'deliver_sm', params, tlvs: parsed.tlvs })).tlvs, parsed.tlvs); + }); + test('types each known TLV by its tag, and an unknown one as octets', () => { const pduObj = decode(encode({ cmdName: 'deliver_sm', -- 2.52.0 From 9379212c688ab5826970c1252002c303582a3ec6 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:31:21 +0200 Subject: [PATCH 07/14] Read an undefined tagId as absent --- src/defs/tlvs.ts | 4 ++-- test/pdu.test.ts | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 74a557a..4b1033a 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -166,8 +166,8 @@ function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue if (keyed.err) return { err: keyed.err }; - if ('tagId' in input && input.tagId !== keyed.tagId) { - return { err: new Error(`TLV "${name}": tagId ${String(input.tagId)} is not the tag its key names, ${String(keyed.tagId)}; drop the tagId`) }; + if ('tagId' in input && input.tagId !== undefined && input.tagId !== keyed.tagId) { + return { err: new Error(`TLV "${name}": its tagId is not ${String(keyed.tagId)}, the tag its key names; drop the tagId`) }; } return { tagId: keyed.tagId, tagValue: input.tagValue }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index dc373d3..e48e28a 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -443,9 +443,9 @@ describe('TLVs', () => { const refusals = [ // @ts-expect-error nils is no tag name { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { nils: { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, - { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5143, tagValue: 'blajfoo' } } }), reason: /tagId 5143/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5143, tagValue: 'blajfoo' } } }), reason: /its tagId is not 5142/ }, // @ts-expect-error the key names the tag - { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: 5, tagValue: 2 } } }), reason: /tagId 5/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: 5, tagValue: 2 } } }), reason: /its tagId is not 1063/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagValue: 300 } } }), reason: /Buffer/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, @@ -553,6 +553,8 @@ describe('TLVs', () => { })); assert.deepEqual(decode(encode({ cmdName: 'deliver_sm', params, tlvs: parsed.tlvs })).tlvs, parsed.tlvs); + // @ts-expect-error the key names the tag, and an undefined tagId names none + assert.equal(objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: undefined, tagValue: 2 } } }).err, undefined); }); test('types each known TLV by its tag, and an unknown one as octets', () => { -- 2.52.0 From 01891bc7649864bcd5f530138d54fd6607d32464 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:33:35 +0200 Subject: [PATCH 08/14] Word a disagreeing tagId as a mismatch, and record how a TLV input is keyed --- AGENTS.md | 2 ++ docs/decisions.md | 9 +++++++++ src/defs/tlvs.ts | 2 +- test/pdu.test.ts | 4 ++-- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b05736e..ee83e0c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,6 +280,8 @@ the file. - A GSM 03.38 message declares `data_coding` 0x00, and an inbound 0x01 is still read as GSM. - Every text field on the wire is latin1, and what the field cannot carry is refused rather than truncated. +- A TLV input is keyed by its tag name, or by its decimal id where the table names none, and a + `tagId` beside the key is accepted only where it agrees. ### [The session's life](docs/decisions.md#the-sessions-life) diff --git a/docs/decisions.md b/docs/decisions.md index 80c122f..28df36d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -490,6 +490,15 @@ rule and an index of the titles below. text field, which would buy one spelling by taking a legitimate octet away from the length-prefixed Octet String, whose length octet is what ends it. +- **A TLV input is keyed by its tag name, or by its decimal id where the table names none, and a + `tagId` beside the key is accepted only where it agrees.** Settled in the architecture and + product-owner reviews of [#30](https://gitea.larvit.se/larvit/smpp-js/pulls/30), 2026-09-27. The + key is the one spelling, because a name and a `tagId` that disagreed sent the `tagId`'s tag under + a record keyed as another. A parsed TLV carries its `tagId`, and goal 8's passthrough means a + parsed PDU's `tlvs` relay as they are, so an agreeing copy is read past rather than refused. + Rejected: refusing every `tagId`, which breaks relaying. Rejected: a `tagId` overriding the key, + the 0.5.0 behaviour. Valid while parsed TLVs carry `tagId`. + ## The session's life - **A close arriving after our own `unbind` is a clean unbind, not an error.** Maintainer's call, diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 4b1033a..d02ef23 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -167,7 +167,7 @@ function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue if (keyed.err) return { err: keyed.err }; if ('tagId' in input && input.tagId !== undefined && input.tagId !== keyed.tagId) { - return { err: new Error(`TLV "${name}": its tagId is not ${String(keyed.tagId)}, the tag its key names; drop the tagId`) }; + return { err: new Error(`TLV "${name}": its tagId does not match ${String(keyed.tagId)}, the tag its key names; drop the tagId`) }; } return { tagId: keyed.tagId, tagValue: input.tagValue }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index e48e28a..06d60b8 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -443,9 +443,9 @@ describe('TLVs', () => { const refusals = [ // @ts-expect-error nils is no tag name { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { nils: { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, - { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5143, tagValue: 'blajfoo' } } }), reason: /its tagId is not 5142/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagId: 5143, tagValue: 'blajfoo' } } }), reason: /does not match 5142/ }, // @ts-expect-error the key names the tag - { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: 5, tagValue: 2 } } }), reason: /its tagId is not 1063/ }, + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: { tagId: 5, tagValue: 2 } } }), reason: /does not match 1063/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 5142: { tagValue: 300 } } }), reason: /Buffer/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, -- 2.52.0 From 9fb8f348c87cdf330f3a1949b09797ee7a790a2d Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:36:56 +0200 Subject: [PATCH 09/14] Drop the alternate TLV names from tlvs, correct the false doc claims the prose pass found, and file the moves --- AGENTS.md | 9 ++++----- CHANGELOG.md | 8 +++++--- MIGRATION.md | 5 +++-- benchmarks/README.md | 4 ++-- interop-tests/AGENTS.md | 2 +- interop-tests/README.md | 4 ++-- src/defs/tlvs.ts | 18 ++++++++---------- todo.md | 13 +++++++++---- 8 files changed, 34 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ee83e0c..3b858a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,8 +14,7 @@ not for structure or style. The goals, in priority order, live in [README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals) — they say where this library is heading, which an outside -reader judges it by. The README states the audience alongside them. Everything below cites a goal by -number. +reader judges it by. The README states the audience alongside them. ## Hard rules @@ -83,7 +82,7 @@ src/ constants.ts consts + constsById, and the SMPP version constants encodings.ts GSM 03.38, LATIN1, UCS2, detection, data_coding resolution errors.ts errors + errorsById (ESME_*) - tlvs.ts TLV definitions, tlvsById, the input shape, and reading and writing a TLV stream + tlvs.ts TLV definitions, tlvsById, the typed read and input shapes, and reading and writing a TLV stream types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays ``` @@ -162,8 +161,8 @@ decision under [The wire](docs/decisions.md#the-wire). - Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant. Two deliberate exceptions: command parameters are in wire order (above), and the `errors` and TLV tables are ordered by their numeric id so they can be diffed against the spec and gaps stay visible. -- Comments are the exception, not the default — see the root `CLAUDE.md` rules. Do not write file - preambles or restate what the code says. +- Comments are the exception, not the default. Do not write file preambles or restate what the + code says. - Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders. - Fixtures that encode the wire are shared so no two files can drift on it: `test/raw-pdus.ts` builds the octets a test writes straight to a socket, the PDUs `objToPdu()` refuses to build included. So diff --git a/CHANGELOG.md b/CHANGELOG.md index 334e5b6..b46c614 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,8 +59,8 @@ for the first occurrence. `objToPdu()`, `session.send()` and `session.sendReturn()` take `{ tagValue: [value] }` for them and refuse a lone value before anything goes out. - `pduObj.tlvs` and every `tlvs` input are typed per tag, as `Tlvs` and `TlvInputs`: - `receipted_message_id` reads as a `string`, `callback_num` as a `Buffer[]`, and a value of the wrong - type for its tag fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote + `receipted_message_id` reads as a `string`, `message_state` as a `number`, and a value the tag + cannot carry fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote `Record` or `Record`; `TlvInput` is gone. **A TLV input is keyed by its name, or by its decimal id where the table names none, and a `tagId` @@ -68,9 +68,11 @@ `{ vendor: { tagId: 5142, … } }`; `{ message_state: { tagId: 5, … } }` used to go out as tag 5. A parsed PDU's `tlvs` still relay as they are. A number for an octet TLV, vendor tags included, is refused, where it went out as its ASCII digits. + A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of - `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. + `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. The + two alternate names are gone from `tlvs` too. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index 77d4253..2759f16 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,9 +32,10 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. - **A TLV is keyed by its name, or by its decimal id where the table names none**, and a `tagId` disagreeing with its key is refused: `{ 5142: { tagValue } }`, not - `{ vendor: { tagId: 5142, tagValue } }`. A number for an octet TLV is refused; give a Buffer. + `{ vendor: { tagId: 5142, tagValue } }`. A number for an octet TLV is refused; give a Buffer or a string. Write `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back - under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`. + under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`, which are gone from + `tlvs` too. - **The `error` event is `sessionError`**, and `serverError` on the server handle. - **`log`** takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods instead of a `larvitutils` one, and is silent by default: [README](README.md#logging). diff --git a/benchmarks/README.md b/benchmarks/README.md index ffc64de..fb224e7 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -59,8 +59,8 @@ Against real peers, same driver, window 50, 20,000 messages: | SMSC | msgs/s | failed | | --- | --- | --- | | this library's sink | 37,125 | 0 | -| Jasmin 0.10 | 2,207 | 0 | -| SMPPSim 3.0.0 | — | 19,000 of 20,000 | +| Jasmin 0.11.0 | 2,207 | 0 | +| SMPPSim 2.6.11 | — | 19,000 of 20,000 | ## Against the other client libraries diff --git a/interop-tests/AGENTS.md b/interop-tests/AGENTS.md index 41a6201..c1476ca 100644 --- a/interop-tests/AGENTS.md +++ b/interop-tests/AGENTS.md @@ -93,7 +93,7 @@ ways the next experiment must see. One fix per defect class, as its own change: 1. A worktree on a branch off `origin/main` (never `origin/v0.4.0`, the 0.4.0 code), named for the defect. 2. Regression tests in `test/` first, naming the behaviour with the reproducer from the findings; - then the implementation; then the decision record in the root `AGENTS.md` where the fix settles + then the implementation; then the decision record in `docs/decisions.md` where the fix settles a question of the wire or the session's life. 3. `/larv-review` on the branch, with the pull request based on `main`. When it marks the PR ready, fast-forward it. diff --git a/interop-tests/README.md b/interop-tests/README.md index ecc1c70..29bfda9 100644 --- a/interop-tests/README.md +++ b/interop-tests/README.md @@ -1,6 +1,6 @@ # interop-tests -Eight real SMPP implementations, run against this library in both directions, with every session +Ten real SMPP implementations, run against this library in both directions, with every session decoded independently by tshark so no result rests on our own view of the wire. It exists because the unit suite and this library's own dummy peers agree with themselves; these peers do not. @@ -40,7 +40,7 @@ These bind to our server: | Peer | What it is for | Run | | --- | --- | --- | -| **Kannel 1.4.5** | The most deployed real ESME there is; parses our receipts with the parser most operators' customers run, and declares 3.4 or 3.3 on demand | `debian:bookworm-slim` + the distribution package; four `.conf` variants under `peers/kannel/` | +| **Kannel 1.4.5** | The most deployed real ESME there is; parses our receipts with the parser most operators' customers run, and declares 3.4 or 3.3 on demand | `debian:bookworm-20260824-slim` + the distribution package; four `.conf` variants under `peers/kannel/` | | **jsmpp** | Strict and low-level: the driver builds UDH, `sar_*` and `message_payload` bytes by hand, and rejects an answer it dislikes | Maven build at a pinned commit, `peers/jsmpp/` | | **Cloudhopper** | The one peer with real windowing knobs, plus a TLS client | Maven build at a pinned commit, `peers/cloudhopper/`. Its 2015-era TLS client cannot do 1.3, so that scenario caps the server at 1.2 | | **python-smpplib 2.2.4** | An independent GSM 03.38 table to cross-check ours character by character | `python:3.12.14-slim-bookworm`, `peers/python/` | diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index d02ef23..85425e8 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -83,13 +83,13 @@ type Specs = typeof specs; export type TlvName = keyof Specs; -// Alternate spellings; the definition behind each keeps its canonical name. -const alternates = { - alert_on_msg_delivery: specs.alert_on_message_delivery, - failed_broadcast_area_identifier: specs.broadcast_area_identifier, +// SMPP 5.0's other spellings, which only name the tag to key instead. +const alternates: Record = { + alert_on_msg_delivery: 'alert_on_message_delivery', + failed_broadcast_area_identifier: 'broadcast_area_identifier', }; -export const tlvs: Record & Record = { ...specs, ...alternates }; +export const tlvs: Record & Record = specs; export const tlvsById: Record = {}; @@ -114,7 +114,6 @@ type WriteValue = Specs[K] extends { multiple: true } ? ReadV type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; -/** A tag the TLV table does not define. */ type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; /** Keyed by tag name, or by its decimal id where the table defines no name. */ @@ -122,7 +121,7 @@ export type Tlvs = { [K in TlvName]?: KnownTlv } & Partial }[TlvName] | UnknownTlv; -/** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ +/** Keyed like `Tlvs`. */ export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } & Partial>; @@ -139,9 +138,9 @@ function isTlvInput(input: unknown): input is { tagValue: TlvValue } { function keyedTagId(name: string): Result<{ tagId: number }> { if (isTlvName(name)) return { tagId: specs[name].id }; - const alternate = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + const alternate = Object.hasOwn(alternates, name) ? alternates[name] : undefined; - if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate.tag}, the name it reads back under`) }; + if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate}, the name it reads back under`) }; if (!/^(0|[1-9]\d*)$/.test(name)) { return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; @@ -156,7 +155,6 @@ function keyedTagId(name: string): Result<{ tagId: number }> { return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; } -/** The key names the tag; a `tagId` beside it, as a parsed TLV carries, has to agree. */ function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { if (!isTlvInput(input)) { return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; diff --git a/todo.md b/todo.md index 2a11169..79cc2bb 100644 --- a/todo.md +++ b/todo.md @@ -194,10 +194,6 @@ and is also what the panel ranked hardest — two methods, one answer. rest silently, which goal 3 argues against; listing it would change every such tag's shape. From the architecture review of #25. -- [ ] **Refuse a TLV input naming one tag under both its spellings.** `broadcast_area_identifier` - and `failed_broadcast_area_identifier` in one `tlvs` record both write, so the peer receives - the union of two lists the caller may have meant as one. From the architecture review of #25. - - [ ] **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. @@ -397,6 +393,15 @@ and is also what the panel ranked hardest — two methods, one answer. ## Worth doing, not blocking +- [ ] **Move the decisions out of AGENTS.md's fixtures paragraph and the two tooling READMEs.** The + one dummy SMSC, `smscPeer()` staying separate and which copied helpers are tolerated + (AGENTS.md Conventions), and why Kannel is absent from `benchmarks/README.md`, go to + `docs/decisions.md` with index lines. Move the planned work written into + `interop-tests/AGENTS.md` (an expected count per peer) and `benchmarks/README.md` (the default + window gap) to this file. Give the `run.py`-in-background footgun in `interop-tests/AGENTS.md` + a rule of its own. Delete AGENTS.md's "message_id values … are UUID v7" line. From the prose + pass of #30. + - [ ] **Make the dumbclient soak's memory sample evidence of no library leak again.** Its rss ends at its maximum (298 MiB, heapUsed 81 MiB after 173,820 messages), which the harness's own per-id `Set` and `answerOrder` explain but cannot separate from a leak in `src/`: sample the heap -- 2.52.0 From ac6206dfecffa3dba1bc26bdfbb94d9a412041cd Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:38:09 +0200 Subject: [PATCH 10/14] Type tlvs by its own names, so a dropped alternate fails to compile --- src/defs/tlvs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index 85425e8..c48ef40 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -89,7 +89,7 @@ const alternates: Record = { failed_broadcast_area_identifier: 'broadcast_area_identifier', }; -export const tlvs: Record & Record = specs; +export const tlvs: Record = specs; export const tlvsById: Record = {}; -- 2.52.0 From 4094949917ed9104f2c52f029c884e17e68debd9 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:38:17 +0200 Subject: [PATCH 11/14] Say tlvs is typed by TlvName --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b46c614..bb3bb43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,7 +72,7 @@ A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. The - two alternate names are gone from `tlvs` too. + two alternate names are gone from `tlvs` too, which is now typed by `TlvName`: index it with one. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 -- 2.52.0 From dd09a27dc64a089648bbd88fc39dbb025a280890 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 15:39:43 +0200 Subject: [PATCH 12/14] Export isTlvName, so a string can be narrowed into tlvs --- CHANGELOG.md | 2 +- README.md | 2 +- src/defs/tlvs.ts | 2 +- src/index.ts | 2 +- test/pdu.test.ts | 6 +++++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb3bb43..02e45dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,7 +72,7 @@ A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. The - two alternate names are gone from `tlvs` too, which is now typed by `TlvName`: index it with one. + two alternate names are gone from `tlvs` too, which is now typed by `TlvName`: narrow a `string` with `isTlvName()` before indexing it. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/README.md b/README.md index 864aa30..ba17dc9 100644 --- a/README.md +++ b/README.md @@ -655,7 +655,7 @@ if (isCommand(pduObj, 'submit_sm')) { | Messages | `encodeMessage`, `decodeMessage`, `splitMessage`, `bitCount`, `messageOctets`, `concatOf`, `concatInfo`, `detect`, `unencodable`, `messageClassOf`, `dataCodingByEncoding`, `encodingByDataCoding` | | Receipts | `dlrFromPdu`, `parseReceipt`, `receiptCodes` | | Time and ids | `smppDate`, `smppTime`, `uuidv7` | -| Spec tables | `cmds`, `consts`, `encodings`, `errors`, `tlvs`, `types`, the `cmdsById`, `constsById`, `errorsById` and `tlvsById` maps, and all of them grouped as `defs`. `isCommandName`, `isErrorName`, `isEncodingName`, `commandNameById` and `errorNameById` narrow a value into them. | +| Spec tables | `cmds`, `consts`, `encodings`, `errors`, `tlvs`, `types`, the `cmdsById`, `constsById`, `errorsById` and `tlvsById` maps, and all of them grouped as `defs`. `isCommandName`, `isErrorName`, `isEncodingName`, `isTlvName`, `commandNameById` and `errorNameById` narrow a value into them. | | Types | Every option, result, event payload and table entry has a named type: `ClientOptions`, `ServerOptions`, `SendSmsOptions`, `SendSmsResult`, `Sms`, `Dlr`, `MessageDlr`, `Receipt`, `PduObject`, `PduHeader`, `SmppLog`, `Result` and the rest in `dist/index.d.ts`. | ## What changed per release diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index c48ef40..fc4d6b0 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -253,7 +253,7 @@ function readTlv(pdu: Buffer, offset: number): Result<{ octets: number; occurren return { occurrence: { definition, tagId, value }, octets: 4 + tagLength }; } -function isTlvName(name: string): name is TlvName { +export function isTlvName(name: string): name is TlvName { return Object.hasOwn(specs, name); } diff --git a/src/index.ts b/src/index.ts index 837f3f0..509250a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,7 @@ export { cmds, cmdsById, commandNameById, isCommandName } from './defs/commands. export { consts, constsById } from './defs/constants.ts'; export { dataCodingByEncoding, detect, encodingByDataCoding, encodings, isEncodingName, messageClassOf, unencodable } from './defs/encodings.ts'; export { errorNameById, errors, errorsById, isErrorName } from './defs/errors.ts'; -export { tlvs, tlvsById } from './defs/tlvs.ts'; +export { isTlvName, tlvs, tlvsById } from './defs/tlvs.ts'; export { types } from './defs/types.ts'; export { diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 06d60b8..c773865 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -3,7 +3,7 @@ import test, { describe } from 'node:test'; import { PduRefusedError, refusalAnswer } from '../src/pdu-refusal.ts'; import { isCommand, isResp, objToPdu, pduReturn, pduToObj } from '../src/pdu.ts'; import { paramText } from '../src/defs/types.ts'; -import { tlvsById } from '../src/defs/tlvs.ts'; +import { isTlvName, tlvsById } from '../src/defs/tlvs.ts'; function encode(...args: Parameters): Buffer { const { buffer, err } = objToPdu(...args); @@ -502,6 +502,10 @@ describe('TLVs', () => { const { err } = objToPdu({ cmdName: 'broadcast_sm_resp', params, tlvs: { failed_broadcast_area_identifier: { tagValue: areas } } }); assert.match(err?.message ?? '', /key it broadcast_area_identifier/); + assert.deepEqual( + ['broadcast_area_identifier', 'failed_broadcast_area_identifier', 'constructor'].map(isTlvName), + [true, false, false], + ); }); test('refuses a repeatable TLV given one value, and a lone TLV given several', () => { -- 2.52.0 From 9724dd1751dda653402e3e0646784fda646b49ba Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 16:41:47 +0200 Subject: [PATCH 13/14] Record #30 merging under the comprehension floor, and put Locality first in 0.6.0 --- AGENTS.md | 1 + docs/decisions.md | 6 +++ todo.md | 104 ++++++++++++++++++++++++---------------------- 3 files changed, 61 insertions(+), 50 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3b858a5..b93c9c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -315,6 +315,7 @@ the file. ### [Internals and tests](docs/decisions.md#internals-and-tests) +- #30 merged under the comprehension floor, and Locality is the next work. - A listener that rejects is routed by Node's `captureRejections`, not by hand-dispatching. - The four-line abort dance is copied across `LinkGate`, `IdleWaiters`, `PendingRequests` and `SendWindow` rather than extracted. diff --git a/docs/decisions.md b/docs/decisions.md index 28df36d..6f1e095 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -775,6 +775,12 @@ rule and an index of the titles below. ## Internals and tests +- **#30 merged under the comprehension floor, and Locality is the next work.** Maintainer's call, + 2026-09-27. A four-seat scoring run, depth 1, read the project at 6, 6, 7 and 6 (mean 6.25), every + seat capped by Locality in the held-message and shutdown code #30 does not touch, where the floor + is 7.0. The chunks after #30 lift Locality to 7 before any other work. Serves goal 8's reshapeable + internals, which a reader has to understand before reshaping. Valid until a scoring run reads 7.0 or above. + - **A listener that rejects is routed by Node's `captureRejections`, not by hand-dispatching.** Both emitters construct with `captureRejections: true` and implement `[EventEmitter.captureRejectionSymbol]`, which lands a rejected `async` listener on `sessionError` diff --git a/todo.md b/todo.md index 79cc2bb..94a0cc6 100644 --- a/todo.md +++ b/todo.md @@ -187,57 +187,11 @@ dimensions, higher where it is cheap. Maintainer's call, 2026-09-20. A systems-a same day returned ALIGN with one blocking-severity finding, which is the first item under Locality and is also what the panel ranked hardest — two methods, one answer. -### Correctness, ahead of everything below +A four-seat scoring run on 2026-09-27 read #30 at 6, 6, 7 and 6, every seat capped by Locality in +the held-message and shutdown code; #30 merged under the floor on condition that Locality is the +next work ([decision](docs/decisions.md#internals-and-tests)). -- [ ] **Settle what a repeated tag not marked `multiple` reads as, and pin it in a test.** A vendor - tag or a known single-value tag a peer sends twice keeps the last occurrence and drops the - rest silently, which goal 3 argues against; listing it would change every such tag's shape. - From the architecture review of #25. - -- [ ] **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. - -- [ ] **Return an `err` where `message` is not a string, rather than throwing.** - `sendSms({ message: undefined })` — a forgotten property — reaches `value.replace()` in - `defs/encodings.ts` through the alphabet detection `checkOptions()` runs, and the `TypeError` - escapes `submitSms()` into the caller's process; `NaN` and `12345` do the same. README promises - "Never throws. Every fallible call resolves to `{ err?, … }`" and AGENTS.md hard rule 1 says it - again, so the docs are false for the likeliest caller mistake there is. From the stability - review of #18. - -- [ ] **Derive `sm_length` for a numeric body, or refuse one.** `resolveBody()` in `pdu.ts` reads the - length only where the body is a Buffer or a string, so - `objToPdu({ cmdName: 'submit_sm', params: { short_message: 12345 } })` writes `sm_length: 0`, - then five octets after it, and reports success — and this library's own parser refuses what it - built, as "TLV 12594 runs past the end of the PDU". Goals 1 and 2. From the stability review - of #18. - -- [ ] **Settle which numbers may spell a text field, refuse the rest, and say so where a consumer - reads it.** `wantText()` takes every finite number through `String()`, so `message_id: 1e21` - writes `1e+21`, `from: 0.1 + 0.2` writes `0.30000000000000004` and `source_addr: -5` writes - `-5` — none of them is the id or the address the caller meant, and all three are reported as - sent. The numeric branch exists for a digit sequence (`message_id: 123`); the product-owner - review of #18 recommends `Number.isSafeInteger(value) && value >= 0` with the refusal naming - the fix, since a 64-bit SMSC id loses digits to a JS number before this library ever sees it. - Goals 2 then 3: `from: 1e21` is reported as sent to an address that reaches nobody, which is - the wrong answer about what happened before it is laxness in what we send. That a number is - accepted at all reaches a consumer in no sentence either: only the type comment at - `defs/commands.ts:239`, and one CHANGELOG line that stops being visible when - 0.7.0 is cut, while README's Building bullet reads as the whole rule for a text field. Whether - this is a supported spelling or 0.4.0 tolerance decides whether that sentence lands in - README.md or in MIGRATION.md — write it in the same change as the rule, so it is worded once. - From the stability and product-owner reviews of #18. - -### Throughput — goal 6, and the default window is where we are slowest - -- [ ] **Close the gap to jsmpp at `maxOutstanding: 10`.** Measured 2026-09-20 against the same sink, - 100,000 messages each: this library 25,358/s, jsmpp 30,771/s, Cloudhopper 27,945/s — we are - last at the one window most callers will ever run, while leading Cloudhopper and trailing jsmpp - by only 5% at 50 and 200. So the cost is not the codec, which the higher windows exercise just - as hard; it is something per-request that the window hides once enough requests overlap. - `benchmarks/` reproduces all three. Goal 6. - -### Locality — 5–6 today, and the gate is 7 +### Locality — next, ahead of everything below; 5–6 today, and the gate is 7 - [ ] **Give `IncomingRequests` a port instead of the `Session` it drives.** It holds its owner and calls eight members of it 18 times, including `this.session.close()` on an inbound `unbind` — @@ -296,6 +250,56 @@ and is also what the panel ranked hardest — two methods, one answer. the unit they would least want to touch, because a mistake here does not throw, does not fail the types, and reaches the peer as somebody's message rendered wrong. +### Correctness + +- [ ] **Settle what a repeated tag not marked `multiple` reads as, and pin it in a test.** A vendor + tag or a known single-value tag a peer sends twice keeps the last occurrence and drops the + rest silently, which goal 3 argues against; listing it would change every such tag's shape. + From the architecture review of #25. + +- [ ] **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. + +- [ ] **Return an `err` where `message` is not a string, rather than throwing.** + `sendSms({ message: undefined })` — a forgotten property — reaches `value.replace()` in + `defs/encodings.ts` through the alphabet detection `checkOptions()` runs, and the `TypeError` + escapes `submitSms()` into the caller's process; `NaN` and `12345` do the same. README promises + "Never throws. Every fallible call resolves to `{ err?, … }`" and AGENTS.md hard rule 1 says it + again, so the docs are false for the likeliest caller mistake there is. From the stability + review of #18. + +- [ ] **Derive `sm_length` for a numeric body, or refuse one.** `resolveBody()` in `pdu.ts` reads the + length only where the body is a Buffer or a string, so + `objToPdu({ cmdName: 'submit_sm', params: { short_message: 12345 } })` writes `sm_length: 0`, + then five octets after it, and reports success — and this library's own parser refuses what it + built, as "TLV 12594 runs past the end of the PDU". Goals 1 and 2. From the stability review + of #18. + +- [ ] **Settle which numbers may spell a text field, refuse the rest, and say so where a consumer + reads it.** `wantText()` takes every finite number through `String()`, so `message_id: 1e21` + writes `1e+21`, `from: 0.1 + 0.2` writes `0.30000000000000004` and `source_addr: -5` writes + `-5` — none of them is the id or the address the caller meant, and all three are reported as + sent. The numeric branch exists for a digit sequence (`message_id: 123`); the product-owner + review of #18 recommends `Number.isSafeInteger(value) && value >= 0` with the refusal naming + the fix, since a 64-bit SMSC id loses digits to a JS number before this library ever sees it. + Goals 2 then 3: `from: 1e21` is reported as sent to an address that reaches nobody, which is + the wrong answer about what happened before it is laxness in what we send. That a number is + accepted at all reaches a consumer in no sentence either: only the type comment at + `defs/commands.ts:239`, and one CHANGELOG line that stops being visible when + 0.7.0 is cut, while README's Building bullet reads as the whole rule for a text field. Whether + this is a supported spelling or 0.4.0 tolerance decides whether that sentence lands in + README.md or in MIGRATION.md — write it in the same change as the rule, so it is worded once. + From the stability and product-owner reviews of #18. + +### Throughput — goal 6, and the default window is where we are slowest + +- [ ] **Close the gap to jsmpp at `maxOutstanding: 10`.** Measured 2026-09-20 against the same sink, + 100,000 messages each: this library 25,358/s, jsmpp 30,771/s, Cloudhopper 27,945/s — we are + last at the one window most callers will ever run, while leading Cloudhopper and trailing jsmpp + by only 5% at 50 and 200. So the cost is not the codec, which the higher windows exercise just + as hard; it is something per-request that the window hides once enough requests overlap. + `benchmarks/` reproduces all three. Goal 6. + ### Shape — 6 today, and the gate is 7 - [ ] **Group `src/` into a second level, and retire whichever record loses.** 34 files on one -- 2.52.0 From ac9cb40d69af6f570719b385a0ed987296731747 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 27 Sep 2026 16:41:53 +0200 Subject: [PATCH 14/14] Reflow the under-floor decision --- docs/decisions.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 6f1e095..dcf4e83 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -778,8 +778,9 @@ rule and an index of the titles below. - **#30 merged under the comprehension floor, and Locality is the next work.** Maintainer's call, 2026-09-27. A four-seat scoring run, depth 1, read the project at 6, 6, 7 and 6 (mean 6.25), every seat capped by Locality in the held-message and shutdown code #30 does not touch, where the floor - is 7.0. The chunks after #30 lift Locality to 7 before any other work. Serves goal 8's reshapeable - internals, which a reader has to understand before reshaping. Valid until a scoring run reads 7.0 or above. + is 7.0. The chunks after #30 lift Locality to 7 before any other work. Serves goal 8's + reshapeable internals, which a reader has to understand before reshaping. Valid until a scoring + run reads 7.0 or above. - **A listener that rejects is routed by Node's `captureRejections`, not by hand-dispatching.** Both emitters construct with `captureRejections: true` and implement -- 2.52.0