diff --git a/AGENTS.md b/AGENTS.md index b05736e..b93c9c6 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 @@ -280,6 +279,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) @@ -314,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/CHANGELOG.md b/CHANGELOG.md index 3a0dbda..02e45dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,21 @@ `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`, `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` + 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. The + 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/MIGRATION.md b/MIGRATION.md index 35b8b09..2759f16 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -30,6 +30,12 @@ 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 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 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`, 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/README.md b/README.md index aee9db5..ba17dc9 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,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`, 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 @@ -648,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/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/docs/decisions.md b/docs/decisions.md index b77ffaa..dcf4e83 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -434,13 +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. - 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 - `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 @@ -493,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, @@ -769,6 +775,13 @@ 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/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 7959625..fc4d6b0 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. - alert_on_msg_delivery: specs.alert_on_message_delivery, - failed_broadcast_area_identifier: specs.broadcast_area_identifier, +export type TlvName = keyof Specs; + +// 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 = specs; + export const tlvsById: Record = {}; for (const definition of Object.values(specs)) { @@ -97,43 +100,88 @@ 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; -export type TlvInput = { - /** Resolved from the record key; pass it for a tag the TLV table does not define. */ - tagId?: number | undefined; - tagValue: TlvValue; -}; +type WireValue = Specs[K]['type']['default']; -export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { - const tagId = input.tagId ?? tlvs[name]?.id; +type ReadValue = Repeated>; - if (tagId === undefined) { - return { err: new Error(`TLV "${name}": unknown tag name, give it a tagId`) }; +/** 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 + : Buffer | string; + +type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; + +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 } & Partial>; + +export type Tlv = { [K in TlvName]: KnownTlv }[TlvName] | UnknownTlv; + +/** Keyed like `Tlvs`. */ +export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } + & Partial>; + +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 keyedTagId(name: string): Result<{ tagId: number }> { + if (isTlvName(name)) return { tagId: specs[name].id }; + + const alternate = Object.hasOwn(alternates, name) ? alternates[name] : undefined; + + 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`) }; } - if (!Number.isInteger(tagId) || tagId < 0 || tagId > 0xFFFF) { - return { err: new Error(`TLV "${name}": tagId ${String(tagId)} out of range 0-65535`) }; + 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 }; +} + +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`) }; } - return { tagId }; + const keyed = keyedTagId(name); + + 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 does not match ${String(keyed.tagId)}, the tag its key names; 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. */ -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 ?? {})) { - 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}`) }; @@ -162,6 +210,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 }; @@ -195,13 +247,43 @@ 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 }; +} + +export 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); +} + +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 +299,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, a defect in this library') }; } -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 +318,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/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 d24f9d0..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 { @@ -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, 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, 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 8ba806f..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 { Tlv, TlvInput } 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; @@ -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 { 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]; @@ -293,7 +278,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 +403,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..c46e2c9 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 type { Tlv } from './defs/tlvs.ts'; -import { detachedTlv, tlvOctets } from './defs/types.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 }; + return { ...pduObj, params, shortMessageOctets: octets }; } // Measured heap beyond the octets, so a PDU of empty fields or empty TLVs is not free. @@ -44,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/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/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 c2de212..c773865 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 { isTlvName, 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' }, }, })); @@ -391,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', @@ -399,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', @@ -432,25 +438,27 @@ 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: 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: /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/ }, + { 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 \}/ }, + ]; - 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', () => { @@ -475,36 +483,106 @@ 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]); }); - 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/); + 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', () => { 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('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('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); + // @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', () => { + const pduObj = decode(encode({ + cmdName: 'deliver_sm', + params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' }, + tlvs: { + 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 an octet field takes no number, which would go out as its digits + 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); + }); + 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({ @@ -517,8 +595,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/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/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({ diff --git a/todo.md b/todo.md index ee74247..94a0cc6 100644 --- a/todo.md +++ b/todo.md @@ -187,69 +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)). -- [ ] **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. - 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. - -- [ ] **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` — @@ -308,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 @@ -405,6 +397,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