diff --git a/AGENTS.md b/AGENTS.md index 42b42d0..ded3adb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -292,6 +292,7 @@ the file. - A string body is written in the alphabet its own `data_coding` names, and one that alphabet cannot carry is refused by the codec — `message_payload` on the same terms as `short_message`. - A GSM 03.38 message declares `data_coding` 0x00, and an inbound 0x01 is still read as GSM. +- Every text field on the wire is latin1, and a character past `U+00FF` is refused. ### [The session's life](docs/decisions.md#the-sessions-life) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dc91e2..60d1055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ one that expires as an ordinary connect failure, so `reconnect` retries it on its usual backoff. A connect previously waited the operating system out, around 130 s on Linux against a host that drops SYNs. `connectTimeout` retunes the bound, and `connectTimeout: false` restores the old wait. +- Addresses, ids and every other text field on the wire are read and written as latin1. A + `source_addr` of `Kaffeé` previously reached the application as `Kaffei`, because the codec wrote + the octet and then masked bit 7 reading it back; `destination_addr`, `system_id`, `message_id`, + `service_type` and the C-Octet String TLVs were affected the same way. A character past `U+00FF` + in one of those fields is now refused, where it used to go out as its low octet. ## 0.5.0 diff --git a/README.md b/README.md index b78ac6c..8c95f4b 100644 --- a/README.md +++ b/README.md @@ -288,7 +288,8 @@ 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. +and 1 for a numeric one; the NPI fields default to 0. An address is latin1, so `Kaffeé` goes out and +comes back as its own octets; a character past `U+00FF` is refused rather than sent truncated. **Encoding.** diff --git a/docs/decisions.md b/docs/decisions.md index 376e871..16066c4 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -443,8 +443,8 @@ rule and an index of the titles below. the guard would be a second spelling that disagrees about correctness. Rejected: refusing a string `message_payload` outright and demanding octets, which contradicts `short_message` on the same PDU. Rejected: guarding every string-valued - field, which `data_coding` says nothing about — an address is a C-Octet String and ASCII by 3.4's - own definition. + field against `data_coding`, which says nothing about them — a text field on the wire has an + alphabet of its own. - **A GSM 03.38 message declares `data_coding` 0x00, and an inbound 0x01 is still read as GSM.** Maintainer's call, 2026-09-09: `dataCodingFor()` and `encodeBody()` both resolved an alphabet @@ -478,6 +478,24 @@ rule and an index of the titles below. concatenation reference rather than on `data_coding` — so a receipt or a segment that crossed the change reads exactly as it did. +- **Every text field on the wire is latin1, and a character past `U+00FF` is refused.** Maintainer's + call, 2026-09-21, from the 0.6.0 correctness list: the codec read these fields with + `toString('ascii')`, which masks bit 7, and wrote them with `write(text, 'ascii')`, which does not, + so an inbound `source_addr` of `Kaffeé` reached the application as `Kaffei` and + `objToPdu(pduToObj(x))` was idempotent for none of `source_addr`, `destination_addr`, `system_id`, + `message_id`, `service_type` or the cstring TLVs. Goal 3 settles the read: 3.4 calls these fields + ASCII, and an operator routing an alphanumeric sender through the upper half is traffic to keep. + The write is named latin1 to match, which is what makes the round trip idempotent and is already + the octets Node put on the wire, so no peer sees a change. `wantText()` is the single place that + says so — which is why the `dest_address` and `unsuccess_sme` structures write their embedded + addresses through `cstring.write()` rather than reaching past it — and a character past `U+00FF` + is refused there rather than truncated to its low octet: a `size()` that agreed with a `write()` + that dropped half a character is a wrong answer about what went out (goal 2), and it is how an + alphabet that cannot carry a message body is already handled. 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 drop exactly the traffic the read + keeps. + ## The session's life - **A close arriving after our own `unbind` is a clean unbind, not an error.** Maintainer's call, diff --git a/src/defs/types.ts b/src/defs/types.ts index 9f8a323..e7589d4 100644 --- a/src/defs/types.ts +++ b/src/defs/types.ts @@ -28,7 +28,7 @@ export type WireType = { export function paramText(value: ParamValue | undefined): string { if (typeof value === 'string') return value; if (typeof value === 'number') return value.toString(); - if (Buffer.isBuffer(value)) return value.toString('ascii'); + if (Buffer.isBuffer(value)) return value.toString('latin1'); return ''; } @@ -79,11 +79,25 @@ function writeInt32(value: ParamValue, buf: Buffer, offset: number): VoidResult return {}; } -function wantText(value: ParamValue): Result<{ text: string }> { - if (typeof value === 'string') return { text: value }; - if (typeof value === 'number') return { text: value.toString() }; +function pastLatin1(text: string): { err: Error } | undefined { + const index = text.search(/[\u0100-\uFFFF]/); - return { err: new Error(`Expected a string, got ${typeof value}`) }; + if (index === -1) return undefined; + + const code = text.charCodeAt(index).toString(16).toUpperCase().padStart(4, '0'); + + return { + err: new Error( + `Character U+${code} at index ${String(index)} is past latin1, which every text field on the wire is written in`, + ), + }; +} + +function wantText(value: ParamValue): Result<{ text: string }> { + if (typeof value === 'number') return { text: value.toString() }; + if (typeof value !== 'string') return { err: new Error(`Expected a string, got ${typeof value}`) }; + + return pastLatin1(value) ?? { text: value }; } function wantBytes(value: ParamValue): Result<{ bytes: Buffer }> { @@ -91,7 +105,7 @@ function wantBytes(value: ParamValue): Result<{ bytes: Buffer }> { const { err, text } = wantText(value); - return err ? { err } : { bytes: Buffer.from(text, 'ascii') }; + return err ? { err } : { bytes: Buffer.from(text, 'latin1') }; } function isDestAddress(value: unknown): value is DestAddress { @@ -167,7 +181,7 @@ function readCstring(buffer: Buffer, offset: number): Result<{ bytesRead: number } } - return { bytesRead: length + 1, value: buffer.toString('ascii', offset, offset + length) }; + return { bytesRead: length + 1, value: buffer.toString('latin1', offset, offset + length) }; } function writeCstring(text: string, buffer: Buffer, offset: number): VoidResult { @@ -175,7 +189,7 @@ function writeCstring(text: string, buffer: Buffer, offset: number): VoidResult if (err) return { err }; - buffer.write(text, offset, 'ascii'); + buffer.write(text, offset, 'latin1'); buffer[offset + text.length] = 0; return {}; @@ -248,7 +262,7 @@ export const string: WireType = { if (err) return { err }; - return { bytesRead: length + 1, value: buffer.toString('ascii', offset + 1, offset + 1 + length) }; + return { bytesRead: length + 1, value: buffer.toString('latin1', offset + 1, offset + 1 + length) }; }, size(value) { const { err, text } = wantText(value); @@ -271,7 +285,7 @@ export const string: WireType = { if (rangeErr) return { err: rangeErr }; buffer.writeUInt8(text.length, offset); - buffer.write(text, offset + 1, 'ascii'); + buffer.write(text, offset + 1, 'latin1'); return {}; }, @@ -401,7 +415,7 @@ export const dest_address_array: WireType = { if ('dl_name' in dest) { buf.writeUInt8(2, offset++); - const name = writeCstring(dest.dl_name, buf, offset); + const name = cstring.write(dest.dl_name, buf, offset); if (name.err) return { err: name.err }; @@ -417,7 +431,7 @@ export const dest_address_array: WireType = { if (npi.err) return { err: npi.err }; - const addr = writeCstring(dest.destination_addr, buf, offset); + const addr = cstring.write(dest.destination_addr, buf, offset); if (addr.err) return { err: addr.err }; @@ -505,7 +519,7 @@ export const unsuccess_sme_array: WireType = { if (npi.err) return { err: npi.err }; - const addr = writeCstring(sme.destination_addr, buf, offset); + const addr = cstring.write(sme.destination_addr, buf, offset); if (addr.err) return { err: addr.err }; @@ -538,7 +552,7 @@ export const tlv = { ? offset + length : terminator; - return { bytesRead: length, value: buf.toString('ascii', offset, end) }; + return { bytesRead: length, value: buf.toString('latin1', offset, end) }; }, size(value: ParamValue) { const { err, text } = wantText(value); @@ -559,7 +573,7 @@ export const tlv = { read(buf: Buffer, offset: number, length = 0) { const err = outOfRange(buf, offset, length); - return err ? { err } : { bytesRead: length, value: buf.toString('ascii', offset, offset + length) }; + return err ? { err } : { bytesRead: length, value: buf.toString('latin1', offset, offset + length) }; }, size(value: ParamValue) { const { err, text } = wantText(value); @@ -575,7 +589,7 @@ export const tlv = { if (rangeErr) return { err: rangeErr }; - buf.write(text, offset, 'ascii'); + buf.write(text, offset, 'latin1'); return {}; }, diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 999337b..8d8a296 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -127,6 +127,24 @@ describe('parsing real PDUs', () => { assert.equal(pduObj.params.short_message, 'hej 一'); assert.equal(pduObj.tlvs.message_state?.tagValue, 2); }); + + // 'ascii' masks bit 7 on the way in, so this address used to come back Kaffei. + test('keeps an alphanumeric sender whole through the wire and back', () => { + const pdu = encode({ + cmdName: 'deliver_sm', + params: { + destination_addr: '46709771337', + short_message: 'hej', + source_addr: 'Kaffeé', + source_addr_ton: 5, + }, + seqNr: 9, + }); + + assert.ok(pdu.includes(Buffer.from('Kaffeé', 'latin1'))); + assert.equal(decode(pdu).params.source_addr, 'Kaffeé'); + assert.ok(objToPdu({ cmdName: 'deliver_sm', params: { source_addr: '一' } }).err instanceof Error); + }); }); describe('encoding submit_sm', () => { diff --git a/test/types.test.ts b/test/types.test.ts index 4d8f0a4..1da0051 100644 --- a/test/types.test.ts +++ b/test/types.test.ts @@ -1,8 +1,8 @@ import assert from 'node:assert/strict'; import test, { describe } from 'node:test'; import type { DestAddress, UnsuccessSme } from '../src/defs/types.ts'; +import { paramText, types } from '../src/defs/types.ts'; import { tlvs } from '../src/defs/tlvs.ts'; -import { types } from '../src/defs/types.ts'; describe('integers', () => { test('int8 reads, sizes and writes one octet', () => { @@ -69,6 +69,20 @@ describe('string (Octet String)', () => { assert.deepEqual(target, encoded); }); + + test('carries every latin1 octet, and refuses a character past it', () => { + const target = Buffer.alloc(4); + + assert.deepEqual(types.string.read(Buffer.from([3, 0xE9, 0x80, 0xFF]), 0), { + bytesRead: 4, + value: 'é\u0080ÿ', + }); + assert.deepEqual(types.string.write('é\u0080ÿ', target, 0), {}); + assert.deepEqual(target, Buffer.from([3, 0xE9, 0x80, 0xFF])); + + assert.ok(types.string.size('一').err instanceof Error); + assert.ok(types.string.write('一', Buffer.alloc(4), 0).err instanceof Error); + }); }); describe('cstring (C-Octet String)', () => { @@ -100,6 +114,20 @@ describe('cstring (C-Octet String)', () => { assert.deepEqual(types.cstring.size(123), { size: 4 }); }); + // 'ascii' masks bit 7 on the way in and keeps it on the way out, so an address a peer wrote as + // Kaffeé came back Kaffei and objToPdu(pduToObj(x)) stopped being idempotent. + test('carries every latin1 octet, and refuses a character past it', () => { + const address = Buffer.from([0x4B, 0x61, 0x66, 0x66, 0x65, 0xE9, 0x00]); + const target = Buffer.alloc(7); + + assert.deepEqual(types.cstring.read(address, 0), { bytesRead: 7, value: 'Kaffeé' }); + assert.deepEqual(types.cstring.write('Kaffeé', target, 0), {}); + assert.deepEqual(target, address); + + assert.ok(types.cstring.size('一').err instanceof Error); + assert.ok(types.cstring.write('一', Buffer.alloc(4), 0).err instanceof Error); + }); + test('refuses a string with no terminator rather than running off the end', () => { assert.ok(types.cstring.read(Buffer.from('abcd'), 0).err instanceof Error); }); @@ -142,6 +170,32 @@ describe('integer TLVs', () => { }); }); +describe('text TLVs', () => { + const encoded = Buffer.from([0xE9, 0x80, 0xFF]); + + test('carry every latin1 octet, and refuse a character past it', () => { + const target = Buffer.alloc(3); + + assert.deepEqual(types.tlv.string.read(encoded, 0, 3), { bytesRead: 3, value: 'é\u0080ÿ' }); + assert.deepEqual(types.tlv.string.write('é\u0080ÿ', target, 0), {}); + assert.deepEqual(target, encoded); + + assert.ok(types.tlv.string.size('一').err instanceof Error); + assert.ok(types.tlv.string.write('一', Buffer.alloc(3), 0).err instanceof Error); + }); + + test('carry them through a cstring tag too, terminator or none', () => { + const target = Buffer.alloc(4); + + assert.deepEqual(types.tlv.cstring.read(encoded, 0, 3), { bytesRead: 3, value: 'é\u0080ÿ' }); + assert.deepEqual(types.tlv.cstring.write('é\u0080ÿ', target, 0), {}); + assert.deepEqual(target, Buffer.from([0xE9, 0x80, 0xFF, 0x00])); + + assert.ok(types.tlv.cstring.size('一').err instanceof Error); + assert.ok(types.tlv.cstring.write('一', Buffer.alloc(4), 0).err instanceof Error); + }); +}); + describe('buffer', () => { const expected = Buffer.from('abcd1234'); @@ -170,6 +224,22 @@ describe('buffer', () => { assert.deepEqual(target, expected); }); + + test('takes a string as the latin1 octets it stands for', () => { + const target = Buffer.alloc(3); + + assert.deepEqual(types.buffer.size('é\u0080ÿ'), { size: 3 }); + assert.deepEqual(types.buffer.write('é\u0080ÿ', target, 0), {}); + assert.deepEqual(target, Buffer.from([0xE9, 0x80, 0xFF])); + }); +}); + +describe('paramText()', () => { + // The one reader of a receipt body that arrived with no octets of its own, so masking bit 7 + // here loses the same characters the wire types used to. + test('renders a Buffer parameter as the latin1 text its octets spell', () => { + assert.equal(paramText(Buffer.from([0x4B, 0x61, 0x66, 0x66, 0x65, 0xE9])), 'Kaffeé'); + }); }); describe('dest_address_array', () => { diff --git a/todo.md b/todo.md index e34ff08..473e856 100644 --- a/todo.md +++ b/todo.md @@ -189,15 +189,6 @@ and is also what the panel ranked hardest — two methods, one answer. ### Correctness, ahead of everything below -- [ ] **Read C-Octet Strings as `latin1`, so an address survives the wire.** `defs/types.ts` reads - with `toString('ascii')` at four sites and writes with `write(text, 'ascii')` at three. Node - masks bit 7 when decoding and not when encoding, so the codec writes `0xE9` and reads back - `0x69`: an inbound `source_addr` of `Kaffeé` reaches the application as `Kaffei`, with no raw - escape hatch as `short_message` has in `shortMessageOctets`. Affects `source_addr`, - `destination_addr`, `system_id`, `message_id`, `service_type` and the cstring TLVs, and makes - `objToPdu(pduToObj(x))` non-idempotent for them. Goals 1 and 3. Verified in the container: - `Buffer.from([0xE9]).toString('ascii')` is `'i'`. - - [ ] **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