diff --git a/AGENTS.md b/AGENTS.md index dfd4751..62a9b4b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -204,7 +204,11 @@ exactly 140. 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. + 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. + 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. - **The TLS tests build their own self-signed certificate in DER** (`test/tls.test.ts`) instead of adding a devDependency or shelling out to openssl. Maintainer's call, 2026-08-26: the dev image diff --git a/README.md b/README.md index 3b8c9e1..e4c47f3 100644 --- a/README.md +++ b/README.md @@ -155,8 +155,9 @@ session.on('sms', async sms => { ``` Delivery receipts travel on the same SMPP command but reach you as `dlr`, so nothing you write has -to tell the two apart. `esm_class` is what tells them apart; a peer that marks no message type -there has the message body read for the standard `id:` and `stat:` receipt fields instead. +to tell the two apart. `esm_class` is what tells them apart; where it names no message type a +`receipted_message_id` TLV does, and failing both the message body is read for the standard +`id:` and `stat:` receipt fields. ## Server @@ -295,7 +296,7 @@ TypeScript users can import `SmppLog` to have the compiler check one. | Event | Fires when | | --- | --- | | `sms` | An SMS arrives, reassembled if it was multipart. Carries `sendResp()`, `sendDlr()` and the `smsId` it was answered with. | -| `dlr` | A delivery report arrives, one per segment. `smsId` is undefined when the peer marked a receipt whose body carries no readable id. | +| `dlr` | A delivery report arrives, one per segment. `smsId` is undefined when the peer marked a receipt whose body carries no readable id. `statusMsg` names `statusId` unless the peer sent a `message_state` this library cannot name — then `statusId` is that raw value and `statusMsg` is whatever the body said, or `UNKNOWN`. | | `messageDlr` | Every segment of a multipart message sent with `dlr: true` has been reported on, carrying the worst status of the segments. Merging needs the SMSC to number its segment ids `-`, which is this library's own server's convention — an SMSC that hands out unrelated ids per segment never fires it. | | `close` | The connection closed. | | `reconnected` | The client re-bound after a drop (only with `reconnect` configured). | diff --git a/src/defs/types.ts b/src/defs/types.ts index e843245..6959e67 100644 --- a/src/defs/types.ts +++ b/src/defs/types.ts @@ -33,6 +33,10 @@ export function paramText(value: ParamValue | undefined): string { return ''; } +export function paramNumber(value: ParamValue | undefined, fallback: number): number { + return typeof value === 'number' ? value : fallback; +} + function outOfRange(buffer: Buffer, offset: number, needed: number): Error | undefined { if (offset < 0 || needed < 0 || offset + needed > buffer.length) { return new Error( diff --git a/src/dlr-merger.ts b/src/dlr-merger.ts index a33b62d..c8d3d3e 100644 --- a/src/dlr-merger.ts +++ b/src/dlr-merger.ts @@ -3,7 +3,7 @@ import type { MessageState } from './defs/constants.ts'; import type { SmppLog } from './log.ts'; import { ExpiringGroups } from './expiring-groups.ts'; -export type MessageDlr = Dlr & { segments: Dlr[] }; +export type MessageDlr = Dlr & { segments: Dlr[]; smsId: string }; export type DlrMergerOptions = { log: SmppLog; @@ -42,12 +42,6 @@ const severity: Record = { UNDELIVERABLE: 9, }; -function severityOf(dlr: Dlr): number { - const ranked: Record = severity; - - return ranked[dlr.statusMsg] ?? severity.UNKNOWN; -} - export class DlrMerger { private readonly groups: ExpiringGroups; private readonly log: SmppLog; @@ -112,7 +106,7 @@ export class DlrMerger { this.groups.delete(base); const segments = [...group.parts.entries()].sort(([a], [b]) => a - b).map(([, one]) => one); - const worst = segments.reduce((carry, one) => (severityOf(one) > severityOf(carry) ? one : carry)); + const worst = segments.reduce((carry, one) => (severity[one.statusMsg] > severity[carry.statusMsg] ? one : carry)); return { ...worst, segments, smsId: base }; } diff --git a/src/dlr.ts b/src/dlr.ts index 80d84aa..f909c7f 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -2,6 +2,8 @@ import type { MessageState } from './defs/constants.ts'; import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; import { consts, constsById } from './defs/constants.ts'; +import { decodeMessage } from './message.ts'; +import { paramNumber, paramText } from './defs/types.ts'; /** * The seven-character status codes carried in a receipt's `stat:` field, mapped to the @@ -49,7 +51,7 @@ export type Dlr = { receipt: Receipt | undefined; smsId: string | undefined; statusId: number; - statusMsg: string; + statusMsg: MessageState; }; const field = (name: string) => new RegExp(`\\b${name}:([^ ]*)`, 'i'); @@ -127,15 +129,25 @@ const messageTypeBits = 0x3c; type MessageType = 'other' | 'receipt' | 'unmarked'; function messageType(pduObj: PduObject): MessageType { - const esmClass = pduObj.params.esm_class; - - if (typeof esmClass !== 'number') return 'unmarked'; - - const type = esmClass & messageTypeBits; + const type = paramNumber(pduObj.params.esm_class, 0) & messageTypeBits; if (type === consts.ESM_CLASS.MC_DELIVERY_RECEIPT) return 'receipt'; + if (type !== 0) return 'other'; - return type === 0 ? 'unmarked' : 'other'; + return pduObj.tlvs.receipted_message_id === undefined ? 'unmarked' : 'receipt'; +} + +/** A UDH-carrying short_message reaches here as a buffer, header and all. */ +function receiptBody(pduObj: PduObject): string { + const message = pduObj.params.short_message; + + if (!Buffer.isBuffer(message)) return paramText(message); + + return decodeMessage( + message, + paramNumber(pduObj.params.data_coding, 0), + paramNumber(pduObj.params.esm_class, 0), + ).message; } function receiptId(tlvId: ParamValue | undefined, receipt: Receipt | undefined): string | undefined { @@ -144,32 +156,39 @@ function receiptId(tlvId: ParamValue | undefined, receipt: Receipt | undefined): return receipt?.id === '' ? undefined : receipt?.id; } +function isMessageState(name: string | undefined): name is MessageState { + return name !== undefined && name in consts.MESSAGE_STATE; +} + +/** The state TLV wins where it names a state we know; an unnameable one leaves the body to say. */ function receiptStatus( tlvState: ParamValue | undefined, receipt: Receipt | undefined, -): { statusId: number; statusMsg: string | undefined } { - if (typeof tlvState === 'number') { - return { statusId: tlvState, statusMsg: constsById.MESSAGE_STATE?.[tlvState] }; +): { statusId: number; statusMsg: MessageState | undefined } { + const scraped = receiptStates[receipt?.stat?.toUpperCase() ?? '']; + + if (typeof tlvState !== 'number') { + return { statusId: consts.MESSAGE_STATE[scraped ?? 'UNKNOWN'], statusMsg: scraped }; } - const state = receiptStates[receipt?.stat?.toUpperCase() ?? '']; + const named = constsById.MESSAGE_STATE?.[tlvState]; - return { statusId: consts.MESSAGE_STATE[state ?? 'UNKNOWN'], statusMsg: state }; + 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 sets a message type; where it sets none, the body - * is read for the standard receipt fields, which is the only thing Kannel and several other SMSCs - * send. The message_state and receipted_message_id TLVs are authoritative over the body. + * 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. */ export function dlrFromPdu(pduObj: PduObject): Dlr | undefined { const type = messageType(pduObj); if (type === 'other') return undefined; - const message = pduObj.params.short_message; - const receipt = typeof message === 'string' ? parseReceipt(message) : undefined; + const body = receiptBody(pduObj); + const receipt = body === '' ? undefined : parseReceipt(body); const smsId = receiptId(pduObj.tlvs.receipted_message_id?.tagValue, receipt); const { statusId, statusMsg } = receiptStatus(pduObj.tlvs.message_state?.tagValue, receipt); diff --git a/src/pdu.ts b/src/pdu.ts index d2b04e5..206e58e 100644 --- a/src/pdu.ts +++ b/src/pdu.ts @@ -8,6 +8,7 @@ import { consts } from './defs/constants.ts'; import { decodeMessage, encodeMessage } from './message.ts'; import { detect, encodingByDataCoding } from './defs/encodings.ts'; import { errorNameById, errors, isErrorName } from './defs/errors.ts'; +import { paramNumber } from './defs/types.ts'; import { tlvDefault, tlvs, tlvsById } from './defs/tlvs.ts'; /** Sequence numbers are a 31-bit field; 0x7fffffff is reserved. */ @@ -63,10 +64,6 @@ export function isCommand( return pduObj.cmdName === cmdName; } -function numberOr(value: ParamValue | undefined, fallback: number): number { - return typeof value === 'number' ? value : fallback; -} - function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { const tagId = input.tagId ?? tlvs[name]?.id; @@ -283,7 +280,7 @@ function readParams( let offset = 16; for (const [name, type] of Object.entries(cmds[cmdName]?.params ?? {})) { - const read = type.read(pdu, offset, numberOr(params.sm_length, 0)); + const read = type.read(pdu, offset, paramNumber(params.sm_length, 0)); if (read.err) { return { err: new Error(`Parameter "${name}" of "${cmdName}": ${read.err.message}`) }; @@ -327,11 +324,11 @@ function parseOnce(pdu: Buffer, trailingNull: boolean): Result<{ aligned: boolea const params = read.params; const message = params.short_message; - const esmClass = numberOr(params.esm_class, 0); + const esmClass = paramNumber(params.esm_class, 0); // A message carrying a UDH stays a buffer; the session needs the header intact to reassemble. if (Buffer.isBuffer(message) && (esmClass & consts.ESM_CLASS.UDH_INDICATOR) !== consts.ESM_CLASS.UDH_INDICATOR) { - params.short_message = decodeMessage(message, numberOr(params.data_coding, 0)).message; + params.short_message = decodeMessage(message, paramNumber(params.data_coding, 0)).message; } return { diff --git a/src/reassembly.ts b/src/reassembly.ts index 9766bc1..efc5e2e 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -5,7 +5,7 @@ import type { SmppLog } from './log.ts'; import type { Tlv } from './defs/tlvs.ts'; import { ExpiringGroups } from './expiring-groups.ts'; import { decodeMessage } from './message.ts'; -import { paramText } from './defs/types.ts'; +import { paramNumber, paramText } from './defs/types.ts'; export type ReassemblerOptions = { log: SmppLog; @@ -71,10 +71,6 @@ function groupKey(pduObj: PduObject, reference: number): string { ].join('_'); } -function numberOr(value: ParamValue | undefined, fallback: number): number { - return typeof value === 'number' ? value : fallback; -} - /** The text of a message, joining its segments in the order they were reassembled. */ export function decodeSegments(pduObjs: PduObject[]): string { let message = ''; @@ -85,8 +81,8 @@ export function decodeSegments(pduObjs: PduObject[]): string { message += Buffer.isBuffer(part) ? decodeMessage( part, - numberOr(pduObj.params.data_coding, 0), - numberOr(pduObj.params.esm_class, 0), + paramNumber(pduObj.params.data_coding, 0), + paramNumber(pduObj.params.esm_class, 0), ).message : paramText(part); } diff --git a/test/dlr.test.ts b/test/dlr.test.ts index 153803d..ea42eef 100644 --- a/test/dlr.test.ts +++ b/test/dlr.test.ts @@ -8,7 +8,7 @@ import type { PduObject, TlvInput } from '../src/pdu.ts'; const receiptText = 'id:0195f0c7 sub:001 dlvrd:001 submit date:2508251430 done date:2508251431 stat:DELIVRD err:000 text:hello'; function deliverSm( - message: string, + message: Buffer | string, tlvs?: Record, esmClass: number = consts.ESM_CLASS.MC_DELIVERY_RECEIPT, ): PduObject { @@ -136,10 +136,41 @@ describe('dlrFromPdu()', () => { assert.equal(dlr.smsId, undefined); assert.equal(dlr.statusMsg, 'UNKNOWN'); assert.equal(dlr.statusId, 7); + }); - const withUdh = consts.ESM_CLASS.MC_DELIVERY_RECEIPT | consts.ESM_CLASS.UDH_INDICATOR; + // pduToObj leaves a UDH-carrying short_message a buffer, so the body needs decoding before it + // can be read at all — and the message type sits under the UDH indicator in the same octet. + test('reads the body of a receipt that carries a UDH', () => { + const udh = Buffer.from([0x05, 0x00, 0x03, 0x2a, 0x01, 0x01]); + const body = Buffer.concat([udh, Buffer.from(receiptText, 'ascii')]); + const dlr = dlrFromPdu(deliverSm( + body, + undefined, + consts.ESM_CLASS.MC_DELIVERY_RECEIPT | consts.ESM_CLASS.UDH_INDICATOR, + )); - assert.ok(dlrFromPdu(deliverSm(receiptText, undefined, withUdh))); + assert.ok(dlr); + assert.equal(dlr.smsId, '0195f0c7'); + assert.equal(dlr.statusMsg, 'DELIVERED'); + }); + + // message_state 0x80-0xFF is reserved for MC-vendor-specific values, which we cannot name. + test('falls back to the body when the state TLV carries a value it cannot name', () => { + const dlr = dlrFromPdu(deliverSm(receiptText, { message_state: { tagValue: 0x84 } })); + + assert.ok(dlr); + assert.equal(dlr.statusId, 0x84); + assert.equal(dlr.statusMsg, 'DELIVERED'); + }); + + test('takes a receipted_message_id TLV as a receipt marker of its own', () => { + const dlr = dlrFromPdu(deliverSm('nothing scrapable here', { + receipted_message_id: { tagValue: 'from-the-tlv' }, + }, 0)); + + assert.ok(dlr); + assert.equal(dlr.smsId, 'from-the-tlv'); + assert.equal(dlr.statusMsg, 'UNKNOWN'); }); test('leaves a message the peer marked as another type to arrive as an SMS', () => { diff --git a/todo.md b/todo.md index 1582cc5..116ab7b 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**: 227 tests, lint and typecheck clean, verified on Node +The rewrite is **feature complete and green**: 230 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,12 @@ 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. +- [ ] **Nothing owns the `esm_class` bits.** Three modules read them with their own literals: + `pdu.ts` decides decode-or-not, `incoming-requests.ts` reassemble-or-not and `dlr.ts` + receipt-or-not. That divergence is what let a UDH-carrying receipt reach `dlrFromPdu()` as an + undecoded buffer. Two predicates next to the constants would collapse it without touching + `index.ts`. + - [ ] **`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 @@ -162,8 +168,9 @@ session message is a change to every call site. change on this list for the most real-world breakage removed. - [ ] **An `onReceipt` hook.** Receipt text is only loosely specified and operators disagree on it, - but `dlrFromPdu()` is wired into `IncomingRequests` with no way past it: an application facing - a format we do not parse has to listen on `incomingPduObj` and reimplement the dispatch. + 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 + dispatch, which owns the response as well. Mirror the `onRequest` seam — return a `Dlr` to own the receipt, `undefined` to fall through to the built-in parser.