diff --git a/AGENTS.md b/AGENTS.md index e224719..53552d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,7 +96,7 @@ src/ send-sms.ts submitSms composition and the submitSmParams builder send-window.ts SendWindow: the maxOutstanding semaphore session-options.ts SessionOptions, ReconnectOptions, bind direction and the session defaults - sms-id.ts The notation a peer writes message ids in, normalised for comparison + sms-id.ts Message ids: the peer's notation, the - a segment gets, which response carries one udh.ts User data header: its length, the concatenation fields of a long SMS and their reference unanswered-error.ts UnansweredError: it went out and no answer came back uuid.ts uuidv7() — the ids the library generates for messages @@ -166,15 +166,15 @@ naming the behaviour. | Binary payloads decoded as text | `data_coding` 0x02, 0x04, 0x14 and 0xF4-0xF7 are 8-bit binary and land on the GSM 03.38 table, which rewrites every octet outside it | | Binary TLVs round-trip corrupt | `pduToObj` turns a `Buffer` TLV value into a hex string (`utils.js:307`), and `objToPdu` writes that string back as its own ASCII, so `message_payload`, `network_error_code`, `callback_num` and the rest are destroyed by any round trip | | `ESME_RINVBCASTCHANIND` typo | Defined as `0x011`, three hex digits; the spec value is `0x0112` | +| Every response carries a message id | `session.js` builds `params = {'message_id': …}` for every response it sends, `deliver_sm_resp` included; SMPP 3.4 4.6.2 makes that field unused and NULL, and Jasmin closes the connection on one | ## Multipart sends and the send window `sendSms` puts every segment of a message on the wire together instead of waiting for each response -in turn. This is not an optimisation: this library's own server holds segments until the whole -message is reassembled before it answers any of them, so sending them one-after-a-response -deadlocks. It follows that a message with more segments than `maxOutstanding` cannot be delivered to -a server that defers responses that way — real SMSCs answer each `submit_sm` immediately, so this -only bites when both ends are this library. +in turn, so a long message costs one round trip rather than one per segment. Nothing on the +receiving side forces the order either way: this library answers each inbound segment as it arrives, +so a peer that dispatches one request at a time is never left waiting on us, and a message with more +segments than `maxOutstanding` goes out a slot at a time and still completes. ## GSM 7-bit is sent unpacked @@ -448,19 +448,54 @@ Grouped by what each one constrains. reports each session's unfinished drain through `serverError`, because its own result says nothing but that the listener stopped. +- **Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the + application's own signal rather than the peer's answer.** Maintainer's call, 2026-09-06, from the + Jasmin interoperability phase: Jasmin dispatches one `submit_sm` per connector at a time and will + not send segment 2 until segment 1 is answered, so holding a group unanswered until it was whole + deadlocked every multi-segment message against a production gateway + ([interop-tests/findings/03-jasmin.md](interop-tests/findings/03-jasmin.md)). Goal 1 has the answer + a real SMSC gives — one `message_id` per `submit_sm`, immediately — so the group's id base is + generated when it opens and each segment is answered `-`, the notation `sms-id.ts` owns + and `DlrMerger` reads back. The id is therefore fixed by the first segment, which is why an `smsId` + or a refusing `status` passed to `sendResp()` on such a message is an error rather than a silent + no-op. `answeredOnArrival` is on `Sms` because nothing the application can compute says it, and the + discriminant a reader would reach for instead is wrong. A message `sendResp()` still answers itself is + untouched, and is where a caller-chosen id and a refusal live; `onRequest` is the escape hatch for + an application that must refuse a PDU the `sms` event could not have shown it yet. `collect()` + answers every segment it will not carry rather than leaving it unanswered, which is the same stall + in miniature: `ESME_RINVESMCLASS` where the UDH belongs to no group, `ESME_RMSGQFUL` where the + segment's own arrival overran the octet cap, since a peer told that still holds it. Rejected: + answering every segment but the one that completes the group, which leaves the peer holding some + segments accepted and one refused with nothing in SMPP to retract the rest, and still cannot honour + a caller's `smsId` on the segments already gone. Rejected: a hook that mints the id per segment, + which asks the application to name a message it cannot read yet — what it wants is `sms.smsId` + afterwards. Rejected: an option to keep the old behaviour, a second spelling whose only + distinguishing feature is that it deadlocks. Accepted: a group given up on — expired, evicted, or + dropped with the link — is traffic the peer will not send again, so each one reaches `sessionError` + as well as the log. Rejected there: an exported `MessageLostError` carrying the group, on the + `PduRefusedError` pattern — no `sms` ever fired for that group, so there is nothing in it the + application could act on, and goal 6 does not buy a second exported class to make a count + distinguishable. Accepted: a completing segment whose own answer the socket would not carry still + reaches the application, because the message is whole and correct and the failed answer is on + `sessionError` — a peer that re-sends after the drop is the smaller risk than dropping a message + in hand. The answer goes out before the `sms` event either way, so a listener's own receipt can + never precede the acceptance of the message it reports on. + - **The drain waits on the messages the application holds, and `sendResp()` is what says it is done with one.** Maintainer's call, 2026-09-01: waiting on the send window alone tore a server session down while the application was still answering a `submit_sm`, so the peer timed out and re-sent — the duplicate goal 2 forbids, in the direction the window already covers. No completion signal was - added to the `sms` event: `sendResp()` is the answer the peer is waiting for, so it is the one the - drain waits for. Counting every inbound request until `sendReturn()` answered it was rejected — + added to the `sms` event: `sendResp()` is what an application already calls when it is done with a + message, so it is the one the drain waits for. Counting every inbound request until `sendReturn()` answered it was rejected — an `onRequest` that deliberately answers nothing would then cost a full `shutdownTimeout` on every close — and a message no listener took is released at once, since nothing is going to answer it. A listener that failed before answering gives it up the same way, but only once every listener has: a throw stops `emit()` where it stands, while a rejection leaves the others running, so the release waits for the last of them rather than answering on their behalf. What ends the wait is the response reaching the wire, not the call — a `sendResp()` the library refused, or one the socket would not - carry, leaves the message held, so `close()` still reports the one the peer is owed. `teardown()` + carry, leaves the message held, so `close()` still reports the one the peer is owed. Where the + segments were answered as they arrived there is no response left to write, so the call itself ends + the wait, an argument the library refuses excepted. `teardown()` drops what is still held for the same reason it drops inbound segments. The release is one turn late, so a listener that sends its receipt straight after the response is still holding when the drain looks; `sendDlr()` is the one send that goes out past the drain's refusal, and only while the @@ -482,9 +517,11 @@ Grouped by what each one constrains. `onDeliverSm()` answers each receipt before the group it belongs to is complete, and `teardown()` runs on every path — an idle timeout and a failed rebind, not only `close()` — so clearing the merges there loses receipts no peer has a reason to send again. They are cleared where the session - is over instead. Inbound segments stay in `teardown()`, because they go unanswered until the - message is whole: the peer still holds them, and answering it on a later link with the old - segments' sequence numbers would correlate with nothing. + is over instead. Inbound segments stay in `teardown()`: an 8-bit concatenation reference is the + peer's own counter, so a half-arrived group kept across a drop would take a later message's + segments as readily as the rest of its own, and goal 2 will not hand the application a message + assembled that way. What goes there is traffic already answered, which is why each group reaches + `sessionError` like every other one given up on. - **A message id base is merged at most once.** A receipt carries nothing but `-`, so a straggler for a message whose group is gone cannot be told from a receipt for a later message the diff --git a/README.md b/README.md index ddbc68e..0a92898 100644 --- a/README.md +++ b/README.md @@ -199,9 +199,10 @@ if (err) throw err; smpp.on('session', session => { session.on('sms', async sms => { // Responding is part of the protocol, not optional. Without arguments it answers - // ESME_ROK with a generated id; pass your own, and a status to refuse the message. - await sms.sendResp(); + // ESME_ROK with a generated id. On a message sendResp() still answers itself — see + // below — you may pass your own id, and a status to refuse the message: // await sms.sendResp({ smsId: yourOwnId, status: 'ESME_RMSGQFUL' }); + await sms.sendResp(); if (sms.dlr) { await sms.sendDlr(); // same as sms.sendDlr('DELIVERED') @@ -213,6 +214,18 @@ console.log(smpp.port); // the port actually bound, useful when 0 was requested await smpp.close(); // stop listening, then drain and close every live session ``` +A message that arrived in several segments was already answered when you see it: each segment is +answered as it lands, because a relaying SMSC will not send the next one until the last is answered. +That answer is `ESME_ROK` unless the segment's header names no message this session can join, which +refuses it, or the reassembly buffer is full, which asks the SMSC to keep it and try again. +`sms.answeredOnArrival` says whether the message you are holding was answered that way — a segment count cannot, since a peer +may number a concatenated message one part of one. The id was fixed with the first segment, so +`sendResp()` there only says you are done with the message, and returns an `err` for an `smsId` or a +refusing `status`; choosing the id and refusing the message belong to a message `sendResp()` still +answers itself. `sms.smsId` is the base either way, and `sendDlr()` names `-1`, `-2` +and so on — the ids a `submit_sm`'s responses carried. A `deliver_sm` is answered with no id at all, +since SMPP marks that field unused, so an inbound message's base is a handle of your own only. + `sendDlr` accepts `SCHEDULED`, `ENROUTE`, `DELIVERED`, `EXPIRED`, `DELETED`, `UNDELIVERABLE`, `ACCEPTED`, `UNKNOWN`, `REJECTED` and `SKIPPED`. `SCHEDULED` and `ENROUTE` go out as intermediate delivery notifications (`esm_class` 0x20), the rest as delivery receipts (0x04). @@ -265,10 +278,14 @@ Runtime failures on a live connection arrive as `sessionError` and `serverError` deliberately not called `error`: Node turns an unhandled `error` event into a thrown exception, which is exactly what this library promises not to do. -`sessionError` carries two kinds of failure, and `PduRefusedError` is what separates them: +`sessionError` carries three kinds of failure, and `PduRefusedError` separates the first from the +rest: - **A PDU the peer sent that the codec could not read with the stream still in sync.** The link is healthy and only that one PDU is lost, so this is the kind to count rather than alert on. +- **A concatenated message given up on before it was whole.** Its arrived segments were answered, so + the peer will not send them again. Also a counting kind: no `sms` event ever fired for it, so + there is nothing to act on beyond knowing traffic was lost. - **Everything else**: the session or the socket failing, and a hook or listener that threw or, if it was `async`, rejected. @@ -335,13 +352,13 @@ 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 its `smsId`. | +| `sms` | An SMS arrives, reassembled if it was multipart. Carries `sendResp()`, `sendDlr()` and its `smsId`. A multipart one was answered as its segments arrived — see [Server](#server). | | `dlr` | A delivery report arrives, one per segment. `intermediate` is true where the report is not final: the SMSC either marked it an intermediate notification, or reported `ENROUTE` or `SCHEDULED`. `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. A report carrying `intermediate` never counts towards it. 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. A base is merged once: a later message the SMSC gives the same ids is reported on through `dlr` alone, and an earlier one still collecting loses its merged report as well. | | `close` | The session is over, because nothing will bring the link back. Fires once, whether you closed it or the link failed for good. | | `disconnected` | The link dropped and the reconnect loop will retry it. Do not open a replacement client here — the session you hold comes back on its own, and `reconnected` says when. Fires again for each attempt that reconnects and then fails, so it is not one-to-one with `reconnected`. | | `reconnected` | The client re-bound after a drop. | -| `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. Fires for each PDU the codec refused as well, carrying a `PduRefusedError` while the link carries on: a refused request is answered with the status SMPP names, and a refused response is answered with nothing and settles the request it named as `unanswered`. [Errors](#errors) tells the two kinds apart. | +| `sessionError` | Something failed on a live session, including a hook or listener that threw or, if it was `async`, rejected. Fires for each PDU the codec refused as well, carrying a `PduRefusedError` while the link carries on: a refused request is answered with the status SMPP names, and a refused response is answered with nothing and settles the request it named as `unanswered`. [Errors](#errors) tells its three kinds apart. | | `data` | Raw bytes arrived on the socket. | | `incomingPdu` | A complete PDU arrived, as a buffer. | | `incomingPduObj` | The same PDU, parsed into an object. | @@ -353,7 +370,8 @@ refuse further sends, wait out the requests this end already sent for up to `shu then tear down whatever is left, resolving to an `err` that says what was lost. They also wait for every `sms` still in the application's hands, so a peer whose `submit_sm` is being handled is answered rather than left to re-send it. That wait ends when `sendResp()` puts the response on the -wire, or when every listener that took the message has failed; answering its PDUs through +wire — or, for a message whose segments were answered as they arrived, when it is called at all — +or when every listener that took the message has failed; answering its PDUs through `sendReturn()` instead leaves the wait running until it gives up. `sendDlr()` is the one send the refusal lets past, and it catches the wait when issued straight after `sendResp()`; await anything in between and it races the shutdown like any other send. `close({ signal })` takes an `AbortSignal` @@ -445,8 +463,8 @@ promises and the rough edges taken off. - **`server()` resolves once, when it is listening**, and gives you a handle with `close()`, `port` and a `session` event. It no longer calls your callback once per incoming connection. - **The id a message is answered with goes to `sendResp({ smsId })`**, and `sms.smsId` is read-only: - it reports the id `sendResp()` was given, or the UUID v7 generated instead. Delete any - `sms.smsId = …` line — assigning to it + it reports the id the segments were answered with, the id `sendResp()` was given, or the UUID v7 + generated instead. Delete any `sms.smsId = …` line — assigning to it throws a `TypeError`, since modules are always strict mode — and pass the id to `sendResp()`. - **`checkuserpass` is now `authenticate`**, takes `{ password, session, systemId, systemType }` and returns `false` or `{ userData }`. @@ -485,6 +503,9 @@ have worked around any of these, remove the workaround: - A message whose last octet was `0x00` was allocated one octet short while `sm_length` still reported the full length, so it went out corrupt. In UCS2 that is any message ending in a character like 一 (U+4E00), which made the bug routine for CJK text. +- Every response carried a `message_id`, `deliver_sm_resp` included, where SMPP 3.4 4.6.2 makes + that field unused and NULL. Jasmin closes the connection on one. Answering an inbound message now + puts nothing in it, and `sms.smsId` is the local handle it always was. - Binary TLVs (`message_payload`, `network_error_code`, `callback_num` and the rest) were parsed into a hex string and written back as the ASCII of that string, so every one that made a round trip went out corrupt. They are `Buffer`s in both directions now, so drop any hex encoding of diff --git a/interop-tests/findings/03-jasmin.md b/interop-tests/findings/03-jasmin.md index 73d01f6..cad8a03 100644 --- a/interop-tests/findings/03-jasmin.md +++ b/interop-tests/findings/03-jasmin.md @@ -165,6 +165,10 @@ segment as they arrive rather than being a relay itself. Worth a decision record each segment as it's held, not only once the group completes; or document that a `server()` sitting behind a serializing relay needs its own segment-level ack), but not fixed here per the phase rules. +**Fixed** in [#83](https://github.com/larvit/larvitsmpp/pull/83): every segment is answered as it +arrives, with `-` off an id the group is opened with. All four multi-segment cases pass, in +200-360 ms each, malformed 0 and expert errors 0. + ## Peer quirks - **Jasmin's own `enquireLinkTimerSecs` (30, `[smpp-server]` default) is an idle timer, not a strict @@ -175,7 +179,7 @@ behind a serializing relay needs its own segment-level ack), but not fixed here Every id the real client's `submit_sm_resp` carried matched exactly what the fake upstream's `sendResp()` assigned; the later receipt named the same id. Since the fake upstream here is this library's own `server()`, a multi-segment message's ids came back in this library's own - `-` shape - an artifact of the test rig (our own server reassembling before answering), + `-` shape - an artifact of the test rig (our own server minting one base per group), not evidence that Jasmin itself produces that convention; a real upstream SMSC would very likely hand back unrelated ids per segment, as the research notes expected. - **`smpps_throughput`, not the connector's `submit_throughput`, gates an ESME's own submission @@ -194,6 +198,11 @@ behind a serializing relay needs its own segment-level ack), but not fixed here printf-style placeholders, nothing is written into the URL by the caller. `dlr-level=1` fires once, immediately, with `message_status=ESME_ROK` (an SMSC-ack, not a terminal state); `dlr-level=3` additionally fires once more, later, with the real terminal status (`DELIVRD` here). +- **Jasmin FINs the connection on a `deliver_sm_resp` carrying a `message_id`.** SMPP 3.4 4.6.2 + makes that field unused and NULL, and Jasmin's own decoder sizes it at one octet; a 38-octet UUID + in it cost the link immediately after the response, taking the rest of the MO group with it. Found + by the fix for the multipart deadlock above, which is what first had this library answer an inbound + `deliver_sm` in this suite at all; the field now goes out empty. - **jcli is a plain-text protocol dressed as Telnet** - it sends real `IAC`/option-negotiation bytes and a couple of ANSI escapes in its banner, but never waits for or requires a reply to them; a raw socket client that ignores negotiation entirely and just reads/writes lines works throughout. diff --git a/src/dlr-merger.ts b/src/dlr-merger.ts index 4dfd9b7..2970b0a 100644 --- a/src/dlr-merger.ts +++ b/src/dlr-merger.ts @@ -2,6 +2,7 @@ import type { Dlr } from './dlr.ts'; import type { MessageState } from './defs/constants.ts'; import type { SmppLog } from './log.ts'; import { ExpiringGroups } from './expiring-groups.ts'; +import { parseSegmentId } from './sms-id.ts'; export type MessageDlr = Dlr & { segments: Dlr[]; smsId: string }; @@ -18,8 +19,6 @@ type Group = { parts: Map; }; -const numbered = /^(.*)-(\d+)$/; - /** * MESSAGE_STATE is a flat enum, not a ranking — ACCEPTED is 6 where UNDELIVERABLE is 5 — so reducing * on the wire value reports a part-failed message as delivered. Rank it deliberately instead. @@ -43,14 +42,12 @@ function idNumbering(smsIds: string[]): { base: string; parts: Set } | u const parts = new Set(); for (const smsId of smsIds) { - const match = numbered.exec(smsId); - const base = match?.[1]; - const part = match?.[2]; + const numbering = parseSegmentId(smsId); - if (base === undefined || part === undefined) return undefined; + if (!numbering) return undefined; - bases.add(base); - parts.add(Number(part)); + bases.add(numbering.base); + parts.add(numbering.part); } const [base] = bases; @@ -110,18 +107,15 @@ export class DlrMerger { if (dlr.intermediate || dlr.smsId === undefined) return undefined; - const match = numbered.exec(dlr.smsId); - const base = match?.[1]; - const part = match?.[2]; + const numbering = parseSegmentId(dlr.smsId); - if (base === undefined || part === undefined) return undefined; + if (!numbering) return undefined; + const { base, part: number } = numbering; const group = this.groups.get(base); if (!group) return undefined; - const number = Number(part); - if (!group.expected.has(number)) return undefined; group.parts.set(number, dlr); @@ -137,8 +131,8 @@ export class DlrMerger { } clear(): void { - this.groups.clear(); - this.spent.clear(); + this.groups.takeAll(); + this.spent.takeAll(); } /** Drops every group past its deadline. Runs before each collect and on its own timer. */ diff --git a/src/expiring-groups.ts b/src/expiring-groups.ts index 584a1a2..9c154cb 100644 --- a/src/expiring-groups.ts +++ b/src/expiring-groups.ts @@ -58,9 +58,18 @@ export class ExpiringGroups { this.idle(); } - clear(): void { + /** Removes every group and hands them over, so an owner that must account for them can. */ + takeAll(): [string, T][] { + const taken: [string, T][] = []; + + for (const [key, entry] of this.entries) { + taken.push([key, entry.group]); + } + this.entries.clear(); this.idle(); + + return taken; } /** Removes every group past its deadline and hands them over. */ diff --git a/src/held-messages.ts b/src/held-messages.ts index 0a2a665..6a317fd 100644 --- a/src/held-messages.ts +++ b/src/held-messages.ts @@ -76,7 +76,7 @@ export class HeldMessages { /** Drops every message: their segments went with the link, so no answer of ours correlates now. */ clear(): void { - this.held.clear(); + this.held.takeAll(); this.idleWaiters.settle(); } diff --git a/src/incoming-requests.ts b/src/incoming-requests.ts index e4208a2..0bf74cb 100644 --- a/src/incoming-requests.ts +++ b/src/incoming-requests.ts @@ -1,4 +1,7 @@ +import type { CommandName } from './defs/commands.ts'; import type { DlrMerger } from './dlr-merger.ts'; +import type { ErrorName } from './defs/errors.ts'; +import type { LostGroup, Refusal } from './reassembly.ts'; import type { OnRequest } from './session-options.ts'; import type { PduObject, PduObjectInput } from './pdu.ts'; import type { Result, VoidResult } from './result.ts'; @@ -13,6 +16,20 @@ import { hasUdh } from './defs/constants.ts'; import { createSms } from './sms.ts'; import { dlrFromPdu } from './dlr.ts'; import { paramNumber, paramText } from './defs/types.ts'; +import { respIdParams, segmentId } from './sms-id.ts'; + +/** SMPP 3.4 lists ESME_RMSGQFUL under submit_sm_resp only; 4.6.2's retryable code is another. */ +export function refusedSegmentStatus(cmdName: CommandName, refusal: Refusal): ErrorName { + if (refusal === 'unplaceable') return 'ESME_RINVESMCLASS'; + + return cmdName === 'deliver_sm' ? 'ESME_RX_T_APPN' : 'ESME_RMSGQFUL'; +} + +const lostReasons: Record = { + evicted: 'the reassembly buffer filled', + expired: 'no further segment arrived in time', + linkGone: 'the link they arrived on went', +}; export type IncomingRequestsOptions = { dlrMerger: DlrMerger; @@ -56,6 +73,7 @@ export class IncomingRequests { log: options.log, max: options.maxReassembly ?? defaults.maxReassembly, maxOctets: options.maxOctets, + onLost: lost => { this.reportLost(lost); }, timeout: options.reassemblyTimeout ?? defaults.reassemblyTimeout, }); this.sendPastDrain = options.sendPastDrain; @@ -94,7 +112,7 @@ export class IncomingRequests { await this.session.sendReturn(pduObj); break; case 'submit_sm': - this.onMessage(pduObj); + await this.onMessage(pduObj); break; case 'unbind': await this.session.sendReturn(pduObj); @@ -148,7 +166,7 @@ export class IncomingRequests { const dlr = dlrFromPdu(pduObj, this.smsIdFormat); if (!dlr) { - this.onMessage(pduObj); + await this.onMessage(pduObj); return; } @@ -162,7 +180,11 @@ export class IncomingRequests { await this.session.sendReturn(pduObj); } - private onMessage(pduObj: PduObject): void { + /** + * A concatenated message is answered segment by segment as it arrives: a peer that dispatches + * one request at a time never sends the second segment until the first has been answered. + */ + private async onMessage(pduObj: PduObject): Promise { const message = pduObj.params.short_message; const carriesUdh = hasUdh(paramNumber(pduObj.params.esm_class, 0)); const concat = carriesUdh && Buffer.isBuffer(message) ? concatInfo(message) : undefined; @@ -173,12 +195,30 @@ export class IncomingRequests { return; } - const whole = this.reassembler.collect(pduObj, concat); + const collected = this.reassembler.collect(pduObj, concat); - if (whole) this.emitSms(whole); + if (!collected.kept) { + await this.session.sendReturn(pduObj, refusedSegmentStatus(pduObj.cmdName, collected.refusal)); + + return; + } + + await this.session.sendReturn( + pduObj, + 'ESME_ROK', + respIdParams(pduObj.cmdName, segmentId(collected.smsId, concat.part - 1, concat.total)), + ); + + if (collected.whole) this.emitSms(collected.whole, collected.smsId); } - private emitSms(pduObjs: PduObject[]): void { + private reportLost(lost: LostGroup): void { + this.session.emit('sessionError', new Error( + `Gave up ${String(lost.parts)} of ${String(lost.total)} segments of an incomplete concatenated message: ${lostReasons[lost.reason]}`, + )); + } + + private emitSms(pduObjs: PduObject[], answeredAs?: string): void { const first = pduObjs[0]; if (!first) return; @@ -188,6 +228,7 @@ export class IncomingRequests { const release = (): void => { setImmediate(() => { this.held.release(pduObjs); }); }; const sms = createSms({ + answeredAs, from: paramText(first.params.source_addr), message: decodeSegments(pduObjs), pduObjs, diff --git a/src/reassembly.ts b/src/reassembly.ts index c7b390d..30d849e 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -6,21 +6,48 @@ import type { Tlv } from './defs/tlvs.ts'; import { ExpiringGroups } from './expiring-groups.ts'; import { decodeMessage } from './message.ts'; import { paramNumber, paramText } from './defs/types.ts'; +import { uuidv7 } from './uuid.ts'; + +/** A concatenated message given up on, whose segments the peer has already been answered for. */ +export type LostGroup = { + parts: number; + reason: 'evicted' | 'expired' | 'linkGone'; + smsId: string; + total: number; +}; export type ReassemblerOptions = { log: SmppLog; max: number; maxOctets?: number | undefined; + /** Injected so the ids a group is answered with can be read back in a test. */ + newId?: (() => string) | undefined; /** Injected so expiry can be exercised without a wall clock. */ now?: (() => number) | undefined; + onLost: (lost: LostGroup) => void; timeout: number; }; +/** Why a segment was not kept: its header joins no message here, or the store had no room for it. */ +export type Refusal = 'full' | 'unplaceable'; + +/** What a segment did to its group. */ +export type Collected = + | { kept: false; refusal: Refusal } + | { + kept: true; + /** The id base the group's segments are answered with. */ + smsId: string; + /** Every segment in order, on the one that completes the message. */ + whole?: PduObject[] | undefined; + }; + const defaultMaxOctets = 64 * 1024 * 1024; type Group = { octets: number; parts: Map; + smsId: string; total: number; }; @@ -101,6 +128,8 @@ export class Reassembler { private readonly log: SmppLog; private readonly max: number; private readonly maxOctets: number; + private readonly newId: () => string; + private readonly onLost: (lost: LostGroup) => void; private octets = 0; constructor(options: ReassemblerOptions) { @@ -113,38 +142,22 @@ export class Reassembler { this.log = options.log; this.max = options.max; this.maxOctets = options.maxOctets ?? defaultMaxOctets; + this.newId = options.newId ?? uuidv7; + this.onLost = options.onLost; } get size(): number { return this.groups.size; } - /** Every segment in order, on the one that completes the message; nothing while it is short. */ - collect(pduObj: PduObject, concat: ConcatInfo): PduObject[] | undefined { + /** The group the segment joined, and always an answer for it: an unanswered one stalls a peer. */ + collect(pduObj: PduObject, concat: ConcatInfo): Collected { this.sweep(); - if (concat.part < 1 || concat.total < 1 || concat.part > concat.total) { - this.log.warn('reassembler - dropping a segment the UDH numbers impossibly', { - part: concat.part, - total: concat.total, - }); - - return undefined; - } - const key = groupKey(pduObj, concat.reference); const existing = this.groups.get(key); - // Parts 1/2 and 2/3 would otherwise complete the stored two-part group as a truncated message. - if (existing && existing.total !== concat.total) { - this.log.warn('reassembler - dropping a segment with an inconsistent UDH total', { - existingTotal: existing.total, - part: concat.part, - total: concat.total, - }); - - return undefined; - } + if (!this.placeable(concat, existing)) return { kept: false, refusal: 'unplaceable' }; const group = existing ?? this.open(key, concat.total); const replaced = group.parts.get(concat.part); @@ -156,34 +169,69 @@ export class Reassembler { this.octets += delta; if (group.parts.size < group.total) { - this.trim(); + this.trim(key); - return undefined; + // Its own arrival overran the octet cap, so the peer keeps it rather than being told we did. + if (this.groups.get(key) !== group) return { kept: false, refusal: 'full' }; + + return { kept: true, smsId: group.smsId }; } this.groups.delete(key); this.octets -= group.octets; - return [...group.parts.entries()].sort(([a], [b]) => a - b).map(([, part]) => part); + return { + kept: true, + smsId: group.smsId, + whole: [...group.parts.entries()].sort(([a], [b]) => a - b).map(([, part]) => part), + }; } clear(): void { - this.groups.clear(); + for (const [, group] of this.groups.takeAll()) { + this.lost(group, 'linkGone'); + } + this.octets = 0; } /** Drops every group past its deadline. Runs before each collect and on its own timer. */ sweep(): void { - for (const [key, group] of this.groups.takeExpired()) { - this.log.info('reassembler - incomplete message expired', { key, total: group.total }); + for (const [, group] of this.groups.takeExpired()) { this.octets -= group.octets; + this.lost(group, 'expired'); } } + /** Whether a segment's UDH can join a group at all: its own numbering, and the group's total. */ + private placeable(concat: ConcatInfo, existing: Group | undefined): boolean { + if (concat.part < 1 || concat.total < 1 || concat.part > concat.total) { + this.log.warn('reassembler - dropping a segment the UDH numbers impossibly', { + part: concat.part, + total: concat.total, + }); + + return false; + } + + // Parts 1/2 and 2/3 would otherwise complete the stored two-part group as a truncated message. + if (existing && existing.total !== concat.total) { + this.log.warn('reassembler - dropping a segment with an inconsistent UDH total', { + existingTotal: existing.total, + part: concat.part, + total: concat.total, + }); + + return false; + } + + return true; + } + private open(key: string, total: number): Group { if (this.groups.full) this.dropOldest(); - const group: Group = { octets: 0, parts: new Map(), total }; + const group: Group = { octets: 0, parts: new Map(), smsId: this.newId(), total }; this.groups.set(key, group); @@ -191,24 +239,43 @@ export class Reassembler { } /** Drops the oldest groups until the retained payload is back under the octet cap. */ - private trim(): void { - while (this.octets > this.maxOctets && this.groups.size > 0) { - this.dropOldest(); + private trim(current: string): void { + while (this.octets > this.maxOctets) { + const oldest = this.takeOldest(); + + if (!oldest) return; + + // The refused segment is in the group but stays with the peer, so it is none of the loss. + const answered = oldest[0] === current ? oldest[1].parts.size - 1 : oldest[1].parts.size; + + if (answered > 0) this.lost(oldest[1], 'evicted', answered); } } - private dropOldest(): void { + private takeOldest(): [string, Group] | undefined { const oldest = this.groups.takeOldest(); - if (!oldest) return; + if (oldest) this.octets -= oldest[1].octets; - const [, group] = oldest; + return oldest; + } - this.log.warn('reassembler - buffer full, dropping the oldest message', { + private dropOldest(): void { + const oldest = this.takeOldest(); + + if (oldest) this.lost(oldest[1], 'evicted'); + } + + /** Its segments are answered, so the peer will not send them again: this is traffic gone. */ + private lost(group: Group, reason: LostGroup['reason'], parts = group.parts.size): void { + const lost: LostGroup = { parts, reason, smsId: group.smsId, total: group.total }; + + this.log.warn('reassembler - gave up a concatenated message', { + ...lost, max: this.max, maxOctets: this.maxOctets, octets: this.octets, }); - this.octets -= group.octets; + this.onLost(lost); } } diff --git a/src/session.ts b/src/session.ts index 4d6a5b4..4ba8ba6 100644 --- a/src/session.ts +++ b/src/session.ts @@ -359,12 +359,16 @@ export class Session extends EventEmitter { if (this.closed) return; this.closed = true; - this.outgoing.linkLost(this.retrying()); + + // Read once: clear() reports lost segments, and a listener could stop the loop between reads. + const retrying = this.retrying(); + + this.outgoing.linkLost(retrying); this.timers.clear(); this.incoming.clear(); this.sock.destroy(); - if (this.retrying()) this.emit('disconnected'); + if (retrying) this.emit('disconnected'); else this.emitClose(); } diff --git a/src/sms-id.ts b/src/sms-id.ts index 27146ff..8a558b6 100644 --- a/src/sms-id.ts +++ b/src/sms-id.ts @@ -1,3 +1,6 @@ +import type { CommandName } from './defs/commands.ts'; +import type { ParamValue } from './defs/types.ts'; + const notations = { decimal: { digits: /^[0-9]+$/, prefix: '' }, hex: { digits: /^[0-9a-f]+$/i, prefix: '0x' }, @@ -33,3 +36,26 @@ export function normaliseSmsId(id: string, notation: SmsIdNotation | undefined): return digits.test(id) ? BigInt(`${prefix}${id}`).toString(10) : id; } + +const numbered = /^(.*)-(\d+)$/; + +/** Each segment of a multipart message gets its own message_id, as a separate submit_sm must. */ +export function segmentId(smsId: string, index: number, total: number): string { + return total === 1 ? smsId : `${smsId}-${String(index + 1)}`; +} + +/** The message and the part an id names, or nothing where `segmentId()` did not write it. */ +export function parseSegmentId(smsId: string): { base: string; part: number } | undefined { + const match = numbered.exec(smsId); + const base = match?.[1]; + const part = match?.[2]; + + if (base === undefined || part === undefined) return undefined; + + return { base, part: Number(part) }; +} + +/** SMPP 3.4 4.6.2 makes `deliver_sm_resp`'s `message_id` unused, and Jasmin FINs the link over one. */ +export function respIdParams(cmdName: CommandName, smsId: string): Record { + return cmdName === 'deliver_sm' ? {} : { message_id: smsId }; +} diff --git a/src/sms.ts b/src/sms.ts index 4f4d655..e83f036 100644 --- a/src/sms.ts +++ b/src/sms.ts @@ -7,6 +7,7 @@ import { UnansweredError } from './unanswered-error.ts'; import { consts } from './defs/constants.ts'; import { receiptCodes, transientStates } from './dlr.ts'; import { smppDate } from './message.ts'; +import { respIdParams, segmentId } from './sms-id.ts'; import { uuidv7 } from './uuid.ts'; /** `pduObjs` holds what the peer took, so a partial failure names what is already receipted. */ @@ -28,6 +29,11 @@ export type SendRespOptions = { * every segment's PDU. */ export type Sms = { + /** + * Whether the peer was answered as the message's segments arrived, which is what a concatenated + * message needs and a segment count cannot tell you. `sendResp()` then writes nothing. + */ + answeredOnArrival: boolean; dlr: boolean; flash: boolean; from: string; @@ -35,16 +41,22 @@ export type Sms = { pduObjs: PduObject[]; /** Sends a delivery report back to the sender. Defaults to DELIVERED. */ sendDlr: (status?: MessageState) => Promise; - /** Answers every segment. Part of the protocol, not optional. Defaults to ESME_ROK. */ + /** + * Answers the message, and says the application is done with it. A concatenated message was + * answered segment by segment as it arrived, so there it only releases a shutdown's wait and + * refuses an `smsId` or a refusing `status`. Part of the protocol, not optional. + */ sendResp: (options?: SendRespOptions) => Promise; session: Session; - /** The id `sendResp()` was given, or a generated UUID v7. */ + /** The id the segments were answered with, the id `sendResp()` was given, or a generated UUID v7. */ readonly smsId: string; submitTime: Date; to: string; }; export type SmsInput = { + /** The id base the segments were already answered with; absent leaves the answer to `sendResp()`. */ + answeredAs?: string | undefined; from: string; message: string; pduObjs: PduObject[]; @@ -59,25 +71,23 @@ export type SmsHandlers = { send: (input: PduObjectInput) => Promise>; }; -/** Each segment of a multipart message gets its own message_id, as a separate submit_sm must. */ -function segmentId(smsId: string, index: number, total: number): string { - return total === 1 ? smsId : `${smsId}-${String(index + 1)}`; -} - export function createSms(input: SmsInput, handlers: SmsHandlers): Sms { const first = input.pduObjs[0]; const registered = first?.params.registered_delivery; const dataCoding = first?.params.data_coding; - const answered = { smsId: uuidv7() }; + const answered = { smsId: input.answeredAs ?? uuidv7() }; const sms: Sms = { + answeredOnArrival: input.answeredAs !== undefined, dlr: typeof registered === 'number' && registered !== 0, flash: typeof dataCoding === 'number' && (dataCoding & 0xF0) === 0x10, from: input.from, message: input.message, pduObjs: input.pduObjs, sendDlr: status => sendDlr(sms, handlers.send, status), - sendResp: options => sendResp(sms, answered, options ?? {}, handlers), + sendResp: options => (input.answeredAs === undefined + ? sendResp(sms, answered, options ?? {}, handlers) + : alreadyAnswered(options ?? {}, handlers)), session: input.session, get smsId(): string { return answered.smsId; @@ -89,6 +99,28 @@ export function createSms(input: SmsInput, handlers: SmsHandlers): Sms { return sms; } +/** Every segment went out answered, so the call is what the shutdown waits for and nothing else. */ +function alreadyAnswered( + options: SendRespOptions, + handlers: Pick, +): Promise { + if (options.smsId !== undefined) { + return Promise.resolve({ + err: new Error('This message\'s id was fixed when its first segment arrived; read sms.smsId'), + }); + } + + if (options.status !== undefined && options.status !== 'ESME_ROK') { + return Promise.resolve({ + err: new Error('Its segments were answered as they arrived, so there is nothing left to refuse; refuse a segment from onRequest instead'), + }); + } + + handlers.onAnswered(); + + return Promise.resolve({}); +} + async function sendResp( sms: Sms, answered: { smsId: string }, @@ -115,7 +147,7 @@ async function sendResp( const results = await Promise.all(sms.pduObjs.map((pduObj, index) => sms.session.sendReturn( pduObj, options.status ?? 'ESME_ROK', - { message_id: segmentId(answered.smsId, index, total) }, + respIdParams(pduObj.cmdName, segmentId(answered.smsId, index, total)), ))); const failure = results.find(result => result.err); diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 810eb7b..c6c2992 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import net from 'node:net'; import test, { describe } from 'node:test'; +import type { Collected, LostGroup } from '../src/reassembly.ts'; import type { Dlr } from '../src/dlr.ts'; import type { ErrorName } from '../src/defs/errors.ts'; import type { MessageState } from '../src/defs/constants.ts'; @@ -13,7 +14,7 @@ import type { Sms } from '../src/sms.ts'; import type { SmppServer } from '../src/server.ts'; import type { TestContext } from 'node:test'; import { HeldMessages } from '../src/held-messages.ts'; -import { IncomingRequests } from '../src/incoming-requests.ts'; +import { IncomingRequests, refusedSegmentStatus } from '../src/incoming-requests.ts'; import { UnansweredError } from '../src/unanswered-error.ts'; import { createSms } from '../src/sms.ts'; import { LinkGate } from '../src/link-gate.ts'; @@ -30,7 +31,8 @@ import { errors } from '../src/defs/errors.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'; +import { splitMessage } from '../src/message.ts'; +import { submitSms, submitSmParams } from '../src/send-sms.ts'; async function startServer( t: TestContext, @@ -153,7 +155,7 @@ describe('merged delivery reports', () => { const [sms] = await Promise.all([ incoming.then(async received => { - await received.sendResp({ smsId: 'merge-me' }); + await received.sendResp(); return received; }), @@ -169,10 +171,10 @@ describe('merged delivery reports', () => { const report = await merged; - assert.equal(report.smsId, 'merge-me'); + assert.equal(report.smsId, sms.smsId); assert.equal(report.segments.length, 3); assert.equal(report.statusMsg, 'DELIVERED'); - assert.deepEqual(perSegment, ['merge-me-1', 'merge-me-2', 'merge-me-3']); + assert.deepEqual(perSegment, [1, 2, 3].map(part => `${sms.smsId}-${String(part)}`)); }); test('reports once, on the final receipts, when the peer reports en route first', async t => { @@ -195,7 +197,7 @@ describe('merged delivery reports', () => { const [sms] = await Promise.all([ incoming.then(async received => { - await received.sendResp({ smsId: 'en-route' }); + await received.sendResp(); return received; }), @@ -212,7 +214,7 @@ describe('merged delivery reports', () => { const report = await merged; - assert.equal(report.smsId, 'en-route'); + assert.equal(report.smsId, sms.smsId); assert.equal(report.statusMsg, 'DELIVERED'); assert.equal(report.segments.length, 3); const notification = consts.ESM_CLASS.INTERMEDIATE_DELIVERY; @@ -244,7 +246,7 @@ describe('merged delivery reports', () => { const [sms] = await Promise.all([ incoming.then(async received => { - await received.sendResp({ smsId: 'partly-failed' }); + await received.sendResp(); return received; }), @@ -604,11 +606,12 @@ describe('reconnect', () => { test('merges the receipts of a multipart message across a drop', async t => { const smpp = await startServer(t); - - smpp.on('session', bound => { - bound.on('sms', sms => { void sms.sendResp({ smsId: 'across-the-drop' }); }); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', sms => { + resolve(sms); + void sms.sendResp(); + })); }); - const { session } = await connect(t, smpp, { reconnect: { maxDelay: 100, minDelay: 20 } }); assert.ok(session); @@ -619,11 +622,12 @@ describe('reconnect', () => { message: 'x'.repeat(400), to: '46709771337', }); + const sms = await incoming; - assert.deepEqual(sent.smsIds, ['across-the-drop-1', 'across-the-drop-2', 'across-the-drop-3']); + assert.deepEqual(sent.smsIds, [1, 2, 3].map(part => `${sms.smsId}-${String(part)}`)); // Answered on the link that then drops, so an SMSC has no reason to ever send it again. - await sendReceipt(peerOf(smpp), 'across-the-drop-1'); + await sendReceipt(peerOf(smpp), `${sms.smsId}-1`); const reconnected = once(resolve => { session.on('reconnected', () => { resolve(true); }); }); @@ -632,12 +636,12 @@ describe('reconnect', () => { const merged = once(resolve => { session.on('messageDlr', resolve); }); - await sendReceipt(peerOf(smpp), 'across-the-drop-2'); - await sendReceipt(peerOf(smpp), 'across-the-drop-3'); + await sendReceipt(peerOf(smpp), `${sms.smsId}-2`); + await sendReceipt(peerOf(smpp), `${sms.smsId}-3`); const report = await merged; - assert.equal(report.smsId, 'across-the-drop'); + assert.equal(report.smsId, sms.smsId); assert.equal(report.segments.length, 3); }); @@ -1450,21 +1454,110 @@ describe('reassembly bounds', () => { reference: number, part: number, total: number, - ): PduObject[] | undefined { + ): Collected { return reassembler.collect(segment(reference, part, total), { part, reference, total }); } test('hands back every segment in order once the last one arrives', () => { - const reassembler = new Reassembler({ log: silentLog, max: 10, now: () => 0, timeout: 60_000 }); + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + now: () => 0, + onLost: () => undefined, + timeout: 60_000, + }); + const second = collect(reassembler, 4, 2, 3); + const third = collect(reassembler, 4, 3, 3); - assert.equal(collect(reassembler, 4, 2, 3), undefined); - assert.equal(collect(reassembler, 4, 3, 3), undefined); + assert.ok(second.kept); + assert.ok(third.kept); + assert.equal(second.whole, undefined); + assert.equal(third.whole, undefined); - const whole = collect(reassembler, 4, 1, 3); + const collected = collect(reassembler, 4, 1, 3); - assert.ok(whole); - assert.deepEqual(whole.map(pduObj => pduObj.seqNr), [1, 2, 3]); + assert.ok(collected.kept); + assert.ok(collected.whole); + assert.deepEqual(collected.whole.map(pduObj => pduObj.seqNr), [1, 2, 3]); assert.equal(reassembler.size, 0); + + // One id base per group: every segment of it was answered with a part of that base. + assert.equal(second.smsId, collected.smsId); + assert.equal(third.smsId, collected.smsId); + }); + + // A group the store cannot hold at all is refused, not accepted and then thrown away. + test('refuses a lone segment whose own arrival overruns the octet cap', () => { + const lost: LostGroup[] = []; + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + maxOctets: 10, + now: () => 0, + onLost: one => { lost.push(one); }, + timeout: 60_000, + }); + const refused = collect(reassembler, 8, 1, 2); + + assert.equal(refused.kept, false); + assert.equal(reassembler.size, 0); + assert.deepEqual(lost, [], 'the peer holds the only segment there was, so nothing was lost'); + }); + + // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. + test('reports the answered segments of a group that overruns the cap mid-message', () => { + const lost: LostGroup[] = []; + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + // One segment is 36 octets, so the second overruns a group already holding the first. + maxOctets: 50, + now: () => 0, + onLost: one => { lost.push(one); }, + timeout: 60_000, + }); + + assert.equal(collect(reassembler, 8, 1, 3).kept, true); + assert.equal(collect(reassembler, 8, 2, 3).kept, false); + assert.equal(reassembler.size, 0); + // One of the two the group held is the refused segment, which the peer still has. + assert.deepEqual( + lost.map(one => ({ parts: one.parts, reason: one.reason, total: one.total })), + [{ parts: 1, reason: 'evicted', total: 3 }], + ); + }); + + // Nothing else says a message the peer has already been answered for was thrown away. + test('names every group it gives up on, and why', () => { + let now = 0; + let issued = 0; + const ids = [ + '0199e0eb-4c11-7a02-9f31-2b6d80c4e517', + '0199e0eb-9d42-7bc6-8e70-51af3c92d6b8', + '0199e0ec-0e73-7d18-bb29-7c04ea51f3a9', + ]; + const lost: LostGroup[] = []; + const reassembler = new Reassembler({ + log: silentLog, + max: 1, + newId: () => ids[issued++] ?? '', + now: () => now, + onLost: one => { lost.push(one); }, + timeout: 60, + }); + + collect(reassembler, 1, 1, 2); + collect(reassembler, 2, 1, 3); + + now = 61; + reassembler.sweep(); + collect(reassembler, 3, 1, 2); + reassembler.clear(); + + assert.deepEqual(lost.map(one => one.reason), ['evicted', 'expired', 'linkGone']); + assert.deepEqual(lost.map(one => one.parts), [1, 1, 1]); + assert.deepEqual(lost.map(one => one.total), [2, 3, 2]); + assert.deepEqual(lost.map(one => one.smsId), ids, 'each group carries an id of its own'); }); // The UDH is peer-controlled, and the default authenticate() accepts every peer. @@ -1478,21 +1571,38 @@ describe('reassembly bounds', () => { verbose: noop, warn: msg => { warnings.push(msg); }, }; - const reassembler = new Reassembler({ log, max: 10, now: () => 0, timeout: 60_000 }); + const reassembler = new Reassembler({ + log, + max: 10, + now: () => 0, + onLost: () => undefined, + timeout: 60_000, + }); - assert.equal(collect(reassembler, 1, 1, 0), undefined); - assert.equal(collect(reassembler, 2, 0, 3), undefined); - assert.equal(collect(reassembler, 3, 4, 3), undefined); + assert.deepEqual( + [collect(reassembler, 1, 1, 0), collect(reassembler, 2, 0, 3), collect(reassembler, 3, 4, 3)] + .map(one => (one.kept ? undefined : one.refusal)), + Array(3).fill('unplaceable'), + ); assert.equal(reassembler.size, 0); assert.deepEqual(warnings, Array(3).fill('reassembler - dropping a segment the UDH numbers impossibly')); }); // Parts 1/2 then 2/3 would otherwise complete the stored two-part group, truncating the message. test('refuses a segment that renumbers how many parts the message has', () => { - const reassembler = new Reassembler({ log: silentLog, max: 10, now: () => 0, timeout: 60_000 }); + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + now: () => 0, + onLost: () => undefined, + timeout: 60_000, + }); - assert.equal(collect(reassembler, 9, 1, 2), undefined); - assert.equal(collect(reassembler, 9, 2, 3), undefined); + const first = collect(reassembler, 9, 1, 2); + + assert.ok(first.kept); + assert.equal(first.whole, undefined); + assert.equal(collect(reassembler, 9, 2, 3).kept, false); assert.equal(reassembler.size, 1); reassembler.clear(); @@ -1500,14 +1610,26 @@ describe('reassembly bounds', () => { // 0.4.0 held incomplete groups without limit and swept them only when other traffic arrived. test('drops the oldest incomplete message once the cap is reached', () => { - const reassembler = new Reassembler({ log: silentLog, max: 2, now: () => 0, timeout: 60_000 }); + const reassembler = new Reassembler({ + log: silentLog, + max: 2, + now: () => 0, + onLost: () => undefined, + timeout: 60_000, + }); for (const reference of [1, 2, 3]) { - assert.equal(collect(reassembler, reference, 1, 2), undefined); + const collected = collect(reassembler, reference, 1, 2); + + assert.ok(collected.kept); + assert.equal(collected.whole, undefined); } // Completing the first one must not produce a message: it was evicted. - assert.equal(collect(reassembler, 1, 2, 2), undefined); + const reopened = collect(reassembler, 1, 2, 2); + + assert.ok(reopened.kept); + assert.equal(reopened.whole, undefined); assert.equal(reassembler.size, 2); reassembler.clear(); @@ -1520,54 +1642,322 @@ describe('reassembly bounds', () => { // One segment is 36 octets: 14 of short_message plus the two 11-octet addresses. maxOctets: 80, now: () => 0, + onLost: () => undefined, timeout: 60_000, }); for (const reference of [1, 2, 3]) { - assert.equal(collect(reassembler, reference, 1, 2), undefined); + const collected = collect(reassembler, reference, 1, 2); + + assert.ok(collected.kept); + assert.equal(collected.whole, undefined); } assert.equal(reassembler.size, 2); - assert.equal(collect(reassembler, 1, 2, 2), undefined); + + const reopened = collect(reassembler, 1, 2, 2); + + assert.ok(reopened.kept); + assert.equal(reopened.whole, undefined); reassembler.clear(); }); // A retained subarray keeps its whole framed PDU alive, up to maxPduLength per segment. test('copies a segment out of the buffer it arrived in', () => { - const reassembler = new Reassembler({ log: silentLog, max: 10, now: () => 0, timeout: 60_000 }); + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + now: () => 0, + onLost: () => undefined, + timeout: 60_000, + }); const framed = Buffer.alloc(1024); const first = segment(6, 1, 2); Buffer.concat([Buffer.from([0x05, 0x00, 0x03, 6, 2, 1]), Buffer.from('fragment')]).copy(framed); first.params.short_message = framed.subarray(0, 14); - assert.equal(reassembler.collect(first, { part: 1, reference: 6, total: 2 }), undefined); + const held = reassembler.collect(first, { part: 1, reference: 6, total: 2 }); + + assert.ok(held.kept); + assert.equal(held.whole, undefined); framed.fill(0x00); - const whole = collect(reassembler, 6, 2, 2); + const collected = collect(reassembler, 6, 2, 2); - assert.ok(whole); - assert.equal(decodeSegments(whole), 'fragmentfragment'); + assert.ok(collected.kept); + assert.ok(collected.whole); + assert.equal(decodeSegments(collected.whole), 'fragmentfragment'); }); test('expires an incomplete message once its timeout has passed', () => { let now = 0; - const reassembler = new Reassembler({ log: silentLog, max: 10, now: () => now, timeout: 60 }); + const reassembler = new Reassembler({ + log: silentLog, + max: 10, + now: () => now, + onLost: () => undefined, + timeout: 60, + }); + const first = collect(reassembler, 9, 1, 2); - assert.equal(collect(reassembler, 9, 1, 2), undefined); + assert.ok(first.kept); + assert.equal(first.whole, undefined); now = 61; // The other half arrives after the group expired, so it starts a new, still-incomplete one. - assert.equal(collect(reassembler, 9, 2, 2), undefined); + const late = collect(reassembler, 9, 2, 2); + + assert.ok(late.kept); + assert.equal(late.whole, undefined); + assert.notEqual(late.smsId, first.smsId); assert.equal(reassembler.size, 1); reassembler.clear(); }); }); +// Jasmin dispatches one request per connector at a time: holding a group unanswered until it was +// whole deadlocked every multi-segment message against it (interop-tests/findings/03-jasmin.md). +describe('the status a refused segment is answered with', () => { + // SMPP 3.4 lists ESME_RMSGQFUL under submit_sm_resp only; 4.6.2's retryable code is another. + test('names one the command the segment arrived on defines', () => { + assert.equal(refusedSegmentStatus('submit_sm', 'full'), 'ESME_RMSGQFUL'); + assert.equal(refusedSegmentStatus('deliver_sm', 'full'), 'ESME_RX_T_APPN'); + assert.equal(refusedSegmentStatus('submit_sm', 'unplaceable'), 'ESME_RINVESMCLASS'); + assert.equal(refusedSegmentStatus('deliver_sm', 'unplaceable'), 'ESME_RINVESMCLASS'); + }); +}); + +describe('a peer that sends the next segment only once the last one is answered', () => { + const text = 'A relay that waits for each response before it sends the next segment. '.repeat(4); + + function segmentsOf(reference: number): Buffer[] { + const segments = splitMessage(text, { reference }); + + assert.ok(segments.length > 1, 'the fixture must need more than one segment'); + + return segments; + } + + async function submit(session: Session, segment: Buffer): Promise { + const answered = await session.send({ + cmdName: 'submit_sm', + params: submitSmParams( + { from: '46701113311', message: text, to: '46709771337' }, + segment, + { encoding: 'ASCII', multipart: true }, + ), + }); + + assert.equal(answered.err, undefined); + assert.ok(answered.pduObj); + + return answered.pduObj; + } + + async function submitSerially(session: Session, reference: number): Promise { + const answers: PduObject[] = []; + + for (const segment of segmentsOf(reference)) { + answers.push(await submit(session, segment)); + } + + return answers; + } + + test('gets every segment answered as it arrives, and the application one whole message', async t => { + const smpp = await startServer(t); + const messages: Sms[] = []; + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', sms => { + messages.push(sms); + resolve(sms); + })); + }); + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + const answers = await submitSerially(session, 0x2A); + const sms = await incoming; + + assert.equal(sms.message, text); + assert.equal(sms.pduObjs.length, answers.length); + assert.equal(messages.length, 1, 'the application sees one message, not one per segment'); + assert.equal(sms.answeredOnArrival, true); + assert.deepEqual(answers.map(answer => answer.cmdStatus), answers.map(() => 'ESME_ROK')); + assert.deepEqual( + answers.map(answer => paramText(answer.params.message_id)), + answers.map((_answer, index) => `${sms.smsId}-${String(index + 1)}`), + ); + assert.deepEqual(await sms.sendResp(), {}); + }); + + // The documented single-segment contract, which the segment-by-segment answer must not touch. + test('answers a single-segment message only once the application does, with the id it chose', async t => { + const smpp = await startServer(t); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', resolve)); + }); + const { session } = await connect(t, smpp); + + assert.ok(session); + + const submitted = session.send({ + cmdName: 'submit_sm', + params: { + destination_addr: '46709771337', + short_message: 'one segment, answered by the application', + source_addr: '46701113311', + }, + }); + const sms = await incoming; + + assert.equal(await within(150, submitted), undefined, 'nothing may answer for the application'); + assert.equal(sms.answeredOnArrival, false); + assert.deepEqual(await sms.sendResp({ smsId: '0199e0e9-4a3e-7c62-9a4b-1f0c5d7e8a21' }), {}); + + const answered = await submitted; + + assert.ok(answered.pduObj); + assert.equal( + paramText(answered.pduObj.params.message_id), + '0199e0e9-4a3e-7c62-9a4b-1f0c5d7e8a21', + ); + }); + + test('refuses an id and a refusing status for segments already on the wire', async t => { + const smpp = await startServer(t); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', resolve)); + }); + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + await submitSerially(session, 0x2B); + + const sms = await incoming; + const named = await sms.sendResp({ smsId: '0199e0ea-1f3d-7ab4-8c21-6d4e5f0a9b73' }); + const refused = await sms.sendResp({ status: 'ESME_RMSGQFUL' }); + + assert.match(named.err?.message ?? '', /fixed when its first segment arrived/); + assert.match(refused.err?.message ?? '', /onRequest/); + assert.deepEqual(await sms.sendResp({ status: 'ESME_ROK' }), {}); + assert.equal(sms.answeredOnArrival, true); + }); + + test('reports a half-arrived message it has already answered, and holds nothing after', async t => { + const smpp = await startServer(t, { reassemblyTimeout: 60 }); + const messages: Sms[] = []; + const lost = once(resolve => { + smpp.on('session', bound => { + bound.on('sessionError', resolve); + bound.on('sms', sms => { messages.push(sms); }); + }); + }); + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + const [first] = segmentsOf(0x2C); + + assert.ok(first); + + const answered = await submit(session, first); + + assert.equal(answered.cmdStatus, 'ESME_ROK'); + assert.match((await lost).message, /Gave up 1 of \d+ segments/); + assert.equal(messages.length, 0); + assert.deepEqual(await peerOf(smpp).close(), {}, 'a group nothing completed is not held'); + }); + + test('close() still waits for a concatenated message the application has not answered', async t => { + const smpp = await startServer(t, { shutdownTimeout: 50 }); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', resolve)); + }); + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + await submitSerially(session, 0x2D); + await incoming; + + const closed = await peerOf(smpp).close(); + + assert.ok(closed.err instanceof Error); + assert.match(closed.err.message, /1 message\(s\) unanswered/); + }); + + // pduObjs.length is 1 either way here, so answeredOnArrival is the only thing that can say. + test('marks a one-part concatenated message answered, as its segment count cannot', async t => { + const smpp = await startServer(t); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', resolve)); + }); + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + const answered = await session.send({ + cmdName: 'submit_sm', + params: { + destination_addr: '46709771337', + esm_class: consts.ESM_CLASS.UDH_INDICATOR, + short_message: Buffer.concat([ + Buffer.from([0x05, 0x00, 0x03, 0x2F, 0x01, 0x01]), + Buffer.from('one part of one'), + ]), + source_addr: '46701113311', + }, + }); + const sms = await incoming; + + assert.equal(answered.err, undefined); + assert.ok(answered.pduObj); + assert.equal(answered.pduObj.cmdStatus, 'ESME_ROK'); + assert.equal(paramText(answered.pduObj.params.message_id), sms.smsId); + assert.equal(sms.pduObjs.length, 1); + assert.equal(sms.answeredOnArrival, true); + assert.deepEqual(await sms.sendResp(), {}); + }); + + // esm_class said there was a UDH, and there is no group its concatenation fields can join. + test('answers a segment whose UDH cannot be honoured rather than leaving the peer waiting', async t => { + const smpp = await startServer(t); + const messages: Sms[] = []; + + smpp.on('session', bound => bound.on('sms', sms => { messages.push(sms); })); + + const { session } = await connect(t, smpp, { responseTimeout: 1000 }); + + assert.ok(session); + + const answered = await session.send({ + cmdName: 'submit_sm', + params: { + destination_addr: '46709771337', + esm_class: consts.ESM_CLASS.UDH_INDICATOR, + short_message: Buffer.concat([ + Buffer.from([0x05, 0x00, 0x03, 0x2E, 0x02, 0x09]), + Buffer.from('part nine of two'), + ]), + source_addr: '46701113311', + }, + }); + + assert.equal(answered.err, undefined); + assert.ok(answered.pduObj); + assert.equal(answered.pduObj.cmdStatus, 'ESME_RINVESMCLASS'); + assert.deepEqual(messages, []); + }); +}); + describe('AbortSignal on a send', () => { test('gives up on an in-flight request when the signal fires', async t => { const accepted: net.Socket[] = []; @@ -1735,10 +2125,10 @@ describe('graceful shutdown', () => { }); const closing = peerOf(smpp).close(); - await sms.sendResp({ smsId: 'held-through-the-drain' }); + assert.deepEqual(await sms.sendResp(), {}); const receiptSent = await sms.sendDlr('DELIVERED'); - const ids = ['held-through-the-drain-1', 'held-through-the-drain-2', 'held-through-the-drain-3']; + const ids = [1, 2, 3].map(part => `${sms.smsId}-${String(part)}`); assert.equal(receiptSent.err, undefined); assert.deepEqual((await receipts).map(dlr => dlr.smsId), ids); @@ -1872,6 +2262,7 @@ describe('graceful shutdown', () => { // The queued segments are the whole reason the drain waits on the window and not on the pending map. test('counts the segments still queued behind a full window', async t => { + // No 'sms' listener, so the single-segment message holding the only slot is never answered. const smpp = await startServer(t); const onWire = once(resolve => { smpp.on('session', bound => bound.on('incomingPduObj', resolve)); @@ -1880,7 +2271,12 @@ describe('graceful shutdown', () => { assert.ok(session); - const sent = session.sendSms({ + const holding = session.sendSms({ + from: '46701113311', + message: 'holds the only slot', + to: '46709771337', + }); + const queued = session.sendSms({ from: '46701113311', message: 'x'.repeat(400), to: '46709771337', @@ -1891,8 +2287,9 @@ describe('graceful shutdown', () => { const closed = await session.close(); assert.ok(closed.err instanceof Error); - assert.match(closed.err.message, /3 request\(s\)/); - assert.ok((await sent).err instanceof Error); + assert.match(closed.err.message, /4 request\(s\)/); + assert.ok((await holding).err instanceof Error); + assert.ok((await queued).err instanceof Error); }); test('an aborted close tears down at once instead of waiting out the drain', async t => { @@ -2048,11 +2445,12 @@ describe('message id notation', () => { test('leaves the segment ids of a multipart send to merge as they are', async t => { const smpp = await startServer(t); - - smpp.on('session', bound => { - bound.on('sms', sms => { void sms.sendResp({ smsId: 'beef' }); }); + const incoming = once(resolve => { + smpp.on('session', bound => bound.on('sms', sms => { + resolve(sms); + void sms.sendResp(); + })); }); - const { session } = await connect(t, smpp, { smsIdFormat: { receipt: 'decimal', submitResp: 'hex' }, }); @@ -2061,14 +2459,15 @@ describe('message id notation', () => { const merged = once(resolve => { session.on('messageDlr', resolve); }); const sent = await sendOne(session, 'x'.repeat(200)); + const sms = await incoming; - assert.deepEqual(sent.smsIds, ['beef-1', 'beef-2']); + assert.deepEqual(sent.smsIds, [1, 2].map(part => `${sms.smsId}-${String(part)}`)); for (const smsId of sent.smsIds) { await sendReceipt(peerOf(smpp), smsId); } - assert.equal((await merged).smsId, 'beef'); + assert.equal((await merged).smsId, sms.smsId); }); test('refuses a notation it cannot apply', () => { diff --git a/test/session.test.ts b/test/session.test.ts index 17923c7..1d6e100 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -566,7 +566,7 @@ describe('sending', () => { const [sms, sent] = await Promise.all([ incoming.then(async received => { - await received.sendResp({ smsId: 'long-id' }); + await received.sendResp(); return received; }), @@ -577,7 +577,7 @@ describe('sending', () => { assert.ok(sms.pduObjs.length > 1); assert.equal(sent.err, undefined); assert.equal(sent.pduObjs.length, 4); - assert.deepEqual(sent.smsIds, ['long-id-1', 'long-id-2', 'long-id-3', 'long-id-4']); + assert.deepEqual(sent.smsIds, [1, 2, 3, 4].map(part => `${sms.smsId}-${String(part)}`)); }); test('carries a UCS2 message through unchanged', async t => { @@ -699,7 +699,12 @@ describe('receiving', () => { assert.ok(answered.pduObj); assert.equal(answered.pduObj.cmdName, 'deliver_sm_resp'); - assert.equal(answered.pduObj.params.message_id, 'inbound-id'); + assert.equal( + paramText(answered.pduObj.params.message_id), + '', + 'SMPP 3.4 4.6.2 leaves deliver_sm_resp\'s message_id unused', + ); + assert.equal(sms.smsId, 'inbound-id', 'the id the application chose is still its own handle'); }); test('hands a client a report as a dlr rather than as an sms', async t => { @@ -804,16 +809,12 @@ describe('receiving', () => { assert.equal(sms.message, message); assert.equal(sms.pduObjs.length, 2); - await sms.sendResp({ smsId: 'inbound-long' }); - - const ids: string[] = []; + assert.deepEqual(await sms.sendResp(), {}); for (const answered of await delivered) { assert.ok(answered.pduObj); - ids.push(paramText(answered.pduObj.params.message_id)); + assert.equal(paramText(answered.pduObj.params.message_id), ''); } - - assert.deepEqual(ids, ['inbound-long-1', 'inbound-long-2']); }); }); @@ -939,7 +940,7 @@ describe('delivery reports', () => { const [sms] = await Promise.all([ incoming.then(async received => { - await received.sendResp({ smsId: 'unrequested' }); + await received.sendResp(); return received; }), @@ -948,7 +949,7 @@ describe('delivery reports', () => { await sms.sendDlr(); - assert.deepEqual(perSegment, ['unrequested-1', 'unrequested-2', 'unrequested-3']); + assert.deepEqual(perSegment, [1, 2, 3].map(part => `${sms.smsId}-${String(part)}`)); assert.equal(merged, 0); }); }); diff --git a/test/sms-id.test.ts b/test/sms-id.test.ts index 3259377..0240cc1 100644 --- a/test/sms-id.test.ts +++ b/test/sms-id.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import test, { describe } from 'node:test'; -import { normaliseSmsId } from '../src/sms-id.ts'; +import { normaliseSmsId, parseSegmentId, respIdParams, segmentId } from '../src/sms-id.ts'; describe('normaliseSmsId()', () => { test('reads an id the length a message_id may be, and leaves a longer one alone', () => { @@ -21,3 +21,27 @@ describe('normaliseSmsId()', () => { assert.equal(normaliseSmsId('1a2B', 'hex'), normaliseSmsId('1A2b', 'hex')); }); }); + +describe('the - a segment is answered with', () => { + const base = '0199e0ed-3a55-7e91-9c04-6b7f2d81a5e2'; + + // One writer, two readers: DlrMerger has to read back exactly what a segment was answered with. + test('reads back the base and the part it was written from', () => { + assert.equal(segmentId(base, 0, 1), base, 'a lone segment is the base itself'); + assert.equal(segmentId(base, 2, 3), `${base}-3`); + assert.deepEqual(parseSegmentId(`${base}-3`), { base, part: 3 }); + }); + + test('reads nothing out of an id no segment convention wrote', () => { + assert.equal(parseSegmentId(base), undefined); + assert.equal(parseSegmentId(`${base}-x`), undefined); + assert.equal(parseSegmentId(''), undefined); + }); +}); + +describe('the id a response carries', () => { + test('leaves a deliver_sm_resp without one, and gives submit_sm_resp its own', () => { + assert.deepEqual(respIdParams('deliver_sm', 'x'), {}); + assert.deepEqual(respIdParams('submit_sm', 'x'), { message_id: 'x' }); + }); +});