diff --git a/AGENTS.md b/AGENTS.md index 7a92543..d9b42e6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,9 @@ These are not preferences. Breaking one is a defect. 1. **Nothing throws.** Every fallible function returns (or resolves to) a DTO carrying an optional `err`. No `throw`, no rejected promises, no exceptions as control flow. Node APIs that throw are wrapped at the boundary and converted into a result. Programmer errors (bad arguments) are - results too. + results too, wherever the types admit one: a function whose argument types are a closed set is + guarded by the compiler and stays total, which is why the encoding helpers return plainly, and + the check belongs at whichever boundary the argument arrives untyped at. 2. **Log messages are static strings.** Every dynamic value goes into the log metadata. Never interpolate, never concatenate. - GOOD: `log.debug('sendSms() - splitting message', { parts: msgs.length, to });` @@ -182,12 +184,12 @@ so a peer that dispatches one request at a time is never left waiting on us. Over SMPP the ESME puts one GSM character per octet in `short_message` and the SMSC packs it into septets. The 140-octet limit applies to that packed result, not to what goes on the wire here, which -is why a concatenated segment is 153 characters plus a 6-octet UDH — 159 octets in `short_message`, -and entirely correct. Do not "fix" this to 134; that number is the packed payload size and would -truncate every long message by a fifth. +is why a concatenated GSM segment is 153 characters plus a 6-octet UDH — 159 octets in +`short_message`, and entirely correct. Do not "fix" that to 134; that number is the packed payload +size and would truncate every long GSM message by a fifth. -UCS2 is not packed, so there the two coincide: 67 characters = 134 octets, plus the 6-octet UDH is -exactly 140. +GSM 7-bit is the only alphabet it applies to. What each of the three is budgeted, and why, is a +decision under [The wire](#the-wire). ## Conventions @@ -301,6 +303,21 @@ Grouped by what each one constrains. a cast and types nothing it carries. Accepted: a second copy of the package installed alongside this one defeats `instanceof`, where `err.name` still reads `PduRefusedError`. +- **`bitCount()`, `encodeMessage()` and `splitMessage()` keep their total signatures, because + `EncodingName` is what keeps an alphabet with no codec away from them.** Maintainer's call, + 2026-09-09, from the architecture review of [#95](https://github.com/larvit/larvitsmpp/pull/95): + all three index `encodings` by name and would throw on one it has no codec for, which hard rule 1 + forbids. That PR left no such name to pass — `encodings` is a `Record`, so + every member of the union has a codec and one added without a codec, or without a segment budget, + fails to compile in four places. What was missing is the door for a caller holding a name at + runtime: `Object.hasOwn(encodings, x)` is the only test the published surface offered and it + narrows nothing, so `isEncodingName()` is exported beside `isCommandName()` and `isErrorName()`, + which serve their own tables that way. Rejected: a `Result` signature on all three, which costs + 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. + ### The wire - **The declared interface version is an option on both `client()` and `server()`, and is not the @@ -557,6 +574,18 @@ Grouped by what each one constrains. the base instead would break that pair. The option is on `client()` only, since a `server()` session writes both ids itself. +- **A concatenated segment is budgeted at 134 octets, which is 153 septets where the SMSC packs them + and 134 octets of anything it does not.** Maintainer's call, 2026-09-09, from the architecture + review of [#95](https://github.com/larvit/larvitsmpp/pull/95): `segmentUnits` handed 153 to + everything but UCS2, so a long `encoding: 'LATIN1'` message went out as segments of 153 octets plus + a 6-octet UDH — 159 on the air where GSM 03.40 carries 140, which no SMSC can deliver. Goal 1 owns + it. There is one budget, 140 less the UDH, and the alphabet decides only what it is counted in, so + Latin-1 and UCS2 both take those 134 octets — 134 characters and 67 — and it is GSM 7-bit's 153 + that is the odd number rather than the other way round. `Record` is what makes + a fourth alphabet state its own. Rejected: 134 for GSM 7-bit too, which is the mistake the + 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. + ### 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 407ea03..08d3626 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,11 @@ It travels in `data_coding` beside the alphabet, so a flash UCS2 message stays U 8-bit data or UCS2, and 8-bit data is not text a handset will display — so the send is refused before anything goes out. -Messages too long for one SMS are split automatically and sent as a concatenated message. You get -one id per segment: +Messages too long for one SMS are split automatically and sent as a concatenated message. One SMS +holds 160 characters as `ASCII`, 140 as `LATIN1` or 70 as `UCS2`, and the concatenation header a +longer message needs takes room off each segment: 153, 134 and 67. GSM's extension characters +(`{}[]\~^|€` and form feed) count as two, as does a character outside the basic multilingual plane +in `UCS2`. You get one id per segment: ```javascript const { err, pduObjs, smsIds, unanswered } = await session.sendSms({ from, message, to }); diff --git a/src/index.ts b/src/index.ts index 0d3701a..44a2c9b 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, messageClassOf } from './defs/encodings.ts'; +export { detect, encodingByDataCoding, encodings, isEncodingName, messageClassOf } 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'; diff --git a/src/message.ts b/src/message.ts index b825bfa..7b02be7 100644 --- a/src/message.ts +++ b/src/message.ts @@ -10,8 +10,8 @@ const singleMessageBits = 1120; /** The concatenation UDH numbers the segments of a message in a single octet. */ export const maxSegments = 255; -/** Budget per concatenated segment: septets for GSM, octets for UCS2. */ -const segmentUnits = { ASCII: 153, UCS2: 67 * 2 } as const; +/** Budget per segment: the 134 octets left of 140 after the UDH, or the 153 septets GSM packs into them. */ +const segmentUnits: Record = { ASCII: 153, LATIN1: 134, UCS2: 134 }; export type SplitOptions = { encoding?: EncodingName; @@ -69,7 +69,7 @@ export function splitMessage(message: string, options: SplitOptions): Buffer[] { return [encodings[encoding].encode(message)]; } - const budget = encoding === 'UCS2' ? segmentUnits.UCS2 : segmentUnits.ASCII; + const budget = segmentUnits[encoding]; const parts: string[] = []; let current = ''; let used = 0; diff --git a/test/message.test.ts b/test/message.test.ts index 886f53e..68bc0fa 100644 --- a/test/message.test.ts +++ b/test/message.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import test, { describe } from 'node:test'; +import type { EncodingName } from '../src/defs/encodings.ts'; import { bitCount, decodeMessage, @@ -9,7 +10,9 @@ import { splitMessage, } from '../src/message.ts'; // Through the public surface: an application handed a PduObject needs this same answer. -import { messageOctets, objToPdu, pduToObj } from '../src/index.ts'; +import { encodings, isEncodingName, messageOctets, objToPdu, pduToObj } from '../src/index.ts'; + +const singleSmsOctets = 140; describe('bitCount()', () => { test('counts GSM characters as seven bits each', () => { @@ -68,6 +71,33 @@ describe('splitMessage()', () => { } }); + test('fits a concatenated Latin-1 segment into the 140 octets an SMS carries', () => { + const message = 'å'.repeat(300); + const segments = splitMessage(message, { encoding: 'LATIN1', reference: 0x4B }); + + assert.equal(segments.length, 3); + + for (const segment of segments) { + assert.ok(segment.length <= singleSmsOctets, `segment of ${String(segment.length)} octets`); + } + + assert.equal(segments[0]?.length, 6 + 134); + assert.equal(segments.map(segment => segment.subarray(6).toString('latin1')).join(''), message); + }); + + test('fits a concatenated 8-bit binary segment into the same 140 octets', () => { + const payload = Buffer.from(Array.from({ length: 400 }, (_, at) => (at * 7 + 3) % 256)); + const segments = splitMessage(payload.toString('latin1'), { encoding: 'LATIN1', reference: 0x4C }); + + assert.equal(segments.length, 3); + + for (const segment of segments) { + assert.ok(segment.length <= singleSmsOctets, `segment of ${String(segment.length)} octets`); + } + + assert.deepEqual(Buffer.concat(segments.map(segment => segment.subarray(6))), payload); + }); + test('prefixes each segment with a concatenation UDH', () => { const segments = splitMessage('a'.repeat(306), { reference: 0x2A }); @@ -78,6 +108,8 @@ describe('splitMessage()', () => { test('produces no segments at all for a message no UDH can number', () => { assert.equal(splitMessage('a'.repeat(153 * 255), { reference: 1 }).length, 255); assert.equal(splitMessage('a'.repeat(153 * 255 + 1), { reference: 1 }).length, 0); + assert.equal(splitMessage('å'.repeat(134 * 255), { encoding: 'LATIN1', reference: 1 }).length, 255); + assert.equal(splitMessage('å'.repeat(134 * 255 + 1), { encoding: 'LATIN1', reference: 1 }).length, 0); }); test('splits on characters, never inside an escape sequence', () => { @@ -132,6 +164,30 @@ describe('encodeMessage() and decodeMessage()', () => { }); }); +describe('the alphabet the encoding helpers are asked for', () => { + const everyName: EncodingName[] = ['ASCII', 'LATIN1', 'UCS2']; + + test('is one of three, each with a codec, so none of the three helpers can reach an absent one', () => { + assert.deepEqual(Object.keys(encodings).sort(), [...everyName].sort()); + + for (const encoding of everyName) { + assert.equal(encodeMessage('Hello', encoding).encoding, encoding); + assert.ok(bitCount('Hello', encoding) > 0); + assert.equal(splitMessage('Hello', { encoding, reference: 1 }).length, 1); + } + }); + + test('is narrowed by isEncodingName(), the door a caller holding a name at runtime takes', () => { + for (const encoding of everyName) { + assert.equal(isEncodingName(encoding), true, encoding); + } + + for (const value of ['FLASH', 'BINARY', 'utf8', 'ascii', 'toString', '', 8, {}, null, undefined]) { + assert.equal(isEncodingName(value), false, JSON.stringify(value)); + } + }); +}); + describe('messageOctets()', () => { /** A data_sm has no short_message field at all, which is the case that has neither. */ function parsed(short: Buffer | undefined, payload?: Buffer) { diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 6a6079c..06ebfa9 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -428,6 +428,17 @@ describe('sendSms()', () => { assert.equal(sent.err, undefined); assert.equal(attempts.length, 4); + + const latin1 = 'å'.repeat(153 * 3); + const overLatin1 = await submitSms(deps, { encoding: 'LATIN1', from: '46701113311', maxSegments: 3, message: latin1, to: '46709771337' }); + + assert.ok(overLatin1.err instanceof Error); + assert.equal(attempts.length, 4); + + const sentLatin1 = await submitSms(deps, { encoding: 'LATIN1', from: '46701113311', maxSegments: 4, message: latin1, to: '46709771337' }); + + assert.equal(sentLatin1.err, undefined); + assert.equal(attempts.length, 8); }); }); diff --git a/todo.md b/todo.md index 301c0f5..90126a2 100644 --- a/todo.md +++ b/todo.md @@ -120,23 +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. -- [ ] **`bitCount()`, `encodeMessage()` and `splitMessage()` throw on an encoding name they have no - codec for**, where `sendSms()` now refuses one by name. All three are value-exported, so this - is the published surface, one door over from the one [#95](https://github.com/larvit/larvitsmpp/pull/95) - closed — and that PR sharpened the edge, since `encodeMessage(msg, 'FLASH')` used to encode - quietly and now throws out of the codec table. `decodeMessage()` is unaffected; it resolves - through `encodingByDataCoding()`. Tag-relevant because of the fix shape: guarding them means - either a `Result` signature on three published functions, which needs a major version after - the tag, or a documented deviation saying a typed caller cannot get here. Maintainer's call. - Raised by the architecture review of [#95](https://github.com/larvit/larvitsmpp/pull/95), - 2026-09-09. -- [ ] **A concatenated Latin-1 segment is 159 octets, and an SMS carries 140.** `segmentUnits` in - `message.ts` budgets 153 for everything that is not UCS2, which is right for GSM 7-bit alone — - the SMSC packs 153 septets into 134 octets. Latin-1 is 8-bit and never packed, so a long - `encoding: 'LATIN1'` message goes out with segments no SMSC can carry. Same family as the - 0.4.0 "Short segments" row, in the other direction; goal 1 owns it, so it wants fixing before - the tag. One line — 134 octets for both unpacked alphabets — plus a regression test. Raised by - the architecture review of [#95](https://github.com/larvit/larvitsmpp/pull/95), 2026-09-09. - [ ] **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 @@ -158,6 +141,25 @@ 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. + - [ ] **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` pinning thread, which stays open until this lands. Pinning every action and runner by hand is