From c06cc0648b3f2984394c5c0817812daa66251542 Mon Sep 17 00:00:00 2001 From: Lillem4n Date: Wed, 9 Sep 2026 18:16:08 +0200 Subject: [PATCH] Refuse what we cannot send: an alphabet that cannot carry the message, and a time nobody can read (#97) * Regression tests for a named alphabet and a time that cannot carry the send * Refuse a message the named alphabet cannot carry, and a time nobody can read * Close the architecture review: a builder that cannot drop a time, and docs that hold * Refuse a time of no known kind, and clamp a relative period to what the format holds * Reflow the decision record and name the clamp in its test --- AGENTS.md | 44 +++++++++- README.md | 16 ++++ src/defs/encodings.ts | 16 ++++ src/index.ts | 4 +- src/message.ts | 50 ++++++----- src/send-sms.ts | 84 ++++++++++++++---- test/encodings.test.ts | 47 +++++++++- test/message.test.ts | 30 +++++-- test/unsendable.test.ts | 188 ++++++++++++++++++++++++++++++++++++++++ todo.md | 28 +++--- 10 files changed, 439 insertions(+), 68 deletions(-) create mode 100644 test/unsendable.test.ts diff --git a/AGENTS.md b/AGENTS.md index d9b42e6..2d2ad3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -210,9 +210,9 @@ decision under [The wire](#the-wire). helpers each file carries are copies, tolerated because a wrong one fails that file's own tests and nothing else, and a helper that only names the parameters of one `objToPdu()` call is on that same footing — it encodes no wire fact `objToPdu()` does not already own. So is a stub standing in for a - collaborator the type system already keeps in step: `recordingDeps()` in `messaging-mode.test.ts` - and `message-class.test.ts` is one `SendSmsDeps.send` that answers nothing, and a field added to - that type fails to compile in both copies at once. + collaborator the type system already keeps in step: `recordingDeps()` in `messaging-mode.test.ts`, + `message-class.test.ts` and `unsendable.test.ts` is one `SendSmsDeps.send` that answers nothing, + and a field added to that type fails to compile in every copy at once. - `message_id` values the library generates are UUID v7. - A test that needs a dummy peer must `resume()` its sockets. An unread socket never processes the peer's FIN, so `server.close()` hangs forever — that is a test bug, not a library one. @@ -316,7 +316,8 @@ Grouped by what each one constrains. every typed consumer a narrow forever — goal 6, and the tag is the last cheap chance to spend it — to guard a state the compiler refuses. Where the domain really is open the check is already there: `sendSms()` takes its options as `unknown` and refuses `encoding` by name, which is what a caller - without types gets. + without types gets. `smppTime.encode()` is where that reasoning lands the other way and is recorded + under [The wire](#the-wire): `Date | number | string` is not a closed set, so it is a `Result`. ### The wire @@ -586,6 +587,41 @@ Grouped by what each one constrains. unpacked-alphabet section above exists to stop. Accepted: a Latin-1 message past the 140 characters one SMS holds now costs more segments than it did, and `smsIds` is that much longer. +- **An alphabet the caller named has to carry the message, and a time the format cannot express is + refused, both before a segment goes out.** Maintainer's call, 2026-09-09, from the architecture and + stability reviews of [#96](https://github.com/larvit/larvitsmpp/pull/96): `encoding: 'LATIN1'` on + `あいう` put `42 44 46` — `"BDF"` — on the wire and returned success, `encoding: 'ASCII'` + flattened every character outside 03.38 to a space, and `validityPeriod: new Date('nope')` wrote + `NaNNaNNaNNaNNaNNaNNaN00+` into the PDU. Goal 2 owns all three: bytes that do not say what the + caller asked, reported as sent. `unencodable()` is the single answer to whether an alphabet can + carry a message, as `messageClassOf()` is to whether a `data_coding` carries a class, and it asks + the codec — `decode(encode(c)) === c` per code point — rather than restating the tables beside it, + so the guard cannot drift from what the encoder writes for any one character, and a fourth + alphabet answers by having a codec at all. It is exported for the reason `concatOf()` is: a caller + composing a `submit_sm` through `send()` and `encodeMessage()` would otherwise rewrite the read + this fixed. `match()` cannot be that answer — it doubles as the auto-selection policy `detect()` + reads, where LATIN1 is hardcoded false so nothing picks it, and using it would refuse the 8-bit + binary body Latin-1 is kept for. The guard is + on the named branch alone, so an unspecified send is untouched: every alphabet `detect()` returns + carries every character it was picked for, over the whole code point range. `smppTime.encode()` + returns a `Result`, where the three encoding helpers stayed total: that argument was that + `EncodingName` is a closed set the compiler guards, and `Date | number | string` is not — an + invalid `Date` and `NaN` inhabit it, which hard rule 1 makes a result "wherever the types admit + one", and `decode()` has been fallible for the same reason since it was written. Rejected: + transcoding to UCS2, which overrides the one option the caller wrote in order to override a choice + — the same reason a flash Latin-1 message is refused rather than promoted, and an operator that + accepts only `data_coding` 0x03 would be handed something it never agreed to take. Rejected: + guarding `sendSms()` alone and leaving `smppTime.encode()` writing `NaN`s, which leaves this + library's own published helper composing the garbage the guard exists to stop. Rejected: a + `holds()` member beside `match()` on `Encoding`, a second per-alphabet table to keep in step with + the codec. Rejected: validating the `string` spelling of a time, which is a stamp the caller + formatted for a peer whose format is theirs to name, its width included, where SMPP 3.4 gives the + field 1 or 17 octets. Accepted: `Infinity` seconds is refused rather than clamped, where a real + period past the 99d 23:59:59 the format holds is still clamped to it. Accepted: GSM's 0x1B is an + extension prefix rather than a character, so a bare ESC beside one of the ten extension bases is + 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. + ### 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 08d3626..711b0d5 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,16 @@ segment goes out. anything else is refused by name rather than guessed at. Leave it out and a message that fits GSM 7-bit goes as `ASCII` and everything else as `UCS2`; `LATIN1` is only ever used when you name it. +An alphabet you name has to carry the message: `LATIN1` beside a character above U+00FF, or `ASCII` +beside one GSM 03.38 has no code for, is refused before anything goes out, and the error names the +character, its code point and where in the message it is. Leaving `encoding` out is never refused — +what detection picks always fits. `LATIN1` still carries every octet, so an 8-bit binary body sent +as `buffer.toString('latin1')` goes out unchanged. + +`scheduleDeliveryTime` and `validityPeriod` take a `Date`, a number of seconds, or a stamp you +formatted yourself. One that names no time — an invalid `Date`, `NaN`, `Infinity` — is refused +before anything goes out. + `flash` asks for GSM 03.38 message class 0, the class a handset shows on arrival instead of storing. It travels in `data_coding` beside the alphabet, so a flash UCS2 message stays UCS2. Pairing it with `encoding: 'LATIN1'` is the one combination with nowhere to go — a message class carries GSM 7-bit, @@ -567,6 +577,12 @@ 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. +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)` +returns `{ err, text }` for a `validity_period` or `schedule_delivery_time`, as `smppTime.decode()` +returns `{ err, date }` for one that arrived. + The spec tables are exported both individually (`cmds`, `consts`, `encodings`, `errors`, `tlvs`, `types`, and the matching `*ById` maps) and grouped as `defs`. diff --git a/src/defs/encodings.ts b/src/defs/encodings.ts index 75eccf0..23a62d7 100644 --- a/src/defs/encodings.ts +++ b/src/defs/encodings.ts @@ -121,6 +121,22 @@ export function detect(value: string): EncodingName { return 'UCS2'; } +export type Unencodable = { char: string; index: number }; + +/** 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]; + let index = 0; + + for (const char of message) { + if (codec.decode(codec.encode(char)) !== char) return { char, index }; + + index += char.length; + } + + return undefined; +} + /** * The GSM 03.38 section 4 message class a `data_coding` octet carries, in bits 1-0, or undefined * where its coding group carries none. Below 0x80 bit 4 says whether one is there; 0xF0 always is. diff --git a/src/index.ts b/src/index.ts index 44a2c9b..33f9a9e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,7 +4,7 @@ export { Session } from './session.ts'; export { cmds, cmdsById, commandNameById, isCommandName } from './defs/commands.ts'; export { consts, constsById } from './defs/constants.ts'; -export { detect, encodingByDataCoding, encodings, isEncodingName, messageClassOf } from './defs/encodings.ts'; +export { 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 { types } from './defs/types.ts'; @@ -62,7 +62,7 @@ export type { } from './session.ts'; export type { CommandName, PduParams, PduParamsInput } from './defs/commands.ts'; export type { ConstGroup, MessageState, SubmitMessagingMode } from './defs/constants.ts'; -export type { Encoding, EncodingName } from './defs/encodings.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 { PduHeader } from './pdu-refusal.ts'; diff --git a/src/message.ts b/src/message.ts index 7b02be7..baf369e 100644 --- a/src/message.ts +++ b/src/message.ts @@ -119,33 +119,41 @@ const relativeTime = /^(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)000R$/; /** The SMPP absolute and relative time format, as used by validity_period and friends. */ export const smppTime = { /** - * A Date becomes an absolute UTC time; a number is a relative period in seconds. Relative - * periods are expressed in days and below, so anything past 99 days is clamped to that. + * A Date becomes an absolute UTC time; a number is a relative period in seconds, expressed in + * days and below, so anything past the 99d 23:59:59 the format holds is clamped to it; a string + * is a stamp the caller formatted itself and is passed through. A value naming no instant or + * period is refused. */ - encode(value: Date | number | string): string { - if (typeof value === 'string') return value; + encode(value: Date | number | string): Result<{ text: string }> { + if (typeof value === 'string') return { text: value }; if (typeof value === 'number') { - const total = Math.max(0, Math.floor(value)); - const days = Math.min(99, Math.floor(total / 86400)); - const capped = days === 99 ? 99 * 86400 + 86399 : total; + if (!Number.isFinite(value)) return { err: new Error(`Not an SMPP time: ${String(value)}`) }; - return '0000' - + pad(Math.floor(capped / 86400), 2) - + pad(Math.floor(capped / 3600) % 24, 2) - + pad(Math.floor(capped / 60) % 60, 2) - + pad(capped % 60, 2) - + '000R'; + const capped = Math.min(Math.max(0, Math.floor(value)), 99 * 86400 + 86399); + + return { + text: '0000' + + pad(Math.floor(capped / 86400), 2) + + pad(Math.floor(capped / 3600) % 24, 2) + + pad(Math.floor(capped / 60) % 60, 2) + + pad(capped % 60, 2) + + '000R', + }; } - return pad(value.getUTCFullYear() % 100, 2) - + pad(value.getUTCMonth() + 1, 2) - + pad(value.getUTCDate(), 2) - + pad(value.getUTCHours(), 2) - + pad(value.getUTCMinutes(), 2) - + pad(value.getUTCSeconds(), 2) - + pad(Math.floor(value.getUTCMilliseconds() / 100), 1) - + '00+'; + if (Number.isNaN(value.getTime())) return { err: new Error('Not an SMPP time: an invalid Date') }; + + return { + text: pad(value.getUTCFullYear() % 100, 2) + + pad(value.getUTCMonth() + 1, 2) + + pad(value.getUTCDate(), 2) + + pad(value.getUTCHours(), 2) + + pad(value.getUTCMinutes(), 2) + + pad(value.getUTCSeconds(), 2) + + pad(Math.floor(value.getUTCMilliseconds() / 100), 1) + + '00+', + }; }, decode(value: string): Result<{ date: Date }> { diff --git a/src/send-sms.ts b/src/send-sms.ts index 9e0d1e7..fd122b4 100644 --- a/src/send-sms.ts +++ b/src/send-sms.ts @@ -1,4 +1,4 @@ -import type { EncodingName } from './defs/encodings.ts'; +import type { EncodingName, Unencodable } from './defs/encodings.ts'; import type { ParamValue } from './defs/types.ts'; import type { SubmitMessagingMode } from './defs/constants.ts'; import type { PduObject, PduObjectInput } from './pdu.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 } from './defs/encodings.ts'; +import { detect, encodingNames, isEncodingName, unencodable } from './defs/encodings.ts'; import { namedValue } from './error-from.ts'; import { normaliseSmsId } from './sms-id.ts'; import { paramText } from './defs/types.ts'; @@ -33,10 +33,13 @@ export type SendSmsOptions = { }; /** The options as they arrive: a caller without types can put anything in the checked fields. */ -export type SendSmsInput = Omit & { - encoding?: unknown; - messagingMode?: unknown; -}; +export type SendSmsInput = + Omit & { + encoding?: unknown; + messagingMode?: unknown; + scheduleDeliveryTime?: unknown; + validityPeriod?: unknown; + }; /** Both arrays hold what the peer accepted, so a partial failure names what is already delivered. */ export type SendSmsResult = { @@ -60,16 +63,24 @@ export type SendSmsDeps = { send: (input: PduObjectInput) => Promise>; }; +/** The time fields as the wire spells them, settled once for a message rather than per segment. */ +type SendTimes = { + scheduleDeliveryTime?: string; + validityPeriod?: string; +}; + /** What the checks below settle, before a segment exists to carry it. */ type CheckedOptions = { encoding: EncodingName; messagingMode: SubmitMessagingMode; + times: SendTimes; }; type SegmentOptions = { encoding: EncodingName; messagingMode?: SubmitMessagingMode; multipart: boolean; + times?: SendTimes; }; /** Alphanumeric senders must be TON 5. */ @@ -92,7 +103,7 @@ function esmClassFor(mode: SubmitMessagingMode | undefined, multipart: boolean): } export function submitSmParams( - sms: SendSmsInput, + sms: Omit, segment: Buffer, options: SegmentOptions, ): Record { @@ -108,13 +119,12 @@ export function submitSmParams( source_addr_ton: sms.sourceAddrTon ?? addressTon(sms.from), }; + const schedule = options.times?.scheduleDeliveryTime; + const validity = options.times?.validityPeriod; + if (sms.dlr === true) params.registered_delivery = consts.REGISTERED_DELIVERY.FINAL; - if (sms.scheduleDeliveryTime !== undefined) { - params.schedule_delivery_time = smppTime.encode(sms.scheduleDeliveryTime); - } - if (sms.validityPeriod !== undefined) { - params.validity_period = smppTime.encode(sms.validityPeriod); - } + if (schedule !== undefined) params.schedule_delivery_time = schedule; + if (validity !== undefined) params.validity_period = validity; return params; } @@ -157,11 +167,42 @@ function refusedEncoding(encoding: unknown): Error { return new Error(`encoding must be ${encodingNames.join(', ')}, got ${namedValue(encoding)}`); } +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`); +} + +/** An alphabet the caller named has to carry the message; the one detect() picks always does. */ function checkEncoding(encoding: unknown, message: string): Result<{ encoding: EncodingName }> { if (encoding === undefined) return { encoding: detect(message) }; - if (isEncodingName(encoding)) return { encoding }; + if (!isEncodingName(encoding)) return { err: refusedEncoding(encoding) }; - return { err: refusedEncoding(encoding) }; + const lost = unencodable(message, encoding); + + return lost ? { err: refusedText(encoding, lost) } : { encoding }; +} + +function checkTimes(sms: SendSmsInput): Result<{ times: SendTimes }> { + const times: SendTimes = {}; + + for (const option of ['scheduleDeliveryTime', 'validityPeriod'] as const) { + const value = sms[option]; + + if (value === undefined) continue; + + if (typeof value !== 'number' && typeof value !== 'string' && !(value instanceof Date)) { + return { err: new Error(`${option} must be a Date, a number of seconds or an SMPP stamp, got ${namedValue(value)}`) }; + } + + const encoded = smppTime.encode(value); + + if (encoded.err) return { err: new Error(`${option}: ${encoded.err.message}`) }; + + times[option] = encoded.text; + } + + return { times }; } /** GSM 03.38 section 4 gives the class groups GSM 7-bit, 8-bit data and UCS2, and no Latin-1 at all. */ @@ -185,7 +226,11 @@ function checkOptions(sms: SendSmsInput): Result { if (unspellable) return { err: unspellable }; - return { encoding: chosen.encoding, messagingMode: mode.messagingMode }; + const times = checkTimes(sms); + + if (times.err) return { err: times.err }; + + return { encoding: chosen.encoding, messagingMode: mode.messagingMode, times: times.times }; } /** Nothing goes on the wire until the whole message fits: a half-sent message bills twice. */ @@ -252,7 +297,12 @@ export async function submitSms(deps: SendSmsDeps, sms: SendSmsInput): Promise deps.send({ cmdName: 'submit_sm', - params: submitSmParams(sms, segment, { encoding, messagingMode: options.messagingMode, multipart }), + params: submitSmParams(sms, segment, { + encoding, + messagingMode: options.messagingMode, + multipart, + times: options.times, + }), }))); return collectSent(sent, deps.respIdNotation); diff --git a/test/encodings.test.ts b/test/encodings.test.ts index b16b627..18d531f 100644 --- a/test/encodings.test.ts +++ b/test/encodings.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import test, { describe } from 'node:test'; -import { detect, encodingByDataCoding, encodings } from '../src/defs/encodings.ts'; +import { detect, encodingByDataCoding, encodings, unencodable } from '../src/defs/encodings.ts'; describe('ASCII (GSM 03.38)', () => { const samples: [string, number[]][] = [ @@ -105,6 +105,51 @@ describe('detect()', () => { assert.equal(detect('تست'), 'UCS2'); assert.equal(detect('۱۲۳۴۵۶۷۸۹۰'), 'UCS2'); }); + + // Exhaustive: sendSms() leaves the automatic path unchecked on this and nothing else. + test('never picks an alphabet that loses a character, for any code point there is', () => { + for (let code = 0; code <= 0x10FFFF; code++) { + if (code >= 0xD800 && code <= 0xDFFF) continue; + + const char = String.fromCodePoint(code); + const lost = unencodable(char, detect(char)); + + if (lost) assert.fail(`detect() picked ${detect(char)} for U+${code.toString(16)}, which loses it`); + } + }); +}); + +describe('unencodable()', () => { + test('names the first character an alphabet cannot carry, and nothing where it carries them all', () => { + assert.deepEqual(unencodable('あいう', 'LATIN1'), { char: 'あ', index: 0 }); + assert.deepEqual(unencodable('Åsa naïve', 'ASCII'), { char: 'ï', index: 6 }); + assert.equal(unencodable('€{}[]\\~^|\f', 'ASCII'), undefined); + assert.equal(unencodable('Åsa', 'ASCII'), undefined); + assert.equal(unencodable('`ÁáçÚ', 'LATIN1'), undefined); + assert.equal(unencodable('あいう😀', 'UCS2'), undefined); + }); + + test('counts the index in the units the message is written in, so a surrogate pair reads back whole', () => { + assert.deepEqual(unencodable('ab😀', 'LATIN1'), { char: '😀', index: 2 }); + assert.deepEqual(unencodable('a😀b', 'ASCII'), { char: '😀', index: 1 }); + }); + + test('carries every octet through Latin-1, which is what an 8-bit binary body is sent as', () => { + const every = Buffer.from(Array.from({ length: 256 }, (_, byte) => byte)); + + assert.equal(unencodable(every.toString('latin1'), 'LATIN1'), undefined); + assert.deepEqual(unencodable('Ā', 'LATIN1'), { char: 'Ā', index: 0 }); + }); + + test('is not match(), which says what detect() may pick rather than what the alphabet holds', () => { + assert.equal(encodings.LATIN1.match('Hello world'), false); + assert.equal(unencodable('Hello world', 'LATIN1'), undefined); + }); + + test('reads a character at a time, so a bare GSM escape beside its base reads as carried', () => { + assert.equal(unencodable('\x1Be', 'ASCII'), undefined); + assert.deepEqual(encodings.ASCII.encode('\x1Be'), encodings.ASCII.encode('€')); + }); }); describe('encodingByDataCoding()', () => { diff --git a/test/message.test.ts b/test/message.test.ts index 68bc0fa..78aaf7e 100644 --- a/test/message.test.ts +++ b/test/message.test.ts @@ -234,25 +234,45 @@ describe('smppDate()', () => { describe('smppTime', () => { test('encodes an absolute time', () => { - assert.equal(smppTime.encode(new Date(Date.UTC(2026, 7, 25, 14, 30, 0))), '260825143000000+'); + assert.equal(smppTime.encode(new Date(Date.UTC(2026, 7, 25, 14, 30, 0))).text, '260825143000000+'); }); - test('encodes a relative time given in seconds', () => { - assert.equal(smppTime.encode(3600), '000000010000000R'); + test('encodes a relative time given in seconds, clamped to what the format holds', () => { + assert.equal(smppTime.encode(3600).text, '000000010000000R'); + assert.equal(smppTime.encode(99 * 86400).text, '000099000000000R'); + assert.equal(smppTime.encode(100 * 86400).text, '000099235959000R'); }); test('passes an already-formatted string through', () => { - assert.equal(smppTime.encode('260825143000000+'), '260825143000000+'); + assert.equal(smppTime.encode('260825143000000+').text, '260825143000000+'); }); test('decodes an absolute time back to the same instant', () => { const when = new Date(Date.UTC(2026, 7, 25, 14, 30, 0)); - const { err, date } = smppTime.decode(smppTime.encode(when)); + const encoded = smppTime.encode(when); + + assert.ok(encoded.text); + + const { err, date } = smppTime.decode(encoded.text); assert.equal(err, undefined); assert.equal(date.toISOString(), when.toISOString()); }); + test('reports a time it cannot express rather than encoding it as a string of NaNs', () => { + for (const value of [new Date('nope'), NaN, Infinity, -Infinity]) { + const encoded = smppTime.encode(value); + + assert.ok(encoded.err instanceof Error, String(value)); + assert.equal(encoded.text, undefined); + } + + const invalidDate = smppTime.encode(new Date('nope')); + + assert.ok(invalidDate.err); + assert.match(invalidDate.err.message, /invalid Date/); + }); + test('reports malformed input rather than returning an invalid date', () => { assert.ok(smppTime.decode('nonsense').err instanceof Error); assert.ok(smppTime.decode('').err instanceof Error); diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts new file mode 100644 index 0000000..b15f297 --- /dev/null +++ b/test/unsendable.test.ts @@ -0,0 +1,188 @@ +import assert from 'node:assert/strict'; +import test, { describe } from 'node:test'; +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 { 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'; + +const from = '46701113311'; +const to = '46709771337'; + +/** A send that records what it is handed, so a refusal shows up as an empty attempt log. */ +function recordingDeps(attempts: PduObjectInput[]): SendSmsDeps { + return { + log: silentLog, + reference: 1, + send: input => { + attempts.push(input); + + return Promise.resolve({ err: new Error('the recording peer never answers') }); + }, + }; +} + +/** The alphabet each submit_sm declared, beside the text those octets read back as. */ +function sentAs(octets: Buffer[]): [number, string][] { + return octets.map(pdu => { + const { pduObj } = pduToObj(pdu); + + assert.ok(pduObj); + + const dataCoding = paramNumber(pduObj.params.data_coding, 0); + + return [dataCoding, decodeMessage(pduObj.shortMessageOctets ?? Buffer.alloc(0), dataCoding).message]; + }); +} + +describe('an alphabet the caller named that cannot carry the message', () => { + // Latin-1 takes the low octet of every code point, so あ (U+3042) went out as 0x42 — the letter B. + test('refuses a Latin-1 send of a character above 0xFF, naming the character', async () => { + const attempts: PduObjectInput[] = []; + const sent = await submitSms(recordingDeps(attempts), { encoding: 'LATIN1', from, message: 'あいう', to }); + + assert.ok(sent.err instanceof Error); + assert.match(sent.err.message, /LATIN1/); + assert.match(sent.err.message, /"あ"/); + assert.match(sent.err.message, /U\+3042/); + assert.deepEqual(sent.pduObjs, []); + assert.deepEqual(sent.smsIds, []); + assert.equal(sent.unanswered, 0); + assert.equal(attempts.length, 0, 'a message the named alphabet cannot carry puts nothing on the wire'); + }); + + // Å is in the GSM table at 0x0E; ï is the one the encoder flattened to a space. + test('refuses an ASCII send of a character GSM 03.38 has no code for, naming that one', async () => { + const attempts: PduObjectInput[] = []; + const sent = await submitSms(recordingDeps(attempts), { encoding: 'ASCII', from, message: 'Åsa naïve', to }); + + assert.ok(sent.err instanceof Error); + assert.match(sent.err.message, /ASCII/); + assert.match(sent.err.message, /"ï"/); + assert.match(sent.err.message, /U\+00EF/); + assert.match(sent.err.message, /index 6/); + assert.equal(attempts.length, 0); + }); + + test('never refuses UCS2, which carries every character there is', async () => { + const attempts: PduObjectInput[] = []; + const deps = recordingDeps(attempts); + + for (const message of ['あいう', 'Åsa naïve', '😀 beyond the basic plane', '�']) { + const sent = await submitSms(deps, { encoding: 'UCS2', from, message, to }); + + assert.equal(sent.err?.message, 'the recording peer never answers', message); + } + + assert.equal(attempts.length, 4); + }); + + test('sends 8-bit binary named as Latin-1 unchanged, the case that codec is named for', async t => { + const messageId = '01a086cc-5ee9-759d-ba4c-96a9afd9aed9'; + const smsc = await dummySmsc(t, { messageIds: [messageId] }); + const session = await bindToSmsc(t, smsc.port, { reconnect: false }); + const payload = Buffer.from(Array.from({ length: 140 }, (_, at) => (at * 7 + 3) % 256)); + const sent = await session.sendSms({ encoding: 'LATIN1', from, message: payload.toString('latin1'), to }); + const first = smsc.octets[0]; + + assert.equal(sent.err, undefined); + assert.deepEqual(sent.smsIds, [messageId]); + assert.equal(smsc.octets.length, 1); + assert.ok(first); + + const { pduObj } = pduToObj(first); + + assert.ok(pduObj); + assert.equal(paramNumber(pduObj.params.data_coding, 0), 0x03); + assert.deepEqual(pduObj.shortMessageOctets, payload); + }); + + test('picks an alphabet that fits where the caller named none, so nothing is ever refused for one', async t => { + const smsc = await dummySmsc(t); + const session = await bindToSmsc(t, smsc.port, { reconnect: false }); + + for (const message of ['Hello world', 'Åsa naïve', 'あいう']) { + assert.equal((await session.sendSms({ from, message, to })).err, undefined, message); + } + + assert.deepEqual(sentAs(smsc.octets), [[0x01, 'Hello world'], [0x08, 'Åsa naïve'], [0x08, 'あいう']]); + }); +}); + +describe('a time no peer can read', () => { + const invalid = new Date('nope'); + + test('refuses an invalid Date rather than writing NaN into the PDU, naming the option', async () => { + const attempts: PduObjectInput[] = []; + const deps = recordingDeps(attempts); + const sends: [string, SendSmsInput][] = [ + ['scheduleDeliveryTime', { from, message: 'Hello world', scheduleDeliveryTime: invalid, to }], + ['validityPeriod', { from, message: 'Hello world', to, validityPeriod: invalid }], + ]; + + for (const [option, send] of sends) { + const sent = await submitSms(deps, send); + + assert.ok(sent.err instanceof Error, option); + assert.match(sent.err.message, new RegExp(option)); + assert.match(sent.err.message, /invalid Date/); + assert.deepEqual(sent.smsIds, []); + } + + assert.equal(attempts.length, 0, 'a time nobody can read puts nothing on the wire'); + }); + + test('refuses a relative period naming no number of seconds', async () => { + const attempts: PduObjectInput[] = []; + const deps = recordingDeps(attempts); + + for (const validityPeriod of [NaN, Infinity, -Infinity]) { + const sent = await submitSms(deps, { from, message: 'Hello world', to, validityPeriod }); + + assert.ok(sent.err instanceof Error, String(validityPeriod)); + assert.match(sent.err.message, /validityPeriod/); + } + + assert.equal(attempts.length, 0); + }); + + test('refuses a time that is no kind of time rather than throwing out of the send', async () => { + const attempts: PduObjectInput[] = []; + const deps = recordingDeps(attempts); + + for (const validityPeriod of [null, {}, true, []]) { + const sent = await submitSms(deps, { from, message: 'Hello world', to, validityPeriod }); + + assert.ok(sent.err instanceof Error, JSON.stringify(validityPeriod)); + assert.match(sent.err.message, /validityPeriod must be a Date/); + } + + assert.equal(attempts.length, 0); + }); + + test('writes the times it can express into every segment it built them for', async t => { + const smsc = await dummySmsc(t); + const session = await bindToSmsc(t, smsc.port, { reconnect: false }); + const sent = await session.sendSms({ + from, + message: 'A message long enough to need a header numbering its segments. '.repeat(4), + scheduleDeliveryTime: new Date(Date.UTC(2026, 8, 9, 14, 30, 0)), + to, + validityPeriod: 3600, + }); + + assert.equal(sent.err, undefined); + assert.equal(smsc.octets.length, 2, 'the fixture must need more than one segment'); + + for (const octets of smsc.octets) { + const { pduObj } = pduToObj(octets); + + assert.ok(pduObj); + assert.equal(paramText(pduObj.params.schedule_delivery_time), '260909143000000+'); + assert.equal(paramText(pduObj.params.validity_period), '000000010000000R'); + } + }); +}); diff --git a/todo.md b/todo.md index 90126a2..8369172 100644 --- a/todo.md +++ b/todo.md @@ -141,24 +141,16 @@ session message is a change to every call site. ## Worth doing, not blocking -- [ ] **A named alphabet that cannot hold the message corrupts it instead of refusing it.** - `encodeMessage('あいう', 'LATIN1')` returns `42 44 46` — `"BDF"` — and `sendSms()` puts that on - the wire: `checkOptions()` refuses an unknown name, `FLASH` and flash-beside-Latin-1, but never - asks whether the alphabet the caller named can carry the text. GSM 7-bit does the same, mapping - anything outside 03.38 to a space. `Encoding.match()` cannot be the guard — `latin1.match()` is - hardcoded `false` because it doubles as the auto-selection policy `detect()` reads, so "can hold - this" and "should be picked for this" would have to be separated first. Send-side, so the tag - is not blocked on it. Raised by the architecture review of - [#96](https://github.com/larvit/larvitsmpp/pull/96), 2026-09-09. - -- [ ] **A time nobody can read goes out as `NaN` rather than being refused.** - `smppTime.encode(new Date('nope'))` returns `NaNNaNNaNNaNNaNNaNNaN00+` and - `smppTime.encode(NaN)` returns `0000NaNNaNNaNNaN000R`; `sendSms({ validityPeriod })` and - `scheduleDeliveryTime` write either straight into the PDU with no check at - `submitSmParams()`. Goal 2, since the peer reads a field that means nothing. `Date` is not the - closed domain hard rule 1's totality clause covers, so this one wants a guard at the send - boundary rather than a `Result` on `smppTime`. Raised by the stability review of - [#96](https://github.com/larvit/larvitsmpp/pull/96), 2026-09-09. +- [ ] **`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 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`