diff --git a/AGENTS.md b/AGENTS.md index 9eb8b75..7277e6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -616,8 +616,15 @@ Grouped by what each one constrains. `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 + field 1 or 17 octets. Accepted: a second count past 99d 23:59:59 is refused rather than clamped to + it, a negative one and `Infinity` with it — clamping `86400 * 365` reported success for a year and + put 99 days on the wire, the wrong answer about what happened that the rest of this bullet exists + to remove. The ceiling is this encoder's rather than the format's: 3.4's `YYMMDDhhmmss000R` + carries years and months, which `decode()` reads back, and no fixed number of seconds is either + one, so spelling a second count in days and below is where the guess would go — which is why the + too-long refusal names the `Date` that reaches every instant the absolute form holds, and the + negative one names nothing, there being no period to reach. Rejected: documenting the clamp, which + leaves the caller told a true thing and still sent the wrong period. 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. @@ -634,7 +641,12 @@ Grouped by what each one constrains. 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 + character, its code point and its index said once for both refusals — unexported where + `unencodable()` is published, since wording `{ char, index }` into a sentence rewrites no read a + caller would get wrong, where asking the codec is, and publishing it would freeze this library's + error prose as API for an application whose own refusal should read like itself. Goal 6, from the + architecture review of [#99](https://github.com/larvit/larvitsmpp/pull/99), 2026-09-09. 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, diff --git a/README.md b/README.md index c1b45c8..b8ee462 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,11 @@ await session.sendSms({ }, { signal }); // optional per-call AbortSignal ``` +One rule decides which choices of yours this library second-guesses, here and on `session.send()`: +it checks what it composes for you — an alphabet or a time you named, a string body under a +`data_coding` you named — and passes on whatever you formed yourself, a `Buffer` body or a stamp you +formatted. + `sourceAddrTon` and `destinationAddrTon` default to 5 for an alphanumeric address and 1 for a numeric one; the NPI fields default to 0. Set them for an operator that requires something else. @@ -125,16 +130,22 @@ segment goes out. `encoding` names the alphabet, and is `ASCII` (GSM 03.38's own 7-bit table), `LATIN1` or `UCS2`; 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. +`consts.ENCODING` is the low-level surface's `data_coding` table rather than this option's list, +`FLASH` and the alphabets no codec here implements included. 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. +what detection picks always fits. `LATIN1` carries every octet, so an 8-bit body passed as +`buffer.toString('latin1')` reaches the SMSC byte for byte — but under `data_coding` 0x03, which +declares Latin-1 text rather than binary. To declare 8-bit binary, hand `session.send()` a `Buffer` +body and the `data_coding` you want; see [Working with PDUs directly](#working-with-pdus-directly). `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. +before anything goes out, and so is a negative number of seconds or one past 99 days 23:59:59: a +period counted in seconds is spelled in days and below, since no fixed number of them is a month or +a year. Name an instant further out as a `Date`, which goes out absolute. `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 @@ -431,7 +442,9 @@ A refused inbound `deliver_sm` is lost traffic: a message or a receipt that neve `dlr`, and this event is where that loss shows up. A refused *response* is reported twice where a call is still waiting on it, once as the `err` that `sendSms()` or `send()` returns and once here. That is deliberate: the call answers what became of that one send, and the event is what shows a peer -answering unreadably at all. +answering unreadably at all. A `PduRefusedError` is always a PDU that arrived, so that is the only +way one reaches a send's `err`: what this library refuses to build or send — an alphabet, a time, a +body a `data_coding` cannot carry — is a plain `Error` in the result. ## Logging diff --git a/src/message.ts b/src/message.ts index 2cb6ebe..d4c5719 100644 --- a/src/message.ts +++ b/src/message.ts @@ -132,6 +132,9 @@ export function smppDate(date: Date): string { + pad(date.getUTCMinutes(), 2); } +/** A second count is spelled in days and below: no fixed number of seconds is a month or a year. */ +const maxRelativeSeconds = 99 * 86400 + 86399; + const absoluteTime = /^(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)(\d)(\d\d)([+-])$/; const relativeTime = /^(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)000R$/; @@ -139,9 +142,8 @@ const relativeTime = /^(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)000R$/; export const smppTime = { /** * 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. + * days and below; a string is a stamp the caller formatted itself and is passed through. A value + * naming no instant or period, and a second count no day field reaches, are refused. */ encode(value: Date | number | string): Result<{ text: string }> { if (typeof value === 'string') return { text: value }; @@ -149,14 +151,22 @@ export const smppTime = { if (typeof value === 'number') { if (!Number.isFinite(value)) return { err: new Error(`Not an SMPP time: ${String(value)}`) }; - const capped = Math.min(Math.max(0, Math.floor(value)), 99 * 86400 + 86399); + const seconds = Math.floor(value); + + if (seconds < 0) { + return { err: new Error(`A relative period cannot be negative, got ${String(value)}`) }; + } + + if (seconds > maxRelativeSeconds) { + return { err: new Error(`A period in seconds is spelled in days and below, so ${String(maxRelativeSeconds)} (99d 23:59:59) is the most, got ${String(value)}; pass a Date for an instant further out`) }; + } 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) + + pad(Math.floor(seconds / 86400), 2) + + pad(Math.floor(seconds / 3600) % 24, 2) + + pad(Math.floor(seconds / 60) % 60, 2) + + pad(seconds % 60, 2) + '000R', }; } diff --git a/test/message.test.ts b/test/message.test.ts index 78aaf7e..37392a4 100644 --- a/test/message.test.ts +++ b/test/message.test.ts @@ -237,10 +237,47 @@ describe('smppTime', () => { assert.equal(smppTime.encode(new Date(Date.UTC(2026, 7, 25, 14, 30, 0))).text, '260825143000000+'); }); - test('encodes a relative time given in seconds, clamped to what the format holds', () => { + test('encodes a relative time given in seconds', () => { + assert.equal(smppTime.encode(0).text, '000000000000000R'); assert.equal(smppTime.encode(3600).text, '000000010000000R'); assert.equal(smppTime.encode(99 * 86400).text, '000099000000000R'); - assert.equal(smppTime.encode(100 * 86400).text, '000099235959000R'); + assert.equal(smppTime.encode(99 * 86400 + 86399).text, '000099235959000R'); + }); + + test('refuses a second count past the day field rather than clamping it, naming the Date spelling', () => { + for (const seconds of [99 * 86400 + 86400, 86400 * 365]) { + const encoded = smppTime.encode(seconds); + + assert.ok(encoded.err instanceof Error, String(seconds)); + assert.equal(encoded.text, undefined); + assert.match(encoded.err.message, /pass a Date/); + } + }); + + test('refuses a negative period without sending the caller to the Date that cannot help', () => { + const encoded = smppTime.encode(-1); + + assert.ok(encoded.err instanceof Error); + assert.equal(encoded.text, undefined); + assert.match(encoded.err.message, /cannot be negative/); + assert.doesNotMatch(encoded.err.message, /Date/); + }); + + test('decodes the years and months the relative format holds but a second count cannot name', () => { + const before = Date.now(); + const year = smppTime.decode('010000000000000R'); + const months = smppTime.decode('001000000000000R'); + const day = 86400_000; + + assert.equal(year.err, undefined); + assert.ok(year.date, 'a year is a period the format holds'); + assert.ok(year.date.getTime() >= before + 365 * day, year.date.toISOString()); + assert.ok(year.date.getTime() <= Date.now() + 366 * day, year.date.toISOString()); + + assert.equal(months.err, undefined); + assert.ok(months.date, 'ten months is a period the format holds'); + assert.ok(months.date.getTime() >= before + 290 * day, months.date.toISOString()); + assert.ok(months.date.getTime() <= Date.now() + 320 * day, months.date.toISOString()); }); test('passes an already-formatted string through', () => { diff --git a/test/unsendable.test.ts b/test/unsendable.test.ts index 4a5541c..1177b3a 100644 --- a/test/unsendable.test.ts +++ b/test/unsendable.test.ts @@ -382,6 +382,17 @@ describe('a time no peer can read', () => { assert.equal(attempts.length, 0); }); + test('refuses a year-long relative period rather than sending 99 days and reporting a year', async () => { + const attempts: PduObjectInput[] = []; + const sent = await submitSms(recordingDeps(attempts), { from, message: 'Hello world', to, validityPeriod: 86400 * 365 }); + + assert.ok(sent.err instanceof Error); + assert.match(sent.err.message, /validityPeriod/); + assert.match(sent.err.message, /Date/); + assert.deepEqual(sent.smsIds, []); + assert.equal(attempts.length, 0, 'a period the relative format cannot hold puts nothing on the wire'); + }); + 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); diff --git a/todo.md b/todo.md index e639383..5b865ca 100644 --- a/todo.md +++ b/todo.md @@ -120,6 +120,19 @@ 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. +- [ ] **A default GSM send declares IA5, not GSM 03.38.** `dataCodingFor()` resolves `encoding` + through `consts.ENCODING`, so `'ASCII'` goes out as `data_coding` 0x01 — SMPP 3.4 5.2.19's + *IA5 (CCITT T.50)/ASCII* — while the codec writes the GSM 03.38 table, where `@` is 0x00 and + `$` is 0x02 against IA5's NUL and STX. `encodingByDataCoding()` reads 0x00 back as GSM 03.38, + so this library's writer and reader disagree, and every peer in `interop-tests/` submits GSM + text at `data_coding` 0. Nothing asserts what our own default send declares, and the echo + tests use characters where the two tables agree, which is why the suite is green. + `test/message-class.test.ts` pins the current value. Goal 1 owns it. The fix is a three-entry + map in `send-sms.ts` (`ASCII` → 0x00, `LATIN1` → 0x03, `UCS2` → 0x08) rather than any move of + a published constant — `consts.ENCODING.ASCII` = 0x01 is a correct name for SMPP's flat-table + IA5 entry, and the defect is using that group as the option's vocabulary. **Which value an + operator should see is the maintainer's call**, and it wants an `interop-tests/` run. Raised + by the architecture review of [#99](https://github.com/larvit/larvitsmpp/pull/99), 2026-09-09. - [ ] Tag `v1.0.0` to publish. - [ ] `npm deprecate larvitsmpp` pointing at `@larvit/smpp`. Maintainer's call to run it; not something CI should do.