Read and write every text field on the wire as latin1 #16

Merged
lilleman merged 6 commits from cstring-latin1 into main 2026-09-21 08:28:44 +02:00
8 changed files with 147 additions and 29 deletions
Showing only changes of commit 7db242e375 - Show all commits
+1
View File
@@ -292,6 +292,7 @@ the file.
- A string body is written in the alphabet its own `data_coding` names, and one that alphabet cannot - 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`. 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. - 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) ### [The session's life](docs/decisions.md#the-sessions-life)
+5
View File
@@ -6,6 +6,11 @@
one that expires as an ordinary connect failure, so `reconnect` retries it on its usual backoff. 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 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. 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 ## 0.5.0
+2 -1
View File
@@ -288,7 +288,8 @@ await session.sendSms({
``` ```
**Addresses.** `sourceAddrTon` and `destinationAddrTon` default to 5 for an alphanumeric address **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.** **Encoding.**
+20 -2
View File
@@ -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 the guard would be a second spelling that disagrees about correctness. Rejected: refusing a string
`message_payload` outright and demanding `message_payload` outright and demanding
octets, which contradicts `short_message` on the same PDU. Rejected: guarding every string-valued 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 field against `data_coding`, which says nothing about them — a text field on the wire has an
own definition. alphabet of its own.
- **A GSM 03.38 message declares `data_coding` 0x00, and an inbound 0x01 is still read as GSM.** - **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 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 concatenation reference rather than on `data_coding` — so a receipt or a segment that crossed the
change reads exactly as it did. 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 ## The session's life
- **A close arriving after our own `unbind` is a clean unbind, not an error.** Maintainer's call, - **A close arriving after our own `unbind` is a clean unbind, not an error.** Maintainer's call,
+30 -16
View File
@@ -28,7 +28,7 @@ export type WireType<T extends ParamValue = ParamValue> = {
export function paramText(value: ParamValue | undefined): string { export function paramText(value: ParamValue | undefined): string {
if (typeof value === 'string') return value; if (typeof value === 'string') return value;
if (typeof value === 'number') return value.toString(); if (typeof value === 'number') return value.toString();
if (Buffer.isBuffer(value)) return value.toString('ascii'); if (Buffer.isBuffer(value)) return value.toString('latin1');
return ''; return '';
} }
@@ -79,11 +79,25 @@ function writeInt32(value: ParamValue, buf: Buffer, offset: number): VoidResult
return {}; return {};
} }
function wantText(value: ParamValue): Result<{ text: string }> { function pastLatin1(text: string): { err: Error } | undefined {
if (typeof value === 'string') return { text: value }; const index = text.search(/[\u0100-\uFFFF]/);
if (typeof value === 'number') return { text: value.toString() };
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 }> { function wantBytes(value: ParamValue): Result<{ bytes: Buffer }> {
@@ -91,7 +105,7 @@ function wantBytes(value: ParamValue): Result<{ bytes: Buffer }> {
const { err, text } = wantText(value); 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 { 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 { 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 }; if (err) return { err };
buffer.write(text, offset, 'ascii'); buffer.write(text, offset, 'latin1');
buffer[offset + text.length] = 0; buffer[offset + text.length] = 0;
return {}; return {};
@@ -248,7 +262,7 @@ export const string: WireType<string> = {
if (err) return { err }; 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) { size(value) {
const { err, text } = wantText(value); const { err, text } = wantText(value);
@@ -271,7 +285,7 @@ export const string: WireType<string> = {
if (rangeErr) return { err: rangeErr }; if (rangeErr) return { err: rangeErr };
buffer.writeUInt8(text.length, offset); buffer.writeUInt8(text.length, offset);
buffer.write(text, offset + 1, 'ascii'); buffer.write(text, offset + 1, 'latin1');
return {}; return {};
}, },
@@ -401,7 +415,7 @@ export const dest_address_array: WireType<DestAddress[]> = {
if ('dl_name' in dest) { if ('dl_name' in dest) {
buf.writeUInt8(2, offset++); 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 }; if (name.err) return { err: name.err };
@@ -417,7 +431,7 @@ export const dest_address_array: WireType<DestAddress[]> = {
if (npi.err) return { err: npi.err }; 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 }; if (addr.err) return { err: addr.err };
@@ -505,7 +519,7 @@ export const unsuccess_sme_array: WireType<UnsuccessSme[]> = {
if (npi.err) return { err: npi.err }; 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 }; if (addr.err) return { err: addr.err };
@@ -538,7 +552,7 @@ export const tlv = {
? offset + length ? offset + length
: terminator; : terminator;
return { bytesRead: length, value: buf.toString('ascii', offset, end) }; return { bytesRead: length, value: buf.toString('latin1', offset, end) };
}, },
size(value: ParamValue) { size(value: ParamValue) {
const { err, text } = wantText(value); const { err, text } = wantText(value);
@@ -559,7 +573,7 @@ export const tlv = {
read(buf: Buffer, offset: number, length = 0) { read(buf: Buffer, offset: number, length = 0) {
const err = outOfRange(buf, offset, length); 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) { size(value: ParamValue) {
const { err, text } = wantText(value); const { err, text } = wantText(value);
@@ -575,7 +589,7 @@ export const tlv = {
if (rangeErr) return { err: rangeErr }; if (rangeErr) return { err: rangeErr };
buf.write(text, offset, 'ascii'); buf.write(text, offset, 'latin1');
return {}; return {};
}, },
+18
View File
@@ -127,6 +127,24 @@ describe('parsing real PDUs', () => {
assert.equal(pduObj.params.short_message, 'hej 一'); assert.equal(pduObj.params.short_message, 'hej 一');
assert.equal(pduObj.tlvs.message_state?.tagValue, 2); 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', () => { describe('encoding submit_sm', () => {
+71 -1
View File
@@ -1,8 +1,8 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test, { describe } from 'node:test'; import test, { describe } from 'node:test';
import type { DestAddress, UnsuccessSme } from '../src/defs/types.ts'; 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 { tlvs } from '../src/defs/tlvs.ts';
import { types } from '../src/defs/types.ts';
describe('integers', () => { describe('integers', () => {
test('int8 reads, sizes and writes one octet', () => { test('int8 reads, sizes and writes one octet', () => {
@@ -69,6 +69,20 @@ describe('string (Octet String)', () => {
assert.deepEqual(target, encoded); 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)', () => { describe('cstring (C-Octet String)', () => {
@@ -100,6 +114,20 @@ describe('cstring (C-Octet String)', () => {
assert.deepEqual(types.cstring.size(123), { size: 4 }); 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', () => { 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); 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', () => { describe('buffer', () => {
const expected = Buffer.from('abcd1234'); const expected = Buffer.from('abcd1234');
@@ -170,6 +224,22 @@ describe('buffer', () => {
assert.deepEqual(target, expected); 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', () => { describe('dest_address_array', () => {
-9
View File
@@ -189,15 +189,6 @@ and is also what the panel ranked hardest — two methods, one answer.
### Correctness, ahead of everything below ### 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 - [ ] **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 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 `sendReturn(pduObj, 'ESME_RINVCMDID')`; `pduReturn()` then finds no response command, and the