From ef13290230cd6dfca4d03ef03317832c1f3d7e74 Mon Sep 17 00:00:00 2001 From: Lillem4n Date: Wed, 9 Sep 2026 19:33:36 +0200 Subject: [PATCH] Refuse a body the PDU's own data_coding cannot carry (#98) * Regression tests for a body the PDU's own data_coding cannot carry * Refuse a body the PDU's own data_coding cannot carry, in the codec both send paths build through * Record the codec's body refusal, and drop two todo entries it and the interop run close * Regression tests for the aliased body TLV and the data_coding short_message settles * Find the body TLV by tag id, leave data_coding to the field that will be read, and correct the record * Record the two architecture findings this change does not close * Regression tests for a duplicated body tag and a short_message the wire never carries * Resolve every body TLV against the field that will be read, and move TLV writing to its table * Regression tests for a body only the TLV carries and an empty short_message * Let only octets the command writes settle the alphabet, and stop shadowing the TLV table --- AGENTS.md | 38 ++++++- README.md | 7 ++ src/defs/encodings.ts | 7 ++ src/defs/tlvs.ts | 58 ++++++++++ src/message.ts | 23 +++- src/pdu.ts | 175 ++++++++++++++++-------------- src/send-sms.ts | 6 +- test/unsendable.test.ts | 235 +++++++++++++++++++++++++++++++++++++++- todo.md | 29 +++-- 9 files changed, 476 insertions(+), 102 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2d2ad3c..9eb8b75 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,7 +109,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 + tlvs.ts TLV definitions, tlvsById, the input shape, and writing a TLV stream types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays ``` @@ -622,6 +622,42 @@ Grouped by what each one constrains. the one input a per-character reading passes and the encoder then writes as the extended character — the only composition in any of the three codecs, and not a character a message is written in. +- **A string body is written in the alphabet its own `data_coding` names, and one that alphabet + cannot carry is refused by the codec — `message_payload` on the same terms as `short_message`.** + Maintainer's call, 2026-09-09, from the architecture review of + [#97](https://github.com/larvit/larvitsmpp/pull/97): `objToPdu()` took the codec off the caller's + own `data_coding` and encoded with it whatever the text was, so `data_coding` 3 beside `あいう` + returned `42 44 46` — `"BDF"` — reported as built, while a string `message_payload` was cut to its + low octets whatever `data_coding` said. Goal 2 owns it, as it owns the `sendSms()` guard above. + The line falls at the string: a `Buffer` is octets the caller already chose and goes out as given + under any `data_coding`, which is what keeps goal 6's escape hatch open — the raw UDH, 8-bit binary + and deliberately malformed bodies `interop-tests/` builds are all still buildable — and a string + with no `data_coding` is untouched, detection carrying every character it was picked for. The + guard is `unencodable()` again rather than a second reading, and `unencodableText()` is the + character, its code point and its index said once for both refusals. It is reached through + `encodeBody()` in `message.ts`, which is where the `data_coding`-to-text pair already lives: + `encodeBody(text, dataCoding)` is `decodeMessage(buffer, dataCoding)`'s mirror and resolves the + alphabet through the same `encodingByDataCoding()`. `send()` and `sendReturn()` inherit it, + since both build through `buildPdu()`; `sendSms()` does not, and keeps its own guard, because + `splitMessage()` hands the codec a Buffer with nothing left to refuse and the index a segment + could name is not the one in the message. The TLV is encoded rather than merely checked because + `data_coding` names the alphabet of the body wherever it is carried — that is how + `messageOctets()` and `decodeMessage()` read one back, and a `data_sm` has nowhere else to put one + — so refusing what Latin-1 cannot hold while still writing UCS-2 text as Latin-1 octets would + close half of it. `short_message` settles the `data_coding` wherever it carries octets at all, the + order `messageOctets()` reads the two in, so the alphabet a PDU declares is the one its body will + be read under — and a `short_message` on a command whose table declares none is ignored here as + `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, which `data_coding` says nothing about — an address is a C-Octet String and ASCII by 3.4's + own definition. + ### The session's life - **A close arriving after our own `unbind` is a clean unbind, not an error.** Maintainer's call, diff --git a/README.md b/README.md index 711b0d5..c1b45c8 100644 --- a/README.md +++ b/README.md @@ -577,6 +577,13 @@ same for the GSM 03.38 message class: `0` for the flash class `sms.flash` alread and `3` for the ME-, SIM- and TE-specific ones, and `undefined` where that `data_coding`'s coding group carries no class at all. +Building a PDU is the mirror: a string `short_message`, or a string `message_payload`, is encoded in +the alphabet the PDU's own `data_coding` names, detected from the text where you name none, and a +`data_coding` whose alphabet cannot carry one of its characters is refused, naming the character, its +code point and where it is. A `Buffer` goes out exactly as given under any `data_coding`, which is +how binary payloads, hand-built user data headers and deliberately malformed bodies are sent. +`session.send()` and `session.sendReturn()` build through the same codec and refuse the same bodies. + Composing a message by hand takes the send-side pair: `unencodable(message, encoding)` gives `{ char, index }` for the first character an alphabet cannot carry and `undefined` where it carries them all, which is the check `sendSms()` makes before it encodes anything; `smppTime.encode(value)` diff --git a/src/defs/encodings.ts b/src/defs/encodings.ts index 23a62d7..166a1af 100644 --- a/src/defs/encodings.ts +++ b/src/defs/encodings.ts @@ -123,6 +123,13 @@ export function detect(value: string): EncodingName { export type Unencodable = { char: string; index: number }; +/** What an alphabet could not carry, said the one way: the character, its code point and its index. */ +export function unencodableText(at: Unencodable): string { + const point = (at.char.codePointAt(0) ?? 0).toString(16).toUpperCase().padStart(4, '0'); + + return `${JSON.stringify(at.char)} (U+${point}) at index ${String(at.index)}`; +} + /** The first character `encoding` cannot carry, or undefined where it carries every one of them. */ export function unencodable(message: string, encoding: EncodingName): Unencodable | undefined { const codec = encodings[encoding]; diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index a893da2..79ff8c2 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -1,4 +1,5 @@ import type { ParamValue, WireType } from './types.ts'; +import type { Result } from '../result.ts'; import { tlv } from './types.ts'; export type TlvDefinition = { @@ -104,3 +105,60 @@ export type Tlv = { tagName: string | undefined; tagValue: ParamValue; }; + +export type TlvInput = { + /** Resolved from the record key; pass it for a tag the TLV table does not define. */ + tagId?: number | undefined; + tagValue: ParamValue; +}; + +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`) }; + } + + if (!Number.isInteger(tagId) || tagId < 0 || tagId > 0xFFFF) { + return { err: new Error(`TLV "${name}": tagId ${String(tagId)} out of range 0-65535`) }; + } + + return { 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[] }> { + const chunks: Buffer[] = []; + + for (const [name, input] of Object.entries(inputs ?? {})) { + const tag = tagIdOf(name, input); + + if (tag.err) return { err: tag.err }; + + const type = tlvsById[tag.tagId]?.type ?? tlvDefault; + const sized = type.size(input.tagValue); + + if (sized.err) { + return { err: new Error(`TLV "${name}": ${sized.err.message}`) }; + } + + if (sized.size > 0xffff) { + return { err: new Error(`TLV "${name}": ${String(sized.size)} octets overflow the two octet length`) }; + } + + const chunk = Buffer.alloc(sized.size + 4); + + chunk.writeUInt16BE(tag.tagId, 0); + chunk.writeUInt16BE(sized.size, 2); + + const written = type.write(input.tagValue, chunk, 4); + + if (written.err) { + return { err: new Error(`TLV "${name}": ${written.err.message}`) }; + } + + chunks.push(chunk); + } + + return { chunks }; +} diff --git a/src/message.ts b/src/message.ts index baf369e..2cb6ebe 100644 --- a/src/message.ts +++ b/src/message.ts @@ -1,7 +1,7 @@ import type { Result } from './result.ts'; import type { EncodingName } from './defs/encodings.ts'; -import { detect, encodingByDataCoding, encodings } from './defs/encodings.ts'; -import { hasUdh } from './defs/constants.ts'; +import { detect, encodingByDataCoding, encodings, unencodable, unencodableText } from './defs/encodings.ts'; +import { consts, hasUdh } from './defs/constants.ts'; import { udhLength } from './udh.ts'; /** A single SMS carries 1120 bits, whatever the alphabet. */ @@ -27,6 +27,25 @@ export function encodeMessage( return { buffer: encodings[resolved].encode(message), encoding: resolved }; } +/** A message body as octets, under the alphabet `dataCoding` resolves to, or one detected for it. */ +export function encodeBody( + text: string, + dataCoding: number | undefined, +): Result<{ buffer: Buffer; dataCoding: number }> { + if (dataCoding === undefined) { + const detected = encodeMessage(text); + + return { buffer: detected.buffer, dataCoding: consts.ENCODING[detected.encoding] }; + } + + const encoding = encodingByDataCoding(dataCoding); + const lost = unencodable(text, encoding); + + return lost + ? { err: new Error(`data_coding ${String(dataCoding)} resolves to ${encoding}, which cannot carry ${unencodableText(lost)}; pass a Buffer of octets, or a data_coding whose alphabet carries them`) } + : { buffer: encodings[encoding].encode(text), dataCoding }; +} + export function decodeMessage( buffer: Buffer, dataCoding: number, diff --git a/src/pdu.ts b/src/pdu.ts index 842c35c..9e68ec6 100644 --- a/src/pdu.ts +++ b/src/pdu.ts @@ -3,15 +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 } from './defs/tlvs.ts'; +import type { Tlv, TlvInput } from './defs/tlvs.ts'; import { PduRefusedError, framingRefusal } from './pdu-refusal.ts'; import { cmds, commandNameById, respNameFor } from './defs/commands.ts'; -import { consts, hasUdh } from './defs/constants.ts'; -import { decodeMessage, encodeMessage } from './message.ts'; -import { detect, encodingByDataCoding } from './defs/encodings.ts'; +import { hasUdh } from './defs/constants.ts'; +import { decodeMessage, encodeBody } from './message.ts'; import { errorNameById, errors, isErrorName } from './defs/errors.ts'; import { paramNumber } from './defs/types.ts'; -import { tlvDefault, tlvs, tlvsById } from './defs/tlvs.ts'; +import { tagIdOf, tlvDefault, tlvs, tlvsById, 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; @@ -19,12 +18,6 @@ export const maxSeqNr = 2147483646; /** What the field holds. Read and echoed in full, because peers do write above the spec's range. */ const maxWireSeqNr = 0xFFFFFFFF; -export type TlvInput = { - /** Resolved from the record key; pass it for a tag the TLV table does not define. */ - tagId?: number | undefined; - tagValue: ParamValue; -}; - export type PduObjectInput = { cmdName: C; cmdStatus?: ErrorName; @@ -53,6 +46,8 @@ export type PduObject = { tlvs: Record; }; +export type { TlvInput }; + const respBit = 0x80000000; export function isResp(pduObj: Pick): boolean { @@ -71,44 +66,98 @@ export function isCommand( return pduObj.cmdName === cmdName; } -function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { - const tagId = input.tagId ?? tlvs[name]?.id; +type ResolvedBody = { + params: Record; + tlvs: Record | undefined; +}; - if (tagId === undefined) { - return { err: new Error(`TLV "${name}": unknown tag name, give it a tagId`) }; - } - - if (!Number.isInteger(tagId) || tagId < 0 || tagId > 0xFFFF) { - return { err: new Error(`TLV "${name}": tagId ${String(tagId)} out of range 0-65535`) }; - } - - return { tagId }; +function codingOf(params: Record): number | undefined { + return typeof params.data_coding === 'number' ? params.data_coding : undefined; } -/** Encoding a string short_message settles data_coding and sm_length; a buffer settles sm_length. */ -function resolveShortMessage( - params: Record, -): Record { - const message = params.short_message; +type CarriedBody = { name: string; text: string; tlv: TlvInput }; - if (Buffer.isBuffer(message)) { - return params.sm_length === undefined - ? { ...params, sm_length: message.length } - : { ...params }; +/** 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 }); + } } - if (typeof message !== 'string') return { ...params }; + return carried; +} - const dataCoding = params.data_coding; - const encoding = typeof dataCoding === 'number' ? encodingByDataCoding(dataCoding) : detect(message); - const encoded = encodeMessage(message, encoding); +/** 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; +} - return { - ...params, - data_coding: typeof dataCoding === 'number' ? dataCoding : consts.ENCODING[encoded.encoding], - short_message: encoded.buffer, - sm_length: encoded.buffer.length, - }; +/** The short_message the command's own table will write, since writeParams() ignores any other. */ +function writtenBody(definition: CommandDefinition, value: ParamValue | undefined): ParamValue | undefined { + return definition.params?.short_message === undefined ? undefined : value; +} + +/** Encoded in place, settling data_coding where `settles` says no mandatory field will carry it. */ +function resolveCarried( + resolved: ResolvedBody, + inputs: Record | undefined, + dataCoding: number | undefined, + settles: boolean, +): VoidResult { + for (const carried of carriedBodies(inputs)) { + const encoded = encodeBody(carried.text, dataCoding); + + if (encoded.err) return { err: new Error(`TLV "${carried.name}": ${encoded.err.message}`) }; + + if (settles) resolved.params.data_coding = encoded.dataCoding; + + resolved.tlvs = { ...resolved.tlvs, [carried.name]: { ...carried.tlv, tagValue: encoded.buffer } }; + } + + return {}; +} + +/** data_coding names the alphabet of the body, and short_message settles it where it carries octets. */ +function resolveBody( + params: Record, + tlvs: Record | undefined, + definition: CommandDefinition, +): Result { + const message = writtenBody(definition, params.short_message); + const resolved: ResolvedBody = { params: { ...params }, tlvs }; + + if (Buffer.isBuffer(message) && params.sm_length === undefined) { + resolved.params.sm_length = message.length; + } + + if (typeof message === 'string') { + const encoded = encodeBody(message, codingOf(params)); + + if (encoded.err) { + return { err: new Error(`Parameter "short_message" of "${definition.command}": ${encoded.err.message}`) }; + } + + if (carriesOctets(encoded.buffer)) resolved.params.data_coding = encoded.dataCoding; + + resolved.params.short_message = encoded.buffer; + resolved.params.sm_length = encoded.buffer.length; + } + + // Only octets the command's own table will write can settle the alphabet the PDU declares. + const settles = !carriesOctets(writtenBody(definition, resolved.params.short_message)); + const carried = resolveCarried( + resolved, + tlvs, + settles ? codingOf(params) : codingOf(resolved.params), + settles, + ); + + return carried.err ? { err: carried.err } : resolved; } function writeParams( @@ -139,42 +188,6 @@ function writeParams( return { chunks }; } -function writeTlvs(tlvs: Record | undefined): Result<{ chunks: Buffer[] }> { - const chunks: Buffer[] = []; - - for (const [name, tlv] of Object.entries(tlvs ?? {})) { - const tag = tagIdOf(name, tlv); - - if (tag.err) return { err: tag.err }; - - const type = tlvsById[tag.tagId]?.type ?? tlvDefault; - const sized = type.size(tlv.tagValue); - - if (sized.err) { - return { err: new Error(`TLV "${name}": ${sized.err.message}`) }; - } - - if (sized.size > 0xffff) { - return { err: new Error(`TLV "${name}": ${String(sized.size)} octets overflow the two octet length`) }; - } - - const chunk = Buffer.alloc(sized.size + 4); - - chunk.writeUInt16BE(tag.tagId, 0); - chunk.writeUInt16BE(sized.size, 2); - - const written = type.write(tlv.tagValue, chunk, 4); - - if (written.err) { - return { err: new Error(`TLV "${name}": ${written.err.message}`) }; - } - - chunks.push(chunk); - } - - return { chunks }; -} - /** * SMPP 3.4: a response reporting a failure carries no body, so its fields are not "unused but * present" — they are absent, and a peer that reads them anyway reads past the end of the PDU. @@ -188,11 +201,15 @@ function buildBody( ): Result<{ body: Buffer }> { if (errors[cmdStatus] !== 0 && definition.id >= respBit) return { body: Buffer.alloc(0) }; - const written = writeParams(definition, resolveShortMessage(params), cmdName); + const body = resolveBody(params, tlvs, definition); + + if (body.err) return { err: body.err }; + + const written = writeParams(definition, body.params, cmdName); if (written.err) return { err: written.err }; - const writtenTlvs = writeTlvs(tlvs); + const writtenTlvs = writeTlvs(body.tlvs); if (writtenTlvs.err) return { err: writtenTlvs.err }; diff --git a/src/send-sms.ts b/src/send-sms.ts index fd122b4..723cf62 100644 --- a/src/send-sms.ts +++ b/src/send-sms.ts @@ -7,7 +7,7 @@ import type { SmppLog } from './log.ts'; import type { SmsIdNotation } from './sms-id.ts'; import { UnansweredError } from './unanswered-error.ts'; import { consts, defaultMessagingMode, isMessagingMode, isSubmitMessagingMode, submitMessagingModes } from './defs/constants.ts'; -import { detect, encodingNames, isEncodingName, unencodable } from './defs/encodings.ts'; +import { detect, encodingNames, isEncodingName, unencodable, unencodableText } from './defs/encodings.ts'; import { namedValue } from './error-from.ts'; import { normaliseSmsId } from './sms-id.ts'; import { paramText } from './defs/types.ts'; @@ -168,9 +168,7 @@ function refusedEncoding(encoding: unknown): Error { } function refusedText(encoding: EncodingName, at: Unencodable): Error { - const point = (at.char.codePointAt(0) ?? 0).toString(16).toUpperCase().padStart(4, '0'); - - return new Error(`encoding ${encoding} cannot carry ${JSON.stringify(at.char)} (U+${point}) at index ${String(at.index)}; name UCS2 or leave encoding out`); + return new Error(`encoding ${encoding} cannot carry ${unencodableText(at)}; name UCS2 or leave encoding out`); } /** An alphabet the caller named has to carry the message; the one detect() picks always does. */ diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts index b15f297..4a5541c 100644 --- a/test/unsendable.test.ts +++ b/test/unsendable.test.ts @@ -4,8 +4,9 @@ import type { PduObjectInput } from '../src/pdu.ts'; import type { SendSmsDeps, SendSmsInput } from '../src/send-sms.ts'; import { bindToSmsc, dummySmsc } from './dummy-smsc.ts'; import { decodeMessage } from '../src/message.ts'; +import { messageOctets } from '../src/message-body.ts'; +import { objToPdu, pduToObj } from '../src/pdu.ts'; import { paramNumber, paramText } from '../src/defs/types.ts'; -import { pduToObj } from '../src/pdu.ts'; import { silentLog } from '../src/log.ts'; import { submitSms } from '../src/send-sms.ts'; @@ -112,6 +113,238 @@ describe('an alphabet the caller named that cannot carry the message', () => { }); }); +describe('a body the PDU\'s own data_coding cannot carry', () => { + /** The octets a built PDU carries as its body, wherever the caller put them. */ + function bodyOf(built: ReturnType): Buffer | undefined { + assert.equal(built.err, undefined); + assert.ok(built.buffer); + + const { pduObj } = pduToObj(built.buffer); + + assert.ok(pduObj); + + return messageOctets(pduObj); + } + + // Latin-1 takes the low octet of every code point, so あ (U+3042) went out as 0x42 — the letter B. + test('refuses a string short_message the named alphabet cannot carry, naming the character', () => { + const built = objToPdu({ + cmdName: 'submit_sm', + params: { data_coding: 0x03, destination_addr: to, short_message: 'あいう', source_addr: from }, + }); + + assert.ok(built.err instanceof Error); + assert.equal(built.buffer, undefined); + assert.match(built.err.message, /short_message/); + assert.match(built.err.message, /LATIN1/); + assert.match(built.err.message, /"あ"/); + assert.match(built.err.message, /U\+3042/); + assert.match(built.err.message, /index 0/); + }); + + // Å is in the GSM table at 0x0E; ï is the one the encoder flattened to a space. + test('refuses a string short_message GSM 03.38 has no code for, naming that one', () => { + for (const dataCoding of [0x00, 0x01]) { + const built = objToPdu({ + cmdName: 'submit_sm', + params: { data_coding: dataCoding, destination_addr: to, short_message: 'Åsa naïve', source_addr: from }, + }); + + assert.ok(built.err instanceof Error, String(dataCoding)); + assert.match(built.err.message, /ASCII/); + assert.match(built.err.message, /"ï"/); + assert.match(built.err.message, /U\+00EF/); + assert.match(built.err.message, /index 6/); + } + }); + + test('refuses a string message_payload on the same terms as short_message', () => { + const built = objToPdu({ + cmdName: 'data_sm', + params: { data_coding: 0x03, destination_addr: to, source_addr: from }, + tlvs: { message_payload: { tagValue: 'あいう' } }, + }); + + assert.ok(built.err instanceof Error); + assert.equal(built.buffer, undefined); + assert.match(built.err.message, /message_payload/); + assert.match(built.err.message, /LATIN1/); + assert.match(built.err.message, /"あ"/); + assert.match(built.err.message, /U\+3042/); + }); + + test('refuses the body TLV under whatever name the caller keyed its tagId to', () => { + const built = objToPdu({ + cmdName: 'data_sm', + params: { data_coding: 0x03, destination_addr: to, source_addr: from }, + tlvs: { body: { tagId: 0x0424, 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/); + }); + + test('leaves data_coding to short_message wherever it carries octets, as messageOctets() reads it', () => { + const short = Buffer.from([0xDE, 0xAD]); + const built = objToPdu({ + cmdName: 'deliver_sm', + params: { destination_addr: to, short_message: short, source_addr: from }, + tlvs: { message_payload: { tagValue: 'あいう' } }, + }); + + assert.equal(built.err, undefined); + assert.ok(built.buffer); + + const { pduObj } = pduToObj(built.buffer); + + assert.ok(pduObj); + assert.equal(pduObj.params.data_coding, 0, 'the TLV must not name an alphabet for octets nothing reads it as'); + 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({ + cmdName: 'deliver_sm', + params: { + destination_addr: to, + source_addr: from, + ...(short === undefined ? {} : { short_message: short }), + }, + tlvs: { message_payload: { tagValue: 'あいう' } }, + }); + const name = short === undefined ? 'absent' : JSON.stringify(short); + + assert.equal(built.err, undefined, name); + assert.ok(built.buffer); + + const { pduObj } = pduToObj(built.buffer); + + assert.ok(pduObj); + assert.equal(pduObj.params.data_coding, 0x08, name); + assert.equal(messageOctets(pduObj)?.toString('hex'), '304230443046', name); + } + }); + + test('ignores a short_message on a command that declares none, which the wire never carries', () => { + for (const short of ['x', Buffer.from([0xDE, 0xAD])]) { + const input: PduObjectInput = { + cmdName: 'data_sm', + params: { destination_addr: to, short_message: short, source_addr: from }, + tlvs: { message_payload: { tagValue: 'あいう' } }, + }; + const built = objToPdu(input); + + assert.equal(built.err, undefined); + assert.ok(built.buffer); + + const { pduObj } = pduToObj(built.buffer); + + assert.ok(pduObj); + assert.equal(pduObj.params.data_coding, 0x08, JSON.stringify(short)); + assert.equal(messageOctets(pduObj)?.toString('hex'), '304230443046'); + } + }); + + test('leaves data_coding at its default where an empty short_message is the whole body', () => { + for (const short of ['', Buffer.alloc(0)]) { + const built = objToPdu({ + cmdName: 'submit_sm', + params: { destination_addr: to, short_message: short, source_addr: from }, + }); + + assert.equal(built.err, undefined); + assert.ok(built.buffer); + + const { pduObj } = pduToObj(built.buffer); + + assert.ok(pduObj); + assert.equal(pduObj.params.data_coding, 0, JSON.stringify(short)); + } + }); + + test('never refuses a Buffer body, whatever the coding, because that is how binary is sent', () => { + const payload = Buffer.from([0x30, 0x42, 0xC3, 0x28, 0xFF, 0x00]); + + for (const dataCoding of [0x00, 0x01, 0x03, 0x04, 0x08, 0xF0]) { + const params = { data_coding: dataCoding, destination_addr: to, source_addr: from }; + const short = objToPdu({ cmdName: 'submit_sm', params: { ...params, short_message: payload } }); + const carried = objToPdu({ + cmdName: 'data_sm', + params, + tlvs: { message_payload: { tagValue: payload } }, + }); + + assert.deepEqual(bodyOf(short), payload, `short_message under data_coding ${String(dataCoding)}`); + assert.deepEqual(bodyOf(carried), payload, `message_payload under data_coding ${String(dataCoding)}`); + } + }); + + test('never refuses a string the caller named no data_coding for, since detection always fits', () => { + for (const message of ['Hello world', 'Åsa naïve', 'あいう', '😀 beyond the basic plane']) { + const built = objToPdu({ + cmdName: 'submit_sm', + params: { destination_addr: to, short_message: message, source_addr: from }, + }); + + assert.equal(built.err, undefined, message); + } + }); + + test('writes a body the coding does carry as the octets that alphabet spells it in', () => { + const bodies: [number, string, string][] = [ + [0x00, 'Hello world', '48656c6c6f20776f726c64'], + [0x03, 'Räksmörgås', '52e46b736df67267e573'], + [0x08, 'あいう', '304230443046'], + ]; + + for (const [dataCoding, message, hex] of bodies) { + const params = { data_coding: dataCoding, destination_addr: to, source_addr: from }; + const short = objToPdu({ cmdName: 'submit_sm', params: { ...params, short_message: message } }); + const carried = objToPdu({ + cmdName: 'data_sm', + params, + tlvs: { message_payload: { tagValue: message } }, + }); + + assert.equal(bodyOf(short)?.toString('hex'), hex, `short_message: ${message}`); + assert.equal(bodyOf(carried)?.toString('hex'), hex, `message_payload: ${message}`); + } + }); + + test('refuses the same body through session.send(), with nothing reaching the socket', async t => { + const smsc = await dummySmsc(t); + const session = await bindToSmsc(t, smsc.port, { reconnect: false }); + const sent = await session.send({ + cmdName: 'submit_sm', + params: { data_coding: 0x03, destination_addr: to, short_message: 'あいう', source_addr: from }, + }); + + assert.ok(sent.err instanceof Error); + assert.match(sent.err.message, /LATIN1/); + assert.deepEqual(smsc.octets, [], 'a body the named alphabet cannot carry never reaches the socket'); + }); +}); + describe('a time no peer can read', () => { const invalid = new Date('nope'); diff --git a/todo.md b/todo.md index 8369172..e639383 100644 --- a/todo.md +++ b/todo.md @@ -120,11 +120,6 @@ session message is a change to every call site. another message's in a correlation table. The cost is that every consumer narrows, including the majority whose SMSC always names an id. Raised by the phase 11 product review, 2026-09-08. **This one is 1.0.0-or-never** — after release it needs a major version. -- [ ] **The interop suite still asserts the defect [#95](https://github.com/larvit/larvitsmpp/pull/95) - fixed.** `interop-tests/smppsim.test.ts:621`, `a raw submit_sm with data_coding 0xF0 is not - read as flash`, fails on the next `./interop-tests/run.py smppsim`; nothing in CI runs that - suite, so it fails silently until someone does. The test name and its assertion both need - inverting, and the C17 row in `findings/02-smppsim.md` names that sub-test, so it follows. - [ ] Tag `v1.0.0` to publish. - [ ] `npm deprecate larvitsmpp` pointing at `@larvit/smpp`. Maintainer's call to run it; not something CI should do. @@ -141,16 +136,20 @@ session message is a change to every call site. ## Worth doing, not blocking -- [ ] **`objToPdu()` corrupts a string body the same way `sendSms()` used to.** - `resolveShortMessage()` in `pdu.ts` picks the codec off the caller's own `data_coding` and - encodes a string `short_message` with it, so - `objToPdu({ params: { data_coding: 3, short_message: 'あいう' } })` returns `42 44 46` — - `"BDF"` — with no error, and `data_coding` 0 flattens to spaces. Same goal 2 defect as the one - the `unencodable()` guard closed at `sendSms()`, one layer down and on the published codec. The - guard is the same call on the branch where `data_coding` is a number and the body is a string; - the `detect()` branch needs none. Held back only because a refusal there may fail - `interop-tests/`, which is being edited elsewhere and cannot be verified from here. Raised by - the architecture review of [#97](https://github.com/larvit/larvitsmpp/pull/97), 2026-09-09. +- [ ] **A send the codec will refuse waits for a link and a window slot first.** `refuse()` in + `outgoing-requests.ts` runs `misuse()` and the abort check before the wait, precisely so a call + that can never go out does not queue for what it will never use; a body `objToPdu()` refuses on + every attempt is the same case, and #98 made it a common one. On a down link the caller waits + `responseTimeout` and is told the link failed rather than that the body could not be built — + goal 2's wrong answer about what happened. The cheap fix builds the PDU twice, so the shape is + the open half. Raised by the architecture review of + [#98](https://github.com/larvit/larvitsmpp/pull/98), 2026-09-09. + +- [ ] **`message.ts` answers two questions.** Message coding and the SMPP time format (`smppDate`, + `smppTime`) share the file, which the architecture map in AGENTS.md already spells out as four + concerns. Nothing is wrong today; if the file has to move for another reason, `smpp-time.ts` is + the split. Raised by the architecture review of + [#98](https://github.com/larvit/larvitsmpp/pull/98), 2026-09-09. - [ ] **A gate that refuses a floating version anywhere in the repo.** Maintainer's ask on [#71](https://github.com/larvit/larvitsmpp/pull/71), 2026-09-06, on the `release.yaml`