Refuse a non-finite number where a text field coerces one #18

Merged
lilleman merged 9 commits from worktree-non-finite-text into main 2026-09-21 10:46:52 +02:00
5 changed files with 20 additions and 7 deletions
Showing only changes of commit e7e71ca9bd - Show all commits
+4
View File
@@ -25,6 +25,10 @@
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. An Octet String carries a NULL as before.
- A number with no decimal spelling — `NaN`, `Infinity`, `-Infinity` — is refused where a text field
takes one. `sendSms({ from: NaN })` put the literal sender `NaN` on the wire and reported the send
as successful; `message_id`, `source_addr` and every other text field took such a number the same
way. A finite number still writes its decimals, so `message_id: 123` is unchanged.
## 0.5.0
+4
View File
@@ -99,6 +99,10 @@ function wantText(value: ParamValue): Result<{ text: string }> {
return { err: new Error(`Expected a string or a number, got ${typeof value}`) };
}
if (typeof value === 'number' && !Number.isFinite(value)) {
return { err: new Error(`Expected a finite number, got ${String(value)}`) };
}
const text = String(value);
return pastLatin1(text) ?? { text };
+4
View File
@@ -153,6 +153,10 @@ describe('parsing real PDUs', () => {
assert.ok(smuggled.err instanceof Error);
assert.equal(smuggled.buffer, undefined);
assert.ok(objToPdu({ cmdName: 'deliver_sm', params: { source_addr: '一' } }).err instanceof Error);
// String(NaN) is a sender the peer reads as the three letters, and the send reports success.
assert.ok(objToPdu({ cmdName: 'deliver_sm', params: { source_addr: NaN } }).err instanceof Error);
assert.ok(objToPdu({ cmdName: 'deliver_sm', params: { source_addr: Infinity } }).err instanceof Error);
});
});
+8 -1
View File
@@ -112,13 +112,20 @@ describe('cstring (C-Octet String)', () => {
assert.deepEqual(target, encoded);
});
test('coerces a numeric value to its decimal string', () => {
test('coerces a numeric value to its decimal string, and refuses one with no decimals', () => {
const target = Buffer.alloc(4);
types.cstring.write(123, target, 0);
assert.deepEqual(target, Buffer.from([0x31, 0x32, 0x33, 0x00]));
assert.deepEqual(types.cstring.size(123), { size: 4 });
for (const value of [NaN, Infinity, -Infinity]) {
assert.ok(types.cstring.size(value).err instanceof Error, String(value));
assert.ok(types.cstring.write(value, Buffer.alloc(9), 0).err instanceof Error, String(value));
assert.ok(types.string.write(value, Buffer.alloc(9), 0).err instanceof Error, String(value));
assert.ok(types.tlv.string.write(value, Buffer.alloc(9), 0).err instanceof Error, String(value));
}
});
test('carries every latin1 octet, and refuses a character past it', () => {
-6
View File
@@ -189,12 +189,6 @@ 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