Keep the body scrape for reserved message types and an empty receipt id marker
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
+16
-11
@@ -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);
|
||||
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user