Leave an unknown notation alone and read a receipt's TLV as the id the submit answered

This commit is contained in:
2026-08-30 23:01:39 +02:00
parent 696e018b77
commit 706b61d6ca
11 changed files with 70 additions and 39 deletions
+4 -1
View File
@@ -292,7 +292,10 @@ exactly 140.
inside a `<base>-<n>` id.** An SMSC may answer `submit_sm_resp` in hex and write the receipt's inside a `<base>-<n>` id.** An SMSC may answer `submit_sm_resp` in hex and write the receipt's
`id:` in decimal, so one transform over both sides cannot make them equal — `smsIdFormat` names `id:` in decimal, so one transform over both sides cannot make them equal — `smsIdFormat` names
`receipt` and `submitResp` separately and reads both into a plain decimal value before `smsIds` `receipt` and `submitResp` separately and reads both into a plain decimal value before `smsIds`
and `dlr.smsId` are compared. Omitting a place is what leaving it alone means, so there is no and `dlr.smsId` are compared. `submitResp` covers the `receipted_message_id` TLV too, which SMPP
3.4 5.3.2.26 defines as the id the `submit_sm_resp` carried: naming one notation for whichever id
a receipt yields would break the peer that sends both, whose TLV correlated before the option was
set. Omitting a place is what leaving it alone means, so there is no
`raw` notation, and a caller-supplied formatter is refused because it would make the promise that `raw` notation, and a caller-supplied formatter is refused because it would make the promise that
those two are comparable unverifiable — `onRequest` and the PDU on the `dlr` event are the escape those two are comparable unverifiable — `onRequest` and the PDU on the `dlr` event are the escape
hatches, and the `onReceipt` hook in todo.md is the seam if one is wanted. An id no notation reads hatches, and the `onReceipt` hook in todo.md is the seam if one is wanted. An id no notation reads
+2
View File
@@ -172,6 +172,8 @@ before you see them:
const { err, session } = await client({ smsIdFormat: { receipt: 'decimal', submitResp: 'hex' } }); const { err, session } = await client({ smsIdFormat: { receipt: 'decimal', submitResp: 'hex' } });
``` ```
`receipt` is the notation of the receipt body's `id:` field, `submitResp` that of the `message_id`
a `submit_sm_resp` carries — and of a receipt's `receipted_message_id` TLV, which is that same id.
An id that is not a number in the notation given is left exactly as it arrived, and the PDUs carry An id that is not a number in the notation given is left exactly as it arrived, and the PDUs carry
what the peer wrote either way — `pduObjs` from the send, and the second argument of the `dlr` event. what the peer wrote either way — `pduObjs` from the send, and the second argument of the `dlr` event.
+1 -1
View File
@@ -168,7 +168,7 @@ async function connect(options: ClientOptions, log: SmppLog): Promise<Result<{ s
const checked = checkSessionOptions(options); const checked = checkSessionOptions(options);
if (checked.err) { if (checked.err) {
log.warn('client - option out of range', { message: checked.err.message }); log.warn('client - unusable option', { message: checked.err.message });
return { err: checked.err }; return { err: checked.err };
} }
+11 -6
View File
@@ -1,7 +1,7 @@
import type { MessageState } from './defs/constants.ts'; import type { MessageState } from './defs/constants.ts';
import type { ParamValue } from './defs/types.ts'; import type { ParamValue } from './defs/types.ts';
import type { PduObject } from './pdu.ts'; import type { PduObject } from './pdu.ts';
import type { SmsIdNotation } from './sms-id.ts'; import type { SmsIdFormat } from './sms-id.ts';
import { consts, constsById, messageTypeOf } from './defs/constants.ts'; import { consts, constsById, messageTypeOf } from './defs/constants.ts';
import { decodeMessage } from './message.ts'; import { decodeMessage } from './message.ts';
import { normaliseSmsId } from './sms-id.ts'; import { normaliseSmsId } from './sms-id.ts';
@@ -164,11 +164,16 @@ function receiptBody(pduObj: PduObject): string {
function receiptId( function receiptId(
tlvId: ParamValue | undefined, tlvId: ParamValue | undefined,
receipt: Receipt | undefined, receipt: Receipt | undefined,
notation: SmsIdNotation | undefined, format: SmsIdFormat,
): string | undefined { ): string | undefined {
const id = nonEmptyText(tlvId) ?? nonEmptyText(receipt?.id); // SMPP 3.4 5.3.2.26: the TLV is the id the submit_sm_resp carried, where the body's is a rendering.
const fromTlv = nonEmptyText(tlvId);
return id === undefined ? undefined : normaliseSmsId(id, notation); if (fromTlv !== undefined) return normaliseSmsId(fromTlv, format.submitResp);
const fromBody = nonEmptyText(receipt?.id);
return fromBody === undefined ? undefined : normaliseSmsId(fromBody, format.receipt);
} }
function isMessageState(name: string | undefined): name is MessageState { function isMessageState(name: string | undefined): name is MessageState {
@@ -192,14 +197,14 @@ function receiptStatus(
} }
/** The delivery report a deliver_sm carries, or nothing when it carries a message instead. */ /** The delivery report a deliver_sm carries, or nothing when it carries a message instead. */
export function dlrFromPdu(pduObj: PduObject, notation?: SmsIdNotation): Dlr | undefined { export function dlrFromPdu(pduObj: PduObject, format: SmsIdFormat = {}): Dlr | undefined {
const type = messageType(pduObj); const type = messageType(pduObj);
if (type === 'other') return undefined; if (type === 'other') return undefined;
const body = receiptBody(pduObj); const body = receiptBody(pduObj);
const receipt = body === '' ? undefined : parseReceipt(body); const receipt = body === '' ? undefined : parseReceipt(body);
const smsId = receiptId(pduObj.tlvs.receipted_message_id?.tagValue, receipt, notation); const smsId = receiptId(pduObj.tlvs.receipted_message_id?.tagValue, receipt, format);
const { statusId, statusMsg } = receiptStatus(pduObj.tlvs.message_state?.tagValue, receipt); const { statusId, statusMsg } = receiptStatus(pduObj.tlvs.message_state?.tagValue, receipt);
if (type === 'unmarked' && (smsId === undefined || statusMsg === undefined)) return undefined; if (type === 'unmarked' && (smsId === undefined || statusMsg === undefined)) return undefined;
+5 -5
View File
@@ -3,7 +3,7 @@ import type { OnRequest } from './session-options.ts';
import type { PduObject } from './pdu.ts'; import type { PduObject } from './pdu.ts';
import type { Session } from './session.ts'; import type { Session } from './session.ts';
import type { SmppLog } from './log.ts'; import type { SmppLog } from './log.ts';
import type { SmsIdNotation } from './sms-id.ts'; import type { SmsIdFormat } from './sms-id.ts';
import { Reassembler, decodeSegments } from './reassembly.ts'; import { Reassembler, decodeSegments } from './reassembly.ts';
import { bindCommands, defaults } from './session-options.ts'; import { bindCommands, defaults } from './session-options.ts';
import { concatInfo } from './udh.ts'; import { concatInfo } from './udh.ts';
@@ -19,7 +19,7 @@ export type IncomingRequestsOptions = {
maxReassembly?: number | undefined; maxReassembly?: number | undefined;
onRequest?: OnRequest | undefined; onRequest?: OnRequest | undefined;
reassemblyTimeout?: number | undefined; reassemblyTimeout?: number | undefined;
receiptIdNotation?: SmsIdNotation | undefined; smsIdFormat?: SmsIdFormat | undefined;
session: Session; session: Session;
systemId?: string | undefined; systemId?: string | undefined;
}; };
@@ -30,8 +30,8 @@ export class IncomingRequests {
private readonly log: SmppLog; private readonly log: SmppLog;
private readonly onRequest: OnRequest | undefined; private readonly onRequest: OnRequest | undefined;
private readonly reassembler: Reassembler; private readonly reassembler: Reassembler;
private readonly receiptIdNotation: SmsIdNotation | undefined;
private readonly session: Session; private readonly session: Session;
private readonly smsIdFormat: SmsIdFormat;
private readonly systemId: string; private readonly systemId: string;
constructor(options: IncomingRequestsOptions) { constructor(options: IncomingRequestsOptions) {
@@ -44,8 +44,8 @@ export class IncomingRequests {
maxOctets: options.maxOctets, maxOctets: options.maxOctets,
timeout: options.reassemblyTimeout ?? defaults.reassemblyTimeout, timeout: options.reassemblyTimeout ?? defaults.reassemblyTimeout,
}); });
this.receiptIdNotation = options.receiptIdNotation;
this.session = options.session; this.session = options.session;
this.smsIdFormat = options.smsIdFormat ?? {};
this.systemId = options.systemId ?? defaults.systemId; this.systemId = options.systemId ?? defaults.systemId;
} }
@@ -101,7 +101,7 @@ export class IncomingRequests {
/** SMPP carries a mobile-originated message and a delivery receipt on the same command. */ /** SMPP carries a mobile-originated message and a delivery receipt on the same command. */
private async onDeliverSm(pduObj: PduObject): Promise<void> { private async onDeliverSm(pduObj: PduObject): Promise<void> {
const dlr = dlrFromPdu(pduObj, this.receiptIdNotation); const dlr = dlrFromPdu(pduObj, this.smsIdFormat);
if (!dlr) { if (!dlr) {
this.onMessage(pduObj); this.onMessage(pduObj);
+5 -5
View File
@@ -145,17 +145,17 @@ function checkSmsIdFormat(smsIdFormat: unknown): VoidResult {
return { err: new Error('smsIdFormat names a notation per place, as { receipt, submitResp }') }; return { err: new Error('smsIdFormat names a notation per place, as { receipt, submitResp }') };
} }
const allowed = smsIdNotations.join(' or '); for (const [place, notation] of Object.entries(smsIdFormat)) {
if (!smsIdPlaces.includes(place)) {
for (const place of smsIdPlaces) { return { err: new Error(`smsIdFormat has no ${place}, name ${smsIdPlaces.join(' or ')}`) };
const notation = smsIdFormat[place]; }
if (notation === undefined || isSmsIdNotation(notation)) continue; if (notation === undefined || isSmsIdNotation(notation)) continue;
// String() throws on a null-prototype object, and this value is whatever the caller passed. // String() throws on a null-prototype object, and this value is whatever the caller passed.
const got = typeof notation === 'string' ? notation : typeof notation; const got = typeof notation === 'string' ? notation : typeof notation;
return { err: new Error(`smsIdFormat.${place} must be ${allowed}, got ${got}`) }; return { err: new Error(`smsIdFormat.${place} must be ${smsIdNotations.join(' or ')}, got ${got}`) };
} }
return {}; return {};
+1 -1
View File
@@ -120,8 +120,8 @@ export class Session extends EventEmitter<SessionEvents> {
maxReassembly: options.maxReassembly, maxReassembly: options.maxReassembly,
onRequest: options.onRequest, onRequest: options.onRequest,
reassemblyTimeout: options.reassemblyTimeout, reassemblyTimeout: options.reassemblyTimeout,
receiptIdNotation: options.smsIdFormat?.receipt,
session: this, session: this,
smsIdFormat: options.smsIdFormat,
systemId: options.systemId, systemId: options.systemId,
}); });
this.pending = new PendingRequests(this.log); this.pending = new PendingRequests(this.log);
+6 -8
View File
@@ -3,18 +3,17 @@ const notations = {
hex: { digits: /^[0-9a-f]+$/i, prefix: '0x' }, hex: { digits: /^[0-9a-f]+$/i, prefix: '0x' },
}; };
const places = ['receipt', 'submitResp'] as const;
/** The notation a peer writes message ids in. */ /** The notation a peer writes message ids in. */
export type SmsIdNotation = keyof typeof notations; export type SmsIdNotation = keyof typeof notations;
/** The notation per place the peer writes an id. An omitted place is left as it arrived. */ /** The notation per place the peer writes an id. An omitted place is left as it arrived. */
export type SmsIdFormat = { export type SmsIdFormat = Partial<Record<typeof places[number], SmsIdNotation | undefined>>;
receipt?: SmsIdNotation | undefined;
submitResp?: SmsIdNotation | undefined;
};
export const smsIdNotations: string[] = Object.keys(notations); export const smsIdNotations: string[] = Object.keys(notations);
export const smsIdPlaces: (keyof SmsIdFormat)[] = ['receipt', 'submitResp']; export const smsIdPlaces: readonly string[] = places;
export function isSmsIdNotation(value: unknown): value is SmsIdNotation { export function isSmsIdNotation(value: unknown): value is SmsIdNotation {
return typeof value === 'string' && Object.hasOwn(notations, value); return typeof value === 'string' && Object.hasOwn(notations, value);
@@ -25,11 +24,10 @@ const maxIdLength = 64;
/** /**
* The id as a plain decimal value, so an SMSC that answers a submit in one notation and writes the * The id as a plain decimal value, so an SMSC that answers a submit in one notation and writes the
* receipt in another still correlates. An id the notation cannot read is left as it arrived, which * receipt in another still correlates.
* is what leaves a `<base>-<n>` id whole for DlrMerger.
*/ */
export function normaliseSmsId(id: string, notation: SmsIdNotation | undefined): string { export function normaliseSmsId(id: string, notation: SmsIdNotation | undefined): string {
if (notation === undefined || id.length > maxIdLength) return id; if (!isSmsIdNotation(notation) || id.length > maxIdLength) return id;
const { digits, prefix } = notations[notation]; const { digits, prefix } = notations[notation];
+14 -7
View File
@@ -208,28 +208,35 @@ describe('dlrFromPdu()', () => {
}); });
test('reads the id in the notation the peer writes receipts in', () => { test('reads the id in the notation the peer writes receipts in', () => {
const hex = dlrFromPdu(deliverSm('id:1a2B stat:DELIVRD err:000 text:'), 'hex'); const hex = dlrFromPdu(deliverSm('id:1a2B stat:DELIVRD err:000 text:'), { receipt: 'hex' });
assert.ok(hex); assert.ok(hex);
assert.equal(hex.smsId, '6699'); assert.equal(hex.smsId, '6699');
assert.equal(hex.receipt?.id, '1a2B', 'the receipt itself keeps the id as it arrived'); assert.equal(hex.receipt?.id, '1a2B', 'the receipt itself keeps the id as it arrived');
assert.equal(dlrFromPdu(deliverSm('id:0000123 stat:DELIVRD'), 'decimal')?.smsId, '123'); assert.equal(dlrFromPdu(deliverSm('id:0000123 stat:DELIVRD'), { receipt: 'decimal' })?.smsId, '123');
assert.equal(dlrFromPdu(deliverSm('nothing scrapable here', { });
// SMPP 3.4 5.3.2.26 makes the TLV the id the submit_sm_resp carried, not the body's rendering.
test('reads the receipted_message_id TLV in the notation the peer answers a submit in', () => {
const marked = deliverSm('nothing scrapable here', {
receipted_message_id: { tagValue: 'FF' }, receipted_message_id: { tagValue: 'FF' },
}, 0), 'hex')?.smsId, '255'); }, 0);
assert.equal(dlrFromPdu(marked, { submitResp: 'hex' })?.smsId, '255');
assert.equal(dlrFromPdu(marked, { receipt: 'hex' })?.smsId, 'FF');
}); });
test('leaves an id the notation cannot read as it arrived', () => { test('leaves an id the notation cannot read as it arrived', () => {
assert.equal(dlrFromPdu(deliverSm('id:beef-1 stat:DELIVRD'), 'hex')?.smsId, 'beef-1'); assert.equal(dlrFromPdu(deliverSm('id:beef-1 stat:DELIVRD'), { receipt: 'hex' })?.smsId, 'beef-1');
assert.equal(dlrFromPdu(deliverSm('id:1a2b stat:DELIVRD'), 'decimal')?.smsId, '1a2b'); assert.equal(dlrFromPdu(deliverSm('id:1a2b stat:DELIVRD'), { receipt: 'decimal' })?.smsId, '1a2b');
assert.equal(dlrFromPdu(deliverSm('id:0195f0c7 stat:DELIVRD'))?.smsId, '0195f0c7'); assert.equal(dlrFromPdu(deliverSm('id:0195f0c7 stat:DELIVRD'))?.smsId, '0195f0c7');
}); });
// Number() reads 9007199254740993 as ...92, which correlates a receipt to the wrong send. // Number() reads 9007199254740993 as ...92, which correlates a receipt to the wrong send.
test('reads an id past the safe integer range without losing a digit', () => { test('reads an id past the safe integer range without losing a digit', () => {
assert.equal( assert.equal(
dlrFromPdu(deliverSm('id:9007199254740993 stat:DELIVRD'), 'decimal')?.smsId, dlrFromPdu(deliverSm('id:9007199254740993 stat:DELIVRD'), { receipt: 'decimal' })?.smsId,
'9007199254740993', '9007199254740993',
); );
}); });
+16 -5
View File
@@ -79,7 +79,7 @@ function peerOf(smpp: SmppServer): Session {
return peer; return peer;
} }
async function sendReceipt(peer: Session, smsId: string): Promise<void> { async function sendReceipt(peer: Session, smsId: string, tlvSmsId = smsId): Promise<void> {
const sent = await peer.send({ const sent = await peer.send({
cmdName: 'deliver_sm', cmdName: 'deliver_sm',
params: { params: {
@@ -90,7 +90,7 @@ async function sendReceipt(peer: Session, smsId: string): Promise<void> {
}, },
tlvs: { tlvs: {
message_state: { tagValue: consts.MESSAGE_STATE.DELIVERED }, message_state: { tagValue: consts.MESSAGE_STATE.DELIVERED },
receipted_message_id: { tagValue: smsId }, receipted_message_id: { tagValue: tlvSmsId },
}, },
}); });
@@ -831,15 +831,21 @@ describe('message id notation', () => {
assert.ok(session); assert.ok(session);
const reported = once<Dlr>(resolve => { session.on('dlr', resolve); }); const reported = once<[Dlr, PduObject]>(resolve => {
session.on('dlr', (dlr, pduObj) => { resolve([dlr, pduObj]); });
});
const sent = await sendOne(session, 'one segment'); const sent = await sendOne(session, 'one segment');
assert.deepEqual(sent.smsIds, ['6699']); assert.deepEqual(sent.smsIds, ['6699']);
assert.equal(paramText(sent.pduObjs[0]?.params.message_id), '1a2b', 'the PDU keeps the id it carried'); assert.equal(paramText(sent.pduObjs[0]?.params.message_id), '1a2b', 'the PDU keeps the id it carried');
await sendReceipt(peerOf(smpp), '6699'); // The receipt renders the id in decimal and mirrors the answered one in its TLV, as the spec has it.
await sendReceipt(peerOf(smpp), '6699', '1a2b');
assert.equal((await reported).smsId, sent.smsIds[0]); const [dlr, pduObj] = await reported;
assert.equal(dlr.smsId, sent.smsIds[0]);
assert.equal(paramText(pduObj.tlvs.receipted_message_id?.tagValue), '1a2b');
}); });
test('leaves the segment ids of a multipart send to merge as they are', async t => { test('leaves the segment ids of a multipart send to merge as they are', async t => {
@@ -874,6 +880,11 @@ describe('message id notation', () => {
assert.match(checked.err.message, /smsIdFormat\.receipt/); assert.match(checked.err.message, /smsIdFormat\.receipt/);
// The shape todo.md sketched, which a caller without types would otherwise pass unnoticed. // The shape todo.md sketched, which a caller without types would otherwise pass unnoticed.
assert.ok(checkSessionOptions({ smsIdFormat: 'hex' }).err instanceof Error); assert.ok(checkSessionOptions({ smsIdFormat: 'hex' }).err instanceof Error);
assert.match(
checkSessionOptions({ smsIdFormat: { receipts: 'decimal' } }).err?.message ?? '',
/receipts/,
'a misspelled place is the same silent no-op',
);
assert.equal(checkSessionOptions({ smsIdFormat: { submitResp: 'hex' } }).err, undefined); assert.equal(checkSessionOptions({ smsIdFormat: { submitResp: 'hex' } }).err, undefined);
}); });
}); });
+5
View File
@@ -156,6 +156,11 @@ session message is a change to every call site.
- [ ] **Coverage reporting.** `node --test --experimental-test-coverage` works today; nothing - [ ] **Coverage reporting.** `node --test --experimental-test-coverage` works today; nothing
publishes the numbers. publishes the numbers.
- [ ] **A receipt whose fields are separated by anything but a space reads as one field.**
`parseReceipt()` takes `id:` as everything up to the next space, so a peer writing CRLF
between fields yields an id of `1a2b\r\nstat:DELIVRD` — one nothing correlates and no
notation can read. Every field pattern has the same shape. Raised by review, 2026-08-30.
- [ ] **An `onReceipt` hook.** Receipt text is only loosely specified and operators disagree on it, - [ ] **An `onReceipt` hook.** Receipt text is only loosely specified and operators disagree on it,
but `dlrFromPdu()` is wired into `IncomingRequests` with no seam of its own: an application but `dlrFromPdu()` is wired into `IncomingRequests` with no seam of its own: an application
facing a format we do not parse has to take the whole PDU on `onRequest` and reimplement the facing a format we do not parse has to take the whole PDU on `onRequest` and reimplement the