From a7eb4923f5d341023a5b5598739deff53afb6566 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 21 Sep 2026 08:20:59 +0200 Subject: [PATCH] Say what is true for a TLV too, and cover the to half of the check --- README.md | 17 +++++++++-------- docs/decisions.md | 25 +++++++++++-------------- src/defs/types.ts | 6 ++---- test/unsendable.test.ts | 19 ++++++++++--------- todo.md | 6 ++++++ 5 files changed, 38 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 53a467e..5305de7 100644 --- a/README.md +++ b/README.md @@ -288,11 +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: `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`. +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-\u00FF]*$/` 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 a refusal such as `ESME_RINVSRCADR`. **Encoding.** @@ -346,9 +346,10 @@ 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 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()`. +**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 diff --git a/docs/decisions.md b/docs/decisions.md index b7c7bc7..061277d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -483,24 +483,21 @@ rule and an index of the titles below. [#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. 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 + sender through the upper half is traffic to keep, and Node's `ascii` write already put those octets + on the wire, so naming the write latin1 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 a mandatory field's reader takes as the end of the field. Goal 4 settles them + twice over: for a great many characters that low octet is `0x00`, and the PDU 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 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. + already sends and operators already accept, on no defect. 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. ## The session's life diff --git a/src/defs/types.ts b/src/defs/types.ts index dfc70ef..5a24a85 100644 --- a/src/defs/types.ts +++ b/src/defs/types.ts @@ -96,7 +96,7 @@ function pastLatin1(text: string): { err: Error } | undefined { function wantText(value: ParamValue): Result<{ text: string }> { if (typeof value !== 'number' && typeof value !== 'string') { - return { err: new Error(`Expected a string, got ${typeof value}`) }; + return { err: new Error(`Expected a string or a number, got ${typeof value}`) }; } const text = String(value); @@ -114,9 +114,7 @@ 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, so the peer would read every field behind it shifted`, - ), + err: new Error(`U+0000 at index ${String(index)} would end the C-Octet String there`), }; } diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts index 5fae834..3c8c7e3 100644 --- a/test/unsendable.test.ts +++ b/test/unsendable.test.ts @@ -350,19 +350,20 @@ describe('an address the field cannot carry', () => { const smsc = await dummySmsc(t); const session = await bindToSmsc(t, smsc.port, { reconnect: false }); - const refusals: [string, RegExp][] = [ - ['46701113311\u0000EVIL', /U\+0000 at index 11/], - ['Kaffe一', /"一" \(U\+4E00\) at index 5/], - ['😀', /"😀" \(U\+1F600\) at index 0/], + const refusals: ['from' | 'to', string, RegExp][] = [ + ['from', '46701113311\u0000EVIL', /U\+0000 at index 11/], + ['from', 'Kaffe一', /"一" \(U\+4E00\) at index 5/], + ['from', '😀', /"😀" \(U\+1F600\) at index 0/], + ['to', 'Kaffe一', /"一" \(U\+4E00\) at index 5/], ]; - for (const [sender, names] of refusals) { - const sent = await session.sendSms({ from: sender, message: 'Hello world', to }); + for (const [option, address, names] of refusals) { + const sent = await session.sendSms({ from, message: 'Hello world', to, [option]: address }); - assert.ok(sent.err instanceof Error, sender); - assert.match(sent.err.message, /^from: /, 'names the option the caller wrote, not the wire field'); + assert.ok(sent.err instanceof Error, address); + assert.match(sent.err.message, new RegExp(`^${option}: `), 'names the option the caller wrote'); assert.match(sent.err.message, names); - assert.deepEqual(sent.smsIds, [], sender); + assert.deepEqual(sent.smsIds, [], address); } assert.deepEqual(smsc.octets, [], 'an address the field cannot carry never reaches the socket'); diff --git a/todo.md b/todo.md index 685cf8a..e062053 100644 --- a/todo.md +++ b/todo.md @@ -189,6 +189,12 @@ and is also what the panel ranked hardest — two methods, one answer. ### Correctness, ahead of everything below +- [ ] **Refuse a non-finite number where a text field coerces one.** `wantText()` in `defs/types.ts` + stringifies a number so `message_id: 123` writes `"123"`, which is deliberate and tested. It + takes `NaN` and `Infinity` on the same path, so `sendSms({ from: NaN })` puts the literal sender + `NaN` on the wire and reports success — goal 2. Gate the numeric branch on `Number.isFinite`. + Predates the latin1 guard; found by the stability review of #16. + - [ ] **Answer `alert_notification` and `outbind` by not answering them.** Both are response-less in SMPP 3.4, both fall through `route()`'s default into `unhandled()`, which calls `sendReturn(pduObj, 'ESME_RINVCMDID')`; `pduReturn()` then finds no response command, and the