From cf315737b71f23fe3aaa3d6778d533f5869c61b2 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 27 Aug 2026 16:32:01 +0200 Subject: [PATCH] Keep the body scrape for reserved message types and an empty receipt id marker --- AGENTS.md | 13 +++++++------ src/defs/constants.ts | 2 +- src/dlr.ts | 27 ++++++++++++++++----------- test/dlr.test.ts | 17 +++++++++++++++++ todo.md | 9 ++++++++- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bf74705..ff19045 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -202,12 +202,13 @@ exactly 140. Message type `MC_DELIVERY_RECEIPT` (0x04) makes it a receipt whatever the body parses to, so a receipt in a format `dlrFromPdu()` cannot read reaches `dlr` with `smsId` undefined instead of arriving as an inbound SMS. Any other named type — delivery or user acknowledgement, conversation - abort, intermediate notification — is not a receipt and its body is not scraped. Message type 0 - keeps the scrape: SMSCs that send text-only receipts leave `esm_class` at 0, and reading that as - the spec's "default message type" would lose every one of them. A `receipted_message_id` TLV marks - a receipt on the same footing where the message type is 0, since nothing but a receipt carries one. - What gets scraped is the decoded `short_message` with any UDH stripped, so a concatenated receipt - is read like any other. + abort, intermediate notification — is not a receipt and its body is not scraped. A message type of + 0, or one of the ten the spec reserves, keeps the scrape: SMSCs that send text-only receipts leave + `esm_class` at 0, and reading that as the spec's "default message type" would lose every one of + them. A non-empty `receipted_message_id` TLV marks a receipt on the same footing there, since + nothing but a receipt carries one. What gets scraped is the decoded `short_message` with any UDH + stripped; receipts never reach the reassembler, so an SMSC that splits one across segments gets a + `dlr` per segment rather than one merged report. The `message_state` TLV is authoritative only where it names a state in the table — SMPP reserves 0x80-0xFF for MC-vendor-specific values, so an unnameable one keeps its raw `statusId` and leaves `statusMsg` to the body. diff --git a/src/defs/constants.ts b/src/defs/constants.ts index 4c38276..0104e7e 100644 --- a/src/defs/constants.ts +++ b/src/defs/constants.ts @@ -103,11 +103,11 @@ export const consts = { }, } as const; -/** esm_class bits 5-2 name the message type; bit 6 is the UDH indicator. */ export function hasUdh(esmClass: number): boolean { return (esmClass & consts.ESM_CLASS.UDH_INDICATOR) === consts.ESM_CLASS.UDH_INDICATOR; } +/** Bits 5-2 of esm_class; the rest carry the messaging mode and the GSM features. */ export function messageTypeOf(esmClass: number): number { return esmClass & 0x3c; } diff --git a/src/dlr.ts b/src/dlr.ts index 55b78b2..db76b7d 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -125,13 +125,25 @@ export function parseReceipt(message: string): Receipt { type MessageType = 'other' | 'receipt' | 'unmarked'; +/** The message types the spec names that are not receipts. It reserves the remaining ten. */ +const notReceiptTypes: number[] = [ + consts.ESM_CLASS.CONVERSATION_ABORT, + consts.ESM_CLASS.DELIVERY_ACKNOWLEDGEMENT, + consts.ESM_CLASS.INTERMEDIATE_DELIVERY, + consts.ESM_CLASS.USER_ACKNOWLEDGEMENT, +]; + +function nonEmptyText(value: ParamValue | undefined): string | undefined { + return typeof value === 'string' && value !== '' ? value : undefined; +} + function messageType(pduObj: PduObject): MessageType { const type = messageTypeOf(paramNumber(pduObj.params.esm_class, 0)); if (type === consts.ESM_CLASS.MC_DELIVERY_RECEIPT) return 'receipt'; - if (type !== 0) return 'other'; + if (notReceiptTypes.includes(type)) return 'other'; - return pduObj.tlvs.receipted_message_id === undefined ? 'unmarked' : 'receipt'; + return nonEmptyText(pduObj.tlvs.receipted_message_id?.tagValue) === undefined ? 'unmarked' : 'receipt'; } /** A UDH-carrying short_message reaches here as a buffer, header and all. */ @@ -148,9 +160,7 @@ function receiptBody(pduObj: PduObject): string { } function receiptId(tlvId: ParamValue | undefined, receipt: Receipt | undefined): string | undefined { - if (typeof tlvId === 'string' && tlvId !== '') return tlvId; - - return receipt?.id === '' ? undefined : receipt?.id; + return nonEmptyText(tlvId) ?? (receipt?.id === '' ? undefined : receipt?.id); } function isMessageState(name: string | undefined): name is MessageState { @@ -173,12 +183,7 @@ function receiptStatus( return { statusId: tlvState, statusMsg: isMessageState(named) ? named : scraped }; } -/** - * Builds a delivery report from a deliver_sm, or nothing if the PDU carries a message rather than a - * receipt. `esm_class` decides that where the peer names a message type and a receipted_message_id - * TLV where it names none; failing both, the body is read for the standard receipt fields, which is - * the only thing Kannel and several other SMSCs send. - */ +/** The delivery report a deliver_sm carries, or nothing when it carries a message instead. */ export function dlrFromPdu(pduObj: PduObject): Dlr | undefined { const type = messageType(pduObj); diff --git a/test/dlr.test.ts b/test/dlr.test.ts index ea42eef..ffc047d 100644 --- a/test/dlr.test.ts +++ b/test/dlr.test.ts @@ -119,6 +119,7 @@ describe('dlrFromPdu()', () => { test('returns nothing when an unmarked deliver_sm identifies no message', () => { assert.equal(dlrFromPdu(deliverSm('just a normal sms', undefined, 0)), undefined); + assert.equal(dlrFromPdu(deliverSm('id:0195f0c7 stat:WEIRDXX', undefined, 0)), undefined); }); test('still reads the body when the peer marks no message type', () => { @@ -171,6 +172,22 @@ describe('dlrFromPdu()', () => { assert.ok(dlr); assert.equal(dlr.smsId, 'from-the-tlv'); assert.equal(dlr.statusMsg, 'UNKNOWN'); + + const empty = dlrFromPdu(deliverSm('an ordinary inbound message', { + receipted_message_id: { tagValue: '' }, + }, 0)); + + assert.equal(empty, undefined, 'an empty id marks nothing'); + }); + + // The spec names six of the sixteen message types and reserves the rest, so a peer that types + // its receipts with one of the reserved bits keeps the body scrape rather than losing them. + test('keeps the body scrape for a message type the spec reserves', () => { + const dlr = dlrFromPdu(deliverSm(receiptText, undefined, 0x0c)); + + assert.ok(dlr); + assert.equal(dlr.smsId, '0195f0c7'); + assert.equal(dlr.statusMsg, 'DELIVERED'); }); test('leaves a message the peer marked as another type to arrive as an SMS', () => { diff --git a/todo.md b/todo.md index 2e4cce1..3d72fb8 100644 --- a/todo.md +++ b/todo.md @@ -5,7 +5,7 @@ rules there constrain every item below. ## Status -The rewrite is **feature complete and green**: 230 tests, lint and typecheck clean, verified on Node +The rewrite is **feature complete and green**: 231 tests, lint and typecheck clean, verified on Node 18, 20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0. ```bash @@ -147,6 +147,13 @@ session message is a change to every call site. `reassembly`, `dlr-merger`, `send-window`, `link-timers`, `reconnect-loop`, `pending-requests` and `send-sms`, so the directory would make that boundary visible. Do it on the next extraction out of `session.ts`, not as a move of its own. +- [ ] **Does an intermediate delivery notification deserve to be a `dlr`?** `esm_class` message type + `INTERMEDIATE_DELIVERY` (0x20) is classified as a message today, so a peer that reports + non-final states with it hands the application a raw `id:… stat:ENROUTE` text as an inbound + SMS. Kannel treats 0x04, 0x08 and 0x20 alike as report-bearing. Against it: a non-final report + would take a segment's slot in `DlrMerger` and complete the group early. Raised by review, + 2026-08-27; needs a decision. + - [ ] **`submit_multi` and the broadcast commands** encode and decode, but nothing exercises them end to end. The interop suite is the natural place. - [ ] **Move to TypeScript 7** once `typescript-eslint` supports it; `renovate.json` pins TypeScript