From dc690b912fe73eade6082773504524fb4479bde9 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 21 Sep 2026 08:14:08 +0200 Subject: [PATCH] Name the option a refused address came from, and the data a 0.5.0 consumer stored --- CHANGELOG.md | 5 +++++ README.md | 19 +++++++++++++------ docs/decisions.md | 20 +++++++++++++------- src/defs/types.ts | 9 ++++++--- src/send-sms.ts | 17 ++++++++++++++++- test/unsendable.test.ts | 6 +++--- 6 files changed, 56 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3027ec5..1ecf03a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ `service_type` and the C-Octet String TLVs were affected the same way. A character past `U+00FF` in one of those fields is now refused, where it used to go out as its low octet — which for `一`, `　` and most emoji is `0x00`, ending the field there. + + **Check what you stored before you roll this out.** Values your application persisted under 0.5.0 + were read with bit 7 masked, so an address or a `message_id` carrying an octet above `0x7F` is + spelled differently now: a stored id will not match the receipt it belongs to, and a stored address + will not match the sender it came from. Ids most SMSCs issue are digits or hex and are unaffected. - A `U+0000` inside a C-Octet String — `source_addr`, `message_id`, `system_id` and the rest — is refused. The peer reads such a field to its first NULL, so one sent inside the value shifted every mandatory field behind it while `command_length` still counted the whole string. An Octet String diff --git a/README.md b/README.md index 1cc3dc7..53a467e 100644 --- a/README.md +++ b/README.md @@ -288,9 +288,11 @@ await session.sendSms({ ``` **Addresses.** `sourceAddrTon` and `destinationAddrTon` default to 5 for an alphanumeric address -and 1 for a numeric one; the NPI fields default to 0. An address is latin1, so `Kaffeé` reaches the -peer as its own octets; a character past `U+00FF`, or a `U+0000`, is refused rather than sent -truncated. +and 1 for a numeric one; the NPI fields default to 0. An address is latin1: `Kaffeé` goes out as the +six octets that spell it, `4B 61 66 66 65 E9`, and an address you received always sends back. One +outside `/^[\u0001-ÿ]*$/` is refused, naming the character and its index — strip or +transliterate it first. An SMSC may still refuse a non-ASCII sender of its own accord, which reaches +you as `ESME_RINVSRCADR`. **Encoding.** @@ -344,8 +346,8 @@ you formatted. Refused before anything goes out: an invalid `Date`, `NaN`, `Infi count, and a count past 99 days 23:59:59, since a count in seconds is spelled in days and below. Name a later instant as a `Date`, which goes out absolute. -**What gets checked.** The library checks what it composes: an alphabet or a time you named, a string -body under a `data_coding` you named. What you formed yourself, a `Buffer` body or a stamp you +**What gets checked.** The library checks what it composes: an address you gave as `from` or `to`, an +alphabet or a time you named, a string body under a `data_coding` you named. What you formed yourself, a `Buffer` body or a stamp you formatted, passes through as written. The same rule holds for `session.send()`. ## Session @@ -610,6 +612,9 @@ if (isCommand(pduObj, 'submit_sm')) { - A string `short_message` or `message_payload` is encoded in the alphabet the PDU's `data_coding` names, detected from the text where you name none. One that alphabet cannot carry is refused, 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, which the + peer reads as the end of the field. - A `Buffer` goes out exactly as given under any `data_coding`: binary payloads, hand-built user data headers, deliberately malformed bodies. - `session.send()` and `session.sendReturn()` build through the same codec and refuse the same bodies. @@ -707,7 +712,9 @@ Who depends on this library, and what they may rely on. spooling, scheduling, retry policy and billing belong to whatever this is the edge of, and state shared between instances goes through the store in goal 9. - **Pre-1.0, so the minor is the breaking unit** and a patch never breaks. What a 0.4.0 consumer has - to change is in [MIGRATION.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/MIGRATION.md). + to change is in [MIGRATION.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/MIGRATION.md); + what each later minor changes is in + [CHANGELOG.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/CHANGELOG.md). Personas this README serves, in order: diff --git a/docs/decisions.md b/docs/decisions.md index e911f74..b7c7bc7 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -480,19 +480,25 @@ rule and an index of the titles below. - **Every text field on the wire is latin1, and what the field cannot carry is refused rather than truncated.** Maintainer's call, 2026-09-21, the refusals from the security and stability passes on - [#16](https://gitea.larvit.se/larvit/smpp-js/pulls/16). Goal 3 settles the alphabet: 3.4 calls - these fields ASCII, and an operator routing an alphanumeric sender through the upper half is - traffic to keep, so the read is latin1 and the write is named latin1 to match — which is what - makes the round trip idempotent, and is already the octets Node put on the wire, so no peer sees a - change. Goal 2 settles the refusals, both of them a `size()` that would have agreed with a + [#16](https://gitea.larvit.se/larvit/smpp-js/pulls/16). 3.4 calls these fields ASCII, so goal 3 + settles the read alone — its generous clause is scoped to reading, and its sender clause is strict. + Goal 1 settles the write, being 3.4 as SMSCs actually run it: an operator routing an alphanumeric + sender through the upper half is traffic to keep, and Node's `ascii` write already emitted the low + octet, so `Kaffeé` went out as `4B 61 66 66 65 E9` before this change and goes out as the same + octets after it. Naming the write latin1 is what makes the round trip idempotent, and no peer sees + a change. Goal 2 settles the refusals, both of them a `size()` that would have agreed with a `write()` that put something else on the wire: a character past `U+00FF` written as its low octet, and a caller's own `U+0000`, which the peer reads as the end of the field, shifting every mandatory - field behind it under a `command_length` that counted the whole string. `wantText()` and + field behind it under a `command_length` that counted the whole string. Goal 4 settles them twice + over, since for `一`, `　` and most emoji that low octet is `0x00` and the field went out malformed + on the operator's parser. `wantText()` and `wantCstringText()` are the only two places that decide it, which is why the `dest_address` and `unsuccess_sme` structures write their embedded addresses through `cstring.write()` rather than reaching past it into `writeCstring()`. Rejected: reading latin1 and leaving the write spelled ASCII, which leaves two halves agreeing only by accident. Rejected: refusing the upper half on send - to stay strict to 3.4's ASCII, which would drop exactly the traffic the read keeps. Rejected: + to stay strict to 3.4's ASCII, which would be a new restriction taking away traffic this library + already sends and operators already accept, on no defect; whether an SMSC wants a non-ASCII sender + stays its own call, answered as `ESME_RINVSRCADR`. Rejected: refusing `U+0000` in every 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. diff --git a/src/defs/types.ts b/src/defs/types.ts index b12dd6a..dfc70ef 100644 --- a/src/defs/types.ts +++ b/src/defs/types.ts @@ -1,4 +1,5 @@ import type { Result, VoidResult } from '../result.ts'; +import { unencodableText } from './encodings.ts'; export type DestAddress = | { dest_addr_npi: number; dest_addr_ton: number; destination_addr: string } @@ -84,11 +85,11 @@ function pastLatin1(text: string): { err: Error } | undefined { if (index === -1) return undefined; - const code = (text.codePointAt(index) ?? 0).toString(16).toUpperCase().padStart(4, '0'); + const char = String.fromCodePoint(text.codePointAt(index) ?? 0); return { err: new Error( - `Character U+${code} at index ${String(index)} is past latin1, which every text field on the wire is written in`, + `latin1 cannot carry ${unencodableText({ char, index })}, and every text field on the wire is written in it; strip or transliterate it`, ), }; } @@ -113,7 +114,9 @@ function wantCstringText(value: ParamValue): Result<{ text: string }> { if (index === -1) return { text }; return { - err: new Error(`U+0000 at index ${String(index)} would end the C-Octet String there`), + err: new Error( + `U+0000 at index ${String(index)} would end the C-Octet String there, so the peer would read every field behind it shifted`, + ), }; } diff --git a/src/send-sms.ts b/src/send-sms.ts index 2223d5b..0224c33 100644 --- a/src/send-sms.ts +++ b/src/send-sms.ts @@ -7,10 +7,10 @@ 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 { cstring, paramText } from './defs/types.ts'; import { dataCodingByEncoding, 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'; import { maxSegments, smppTime, splitMessage } from './message.ts'; export type SendSmsOptions = { @@ -211,8 +211,23 @@ function checkFlash(encoding: EncodingName, flash: boolean): Error | undefined { return new Error('flash has no Latin-1 spelling: a message class carries GSM 7-bit, 8-bit data or UCS2, and 8-bit data is not text a handset will display, so send it as UCS2 or drop flash'); } +/** Asked of the wire type itself, so the codec cannot refuse an address this let through. */ +function checkAddresses(sms: SendSmsInput): Error | undefined { + for (const option of ['from', 'to'] as const) { + const { err } = cstring.size(sms[option]); + + if (err) return new Error(`${option}: ${err.message}`); + } + + return undefined; +} + /** Every option a send can be refused for, so nothing is built for a message that will not go. */ function checkOptions(sms: SendSmsInput): Result { + const unwritable = checkAddresses(sms); + + if (unwritable) return { err: unwritable }; + const mode = checkMessagingMode(sms.messagingMode, sms.dlr === true); if (mode.err) return { err: mode.err }; diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts index 07f88e5..5fae834 100644 --- a/test/unsendable.test.ts +++ b/test/unsendable.test.ts @@ -352,15 +352,15 @@ describe('an address the field cannot carry', () => { const refusals: [string, RegExp][] = [ ['46701113311\u0000EVIL', /U\+0000 at index 11/], - ['Kaffe一', /U\+4E00 at index 5/], - ['😀', /U\+1F600 at index 0/], + ['Kaffe一', /"一" \(U\+4E00\) at index 5/], + ['😀', /"😀" \(U\+1F600\) at index 0/], ]; for (const [sender, names] of refusals) { const sent = await session.sendSms({ from: sender, message: 'Hello world', to }); assert.ok(sent.err instanceof Error, sender); - assert.match(sent.err.message, /source_addr/); + assert.match(sent.err.message, /^from: /, 'names the option the caller wrote, not the wire field'); assert.match(sent.err.message, names); assert.deepEqual(sent.smsIds, [], sender); }