diff --git a/AGENTS.md b/AGENTS.md index 34678b3..1ebe11f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -292,7 +292,10 @@ exactly 140. inside a `-` 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 `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 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 diff --git a/README.md b/README.md index a35c594..ddb8833 100644 --- a/README.md +++ b/README.md @@ -172,6 +172,8 @@ before you see them: 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 what the peer wrote either way — `pduObjs` from the send, and the second argument of the `dlr` event. diff --git a/src/client.ts b/src/client.ts index ba07528..0176f19 100644 --- a/src/client.ts +++ b/src/client.ts @@ -168,7 +168,7 @@ async function connect(options: ClientOptions, log: SmppLog): Promise { - const dlr = dlrFromPdu(pduObj, this.receiptIdNotation); + const dlr = dlrFromPdu(pduObj, this.smsIdFormat); if (!dlr) { this.onMessage(pduObj); diff --git a/src/session-options.ts b/src/session-options.ts index 3370fef..9905f14 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -145,17 +145,17 @@ function checkSmsIdFormat(smsIdFormat: unknown): VoidResult { return { err: new Error('smsIdFormat names a notation per place, as { receipt, submitResp }') }; } - const allowed = smsIdNotations.join(' or '); - - for (const place of smsIdPlaces) { - const notation = smsIdFormat[place]; + for (const [place, notation] of Object.entries(smsIdFormat)) { + if (!smsIdPlaces.includes(place)) { + return { err: new Error(`smsIdFormat has no ${place}, name ${smsIdPlaces.join(' or ')}`) }; + } if (notation === undefined || isSmsIdNotation(notation)) continue; // String() throws on a null-prototype object, and this value is whatever the caller passed. 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 {}; diff --git a/src/session.ts b/src/session.ts index 95ca5f8..e18fd53 100644 --- a/src/session.ts +++ b/src/session.ts @@ -120,8 +120,8 @@ export class Session extends EventEmitter { maxReassembly: options.maxReassembly, onRequest: options.onRequest, reassemblyTimeout: options.reassemblyTimeout, - receiptIdNotation: options.smsIdFormat?.receipt, session: this, + smsIdFormat: options.smsIdFormat, systemId: options.systemId, }); this.pending = new PendingRequests(this.log); diff --git a/src/sms-id.ts b/src/sms-id.ts index 6f54ef6..202161a 100644 --- a/src/sms-id.ts +++ b/src/sms-id.ts @@ -3,18 +3,17 @@ const notations = { hex: { digits: /^[0-9a-f]+$/i, prefix: '0x' }, }; +const places = ['receipt', 'submitResp'] as const; + /** The notation a peer writes message ids in. */ export type SmsIdNotation = keyof typeof notations; /** The notation per place the peer writes an id. An omitted place is left as it arrived. */ -export type SmsIdFormat = { - receipt?: SmsIdNotation | undefined; - submitResp?: SmsIdNotation | undefined; -}; +export type SmsIdFormat = Partial>; 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 { 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 - * receipt in another still correlates. An id the notation cannot read is left as it arrived, which - * is what leaves a `-` id whole for DlrMerger. + * receipt in another still correlates. */ 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]; diff --git a/test/dlr.test.ts b/test/dlr.test.ts index b569394..2dc499c 100644 --- a/test/dlr.test.ts +++ b/test/dlr.test.ts @@ -208,28 +208,35 @@ describe('dlrFromPdu()', () => { }); 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.equal(hex.smsId, '6699'); 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('nothing scrapable here', { + assert.equal(dlrFromPdu(deliverSm('id:0000123 stat:DELIVRD'), { receipt: 'decimal' })?.smsId, '123'); + }); + + // 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' }, - }, 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', () => { - assert.equal(dlrFromPdu(deliverSm('id:beef-1 stat:DELIVRD'), 'hex')?.smsId, 'beef-1'); - assert.equal(dlrFromPdu(deliverSm('id:1a2b stat:DELIVRD'), 'decimal')?.smsId, '1a2b'); + assert.equal(dlrFromPdu(deliverSm('id:beef-1 stat:DELIVRD'), { receipt: 'hex' })?.smsId, 'beef-1'); + assert.equal(dlrFromPdu(deliverSm('id:1a2b stat:DELIVRD'), { receipt: 'decimal' })?.smsId, '1a2b'); assert.equal(dlrFromPdu(deliverSm('id:0195f0c7 stat:DELIVRD'))?.smsId, '0195f0c7'); }); // 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', () => { assert.equal( - dlrFromPdu(deliverSm('id:9007199254740993 stat:DELIVRD'), 'decimal')?.smsId, + dlrFromPdu(deliverSm('id:9007199254740993 stat:DELIVRD'), { receipt: 'decimal' })?.smsId, '9007199254740993', ); }); diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 3adf63b..81c564a 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -79,7 +79,7 @@ function peerOf(smpp: SmppServer): Session { return peer; } -async function sendReceipt(peer: Session, smsId: string): Promise { +async function sendReceipt(peer: Session, smsId: string, tlvSmsId = smsId): Promise { const sent = await peer.send({ cmdName: 'deliver_sm', params: { @@ -90,7 +90,7 @@ async function sendReceipt(peer: Session, smsId: string): Promise { }, tlvs: { 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); - const reported = once(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'); assert.deepEqual(sent.smsIds, ['6699']); 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 => { @@ -874,6 +880,11 @@ describe('message id notation', () => { assert.match(checked.err.message, /smsIdFormat\.receipt/); // The shape todo.md sketched, which a caller without types would otherwise pass unnoticed. 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); }); }); diff --git a/todo.md b/todo.md index a082f0a..4a3b0db 100644 --- a/todo.md +++ b/todo.md @@ -156,6 +156,11 @@ session message is a change to every call site. - [ ] **Coverage reporting.** `node --test --experimental-test-coverage` works today; nothing 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, 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