diff --git a/AGENTS.md b/AGENTS.md index 524b2d9..b5904d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -302,6 +302,16 @@ Grouped by what each one constrains. transient rather than as saying nothing, maintainer's call, 2026-09-03, since the codec refuses a zero-length integer TLV and so an absent one cannot land there. +- **A transient state goes out as an intermediate delivery notification (0x20), every other state as + a delivery receipt (0x04).** Appendix B makes a receipt's `stat` the message's final status, so + 0x04 over `ENROUTE` emits the two disagreeing spellings of finality the reading side above has to + reconcile, and goal 3 has our own senders write the marker 3.4 defines. `sendDlr()` takes the list + from `transientStates` in `dlr.ts`, the same one the reader uses, so the two cannot drift. + Rejected: 0x04 for every state, for the sake of a peer keyed to that marker alone — such a peer + then reads a transient report as a final one, which is goal 2's wrong answer handed to the peer + instead of to the application. A 0.4.0 client is unaffected either way: its bind declares 0x00, so + it is sent no TLVs and refuses the receipt regardless. + - **`smsIdFormat` names a notation per place, and normalisation never reaches 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. `submitResp` covers the `receipted_message_id` diff --git a/README.md b/README.md index 1bf5340..fc06c9e 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,8 @@ await smpp.close(); // stop listening, then drain and close every live sess ``` `sendDlr` accepts `SCHEDULED`, `ENROUTE`, `DELIVERED`, `EXPIRED`, `DELETED`, `UNDELIVERABLE`, -`ACCEPTED`, `UNKNOWN`, `REJECTED` and `SKIPPED`. +`ACCEPTED`, `UNKNOWN`, `REJECTED` and `SKIPPED`. `SCHEDULED` and `ENROUTE` go out as intermediate +delivery notifications (`esm_class` 0x20), the rest as delivery receipts (0x04). A message whose `data_coding` says 8-bit binary arrives as Latin-1, so `Buffer.from(sms.message, 'latin1')` gives you back the original octets. @@ -429,6 +430,9 @@ have worked around any of these, remove the workaround: - LATIN1 (`data_coding` 0x03) was silently decoded as ASCII, corrupting the message. - Delivery receipt dates were a month off, and the status field read `UNDELIVERABLE` where the spec defines the 7-character `UNDELIV`. +- Every receipt went out as `esm_class` 0x04, which SMPP 3.4 defines as the report of a message's + final state. A receipt for a transient state — `sendDlr('ENROUTE')` — is now marked 0x20, the + intermediate delivery notification. - `flash: true` discarded UCS2, mangling flash messages containing non-GSM characters. - The multipart reference counter was shared by every session in the process. - `tls: true` never performed a handshake, so the connection was not actually encrypted. diff --git a/src/dlr.ts b/src/dlr.ts index ac92eac..1e855fe 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -131,7 +131,7 @@ export function parseReceipt(message: string): Receipt { type MessageType = 'intermediate' | 'other' | 'receipt' | 'unmarked'; /** SMPP 3.4 Appendix B lists every other receipt state as final. */ -const transientStates: MessageState[] = ['ENROUTE', 'SCHEDULED']; +export const transientStates: MessageState[] = ['ENROUTE', 'SCHEDULED']; /** Written by the far-end SME, not by the MC reporting on a message we submitted. */ const smeMessageTypes: number[] = [ diff --git a/src/sms.ts b/src/sms.ts index 9f5d898..533c23a 100644 --- a/src/sms.ts +++ b/src/sms.ts @@ -5,7 +5,7 @@ import type { Result, VoidResult } from './result.ts'; import type { Session } from './session.ts'; import { UnansweredError } from './unanswered-error.ts'; import { consts } from './defs/constants.ts'; -import { receiptCodes } from './dlr.ts'; +import { receiptCodes, transientStates } from './dlr.ts'; import { smppDate } from './message.ts'; import { uuidv7 } from './uuid.ts'; @@ -192,7 +192,9 @@ async function sendDlr( cmdName: 'deliver_sm', params: { destination_addr: sms.from, - esm_class: consts.ESM_CLASS.MC_DELIVERY_RECEIPT, + esm_class: transientStates.includes(status) + ? consts.ESM_CLASS.INTERMEDIATE_DELIVERY + : consts.ESM_CLASS.MC_DELIVERY_RECEIPT, short_message: receiptText(sms, smsId, status), source_addr: sms.to, }, diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index b1bfb65..9499f30 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -26,7 +26,7 @@ import { closeAfter, closeListenerAfter } from './teardown.ts'; import { consts } from '../src/defs/constants.ts'; import { errors } from '../src/defs/errors.ts'; import { objToPdu } from '../src/pdu.ts'; -import { paramText } from '../src/defs/types.ts'; +import { paramNumber, paramText } from '../src/defs/types.ts'; import { server } from '../src/server.ts'; import { silentLog } from '../src/log.ts'; import { submitSms } from '../src/send-sms.ts'; @@ -179,8 +179,12 @@ describe('merged delivery reports', () => { const merged = once(resolve => { session.on('messageDlr', resolve); }); const perSegment: boolean[] = []; + const markers: (number | undefined)[] = []; - session.on('dlr', dlr => perSegment.push(dlr.intermediate)); + session.on('dlr', (dlr, pduObj) => { + perSegment.push(dlr.intermediate); + markers.push(paramNumber(pduObj.params.esm_class, 0)); + }); const [sms] = await Promise.all([ incoming.then(async received => { @@ -205,6 +209,14 @@ describe('merged delivery reports', () => { assert.equal(report.statusMsg, 'DELIVERED'); assert.equal(report.segments.length, 3); assert.deepEqual(perSegment, [true, true, true, false, false, false]); + const notification = consts.ESM_CLASS.INTERMEDIATE_DELIVERY; + const receipt = consts.ESM_CLASS.MC_DELIVERY_RECEIPT; + + assert.deepEqual( + markers, + [notification, notification, notification, receipt, receipt, receipt], + 'a transient state goes out under the marker the spec gives it', + ); }); test('reports the worst status across the segments', async t => { diff --git a/todo.md b/todo.md index aa2abf7..7487ce6 100644 --- a/todo.md +++ b/todo.md @@ -65,6 +65,7 @@ Rules the API follows: | Every runnable README example | `test/readme.test.ts` | | Receipt-versus-message classification by `esm_class` | `test/dlr.test.ts`, `test/session.test.ts` | | An intermediate delivery notification read as a report marked `intermediate`, as is a receipt reporting `ENROUTE` or `SCHEDULED`, and never counted into a merge | `test/dlr.test.ts`, `test/session.test.ts`, `test/session-extras.test.ts` | +| A transient state sent under the marker the spec gives it, off the same list the reader uses | `test/session-extras.test.ts` | | A listener that throws, or rejects, reaching `sessionError`/`serverError` rather than the process | `test/session.test.ts`, `test/error-from.test.ts` | | Cross-checked against node-smpp both ways and over a live session | `test/interop.test.ts` | | CI on Node 18/20/22/24, Renovate, tag-triggered publish | `.github/workflows/` | @@ -132,13 +133,6 @@ session message is a change to every call site. and applies it to the other is wrong. A budget type both take would close it. Raised by review, 2026-09-01. -- [ ] **Write a transient state as an intermediate delivery notification.** `sendDlr()` marks every - receipt `esm_class` 0x04, so `sendDlr('ENROUTE')` announces a non-final state with the marker - SMPP 3.4 Appendix B reserves for the final one — the two disagreeing spellings of finality the - reading side was taught to reconcile on 2026-09-03. Decided the same day: fix it, off - `transientStates` in `dlr.ts` so the writer and the reader cannot drift, in a PR of its own. - Costs a bullet in README's "Behaviour that changed on the wire". - - [ ] **`once()` is copied into four test files, and two copies never give up.** `session-extras.test.ts` and `readme.test.ts` reject after 5000 ms; `session.test.ts` and `tls.test.ts` wait forever, so an event that never fires still hangs the run the way an