Name the refusal rule, and refuse a period the encoder cannot spell (#99)
* Refuse a relative validity period the SMPP time format cannot hold * Export unencodableText(), so a caller can word a refusal as the library does * Name the refusal rule in the README, and correct what it claims * Word the period refusal by what the encoder spells, and keep unencodableText() internal * Correct the decision bullet premise about what the relative format holds * Close the stability nits on the time guard and its tests
This commit is contained in:
GitHub
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
+18
-8
@@ -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',
|
||||
};
|
||||
}
|
||||
|
||||
+39
-2
@@ -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', () => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user