diff --git a/AGENTS.md b/AGENTS.md index ba0f185..34678b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -287,3 +287,16 @@ exactly 140. `node:24.18.0-bookworm-slim` ships no openssl binary, so a shelled-out fixture would pass in CI and fail on every developer machine, and a committed key leaks in a public repository. Valid while the dev image has no openssl. + +- **The notation a peer writes message ids in is named 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 — `smsIdFormat` names + `receipt` and `submitResp` separately and reads both into a plain decimal value before `smsIds` + and `dlr.smsId` are compared. Omitting a place is what leaving it alone means, so there is no + `raw` notation, and a caller-supplied formatter is refused because it would make the promise that + those two are comparable unverifiable — `onRequest` and the PDU on the `dlr` event are the escape + hatches, and the `onReceipt` hook in todo.md is the seam if one is wanted. An id no notation reads + is left exactly as it arrived, which is what keeps `DlrMerger` working: a `-` id parses + as no number and so reaches `expect()` and `collect()` unchanged. Normalising the base instead + would break that pair. The option is on `client()` only — a `server()` session generates its own + ids and writes its own receipts, so both places are already one notation. diff --git a/README.md b/README.md index 33eee92..a35c594 100644 --- a/README.md +++ b/README.md @@ -172,8 +172,8 @@ before you see them: const { err, session } = await client({ smsIdFormat: { receipt: 'decimal', submitResp: 'hex' } }); ``` -An id that is not a number in the notation given is left exactly as it arrived, and `pduObjs` and -`dlr.receipt` carry the id as the peer wrote it either way. +An id that is not a number in the notation given is left exactly as it arrived, and the PDUs carry +what the peer wrote either way — `pduObjs` from the send, and the second argument of the `dlr` event. ## Server diff --git a/src/client.ts b/src/client.ts index 3100758..ba07528 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2,7 +2,7 @@ import type { ConnectionOptions } from 'node:tls'; import type { Result, VoidResult } from './result.ts'; import type { BindType } from './session-options.ts'; import type { SmppLog } from './log.ts'; -import type { SmsIdFormats } from './sms-id.ts'; +import type { SmsIdFormat } from './sms-id.ts'; import type { Socket } from 'node:net'; export type { BindType }; @@ -30,7 +30,7 @@ export type ClientOptions = { responseTimeout?: number; shutdownTimeout?: number; signal?: AbortSignal; - smsIdFormat?: SmsIdFormats; + smsIdFormat?: SmsIdFormat; systemType?: string; tls?: ConnectionOptions | boolean; username?: string; diff --git a/src/dlr.ts b/src/dlr.ts index 6ced864..c736ff9 100644 --- a/src/dlr.ts +++ b/src/dlr.ts @@ -1,7 +1,7 @@ import type { MessageState } from './defs/constants.ts'; import type { ParamValue } from './defs/types.ts'; import type { PduObject } from './pdu.ts'; -import type { SmsIdFormat } from './sms-id.ts'; +import type { SmsIdNotation } from './sms-id.ts'; import { consts, constsById, messageTypeOf } from './defs/constants.ts'; import { decodeMessage } from './message.ts'; import { normaliseSmsId } from './sms-id.ts'; @@ -164,11 +164,11 @@ function receiptBody(pduObj: PduObject): string { function receiptId( tlvId: ParamValue | undefined, receipt: Receipt | undefined, - format: SmsIdFormat | undefined, + notation: SmsIdNotation | undefined, ): string | undefined { const id = nonEmptyText(tlvId) ?? nonEmptyText(receipt?.id); - return id === undefined ? undefined : normaliseSmsId(id, format); + return id === undefined ? undefined : normaliseSmsId(id, notation); } function isMessageState(name: string | undefined): name is MessageState { @@ -192,14 +192,14 @@ function receiptStatus( } /** The delivery report a deliver_sm carries, or nothing when it carries a message instead. */ -export function dlrFromPdu(pduObj: PduObject, format?: SmsIdFormat): Dlr | undefined { +export function dlrFromPdu(pduObj: PduObject, notation?: SmsIdNotation): Dlr | undefined { const type = messageType(pduObj); if (type === 'other') return undefined; const body = receiptBody(pduObj); const receipt = body === '' ? undefined : parseReceipt(body); - const smsId = receiptId(pduObj.tlvs.receipted_message_id?.tagValue, receipt, format); + const smsId = receiptId(pduObj.tlvs.receipted_message_id?.tagValue, receipt, notation); const { statusId, statusMsg } = receiptStatus(pduObj.tlvs.message_state?.tagValue, receipt); if (type === 'unmarked' && (smsId === undefined || statusMsg === undefined)) return undefined; diff --git a/src/incoming-requests.ts b/src/incoming-requests.ts index b273229..b0d93be 100644 --- a/src/incoming-requests.ts +++ b/src/incoming-requests.ts @@ -3,7 +3,7 @@ import type { OnRequest } from './session-options.ts'; import type { PduObject } from './pdu.ts'; import type { Session } from './session.ts'; import type { SmppLog } from './log.ts'; -import type { SmsIdFormat } from './sms-id.ts'; +import type { SmsIdNotation } from './sms-id.ts'; import { Reassembler, decodeSegments } from './reassembly.ts'; import { bindCommands, defaults } from './session-options.ts'; import { concatInfo } from './udh.ts'; @@ -19,7 +19,7 @@ export type IncomingRequestsOptions = { maxReassembly?: number | undefined; onRequest?: OnRequest | undefined; reassemblyTimeout?: number | undefined; - receiptIdFormat?: SmsIdFormat | undefined; + receiptIdNotation?: SmsIdNotation | undefined; session: Session; systemId?: string | undefined; }; @@ -30,7 +30,7 @@ export class IncomingRequests { private readonly log: SmppLog; private readonly onRequest: OnRequest | undefined; private readonly reassembler: Reassembler; - private readonly receiptIdFormat: SmsIdFormat | undefined; + private readonly receiptIdNotation: SmsIdNotation | undefined; private readonly session: Session; private readonly systemId: string; @@ -44,7 +44,7 @@ export class IncomingRequests { maxOctets: options.maxOctets, timeout: options.reassemblyTimeout ?? defaults.reassemblyTimeout, }); - this.receiptIdFormat = options.receiptIdFormat; + this.receiptIdNotation = options.receiptIdNotation; this.session = options.session; this.systemId = options.systemId ?? defaults.systemId; } @@ -101,7 +101,7 @@ export class IncomingRequests { /** SMPP carries a mobile-originated message and a delivery receipt on the same command. */ private async onDeliverSm(pduObj: PduObject): Promise { - const dlr = dlrFromPdu(pduObj, this.receiptIdFormat); + const dlr = dlrFromPdu(pduObj, this.receiptIdNotation); if (!dlr) { this.onMessage(pduObj); diff --git a/src/index.ts b/src/index.ts index 453943f..72a4615 100644 --- a/src/index.ts +++ b/src/index.ts @@ -39,7 +39,7 @@ export type { SendRespOptions, Sms, SmsInput } from './sms.ts'; export type { ConcatInfo } from './udh.ts'; export type { Result, VoidResult } from './result.ts'; export type { SmppLog } from './log.ts'; -export type { SmsIdFormat, SmsIdFormats } from './sms-id.ts'; +export type { SmsIdFormat, SmsIdNotation } from './sms-id.ts'; export type { AuthenticateInput, AuthenticateResult, diff --git a/src/send-sms.ts b/src/send-sms.ts index 3688a83..509b8c7 100644 --- a/src/send-sms.ts +++ b/src/send-sms.ts @@ -3,7 +3,7 @@ import type { ParamValue } from './defs/types.ts'; import type { PduObject, PduObjectInput } from './pdu.ts'; import type { Result } from './result.ts'; import type { SmppLog } from './log.ts'; -import type { SmsIdFormat } from './sms-id.ts'; +import type { SmsIdNotation } from './sms-id.ts'; import { consts } from './defs/constants.ts'; import { detect } from './defs/encodings.ts'; import { normaliseSmsId } from './sms-id.ts'; @@ -34,7 +34,7 @@ export type SendSmsResult = { err?: Error; pduObjs: PduObject[]; smsIds: string[ export type SendSmsDeps = { log: SmppLog; reference: number; - respIdFormat?: SmsIdFormat | undefined; + respIdNotation?: SmsIdNotation | undefined; send: (input: PduObjectInput) => Promise>; }; @@ -103,7 +103,7 @@ function checkSegments(allowed: number, segments: number): Error | undefined { function collectSent( sent: Result<{ pduObj: PduObject }>[], - format: SmsIdFormat | undefined, + notation: SmsIdNotation | undefined, ): SendSmsResult { const pduObjs: PduObject[] = []; const smsIds: string[] = []; @@ -114,7 +114,7 @@ function collectSent( failure ??= one.err; } else if (one.pduObj.cmdStatus === 'ESME_ROK') { pduObjs.push(one.pduObj); - smsIds.push(normaliseSmsId(paramText(one.pduObj.params.message_id), format)); + smsIds.push(normaliseSmsId(paramText(one.pduObj.params.message_id), notation)); } else { const refusal = one.pduObj.cmdStatus ?? String(one.pduObj.cmdStatusId); @@ -144,5 +144,5 @@ export async function submitSms(deps: SendSmsDeps, sms: SendSmsOptions): Promise params: submitSmParams(sms, segment, { encoding, multipart }), }))); - return collectSent(sent, deps.respIdFormat); + return collectSent(sent, deps.respIdNotation); } diff --git a/src/session-options.ts b/src/session-options.ts index 224738e..3370fef 100644 --- a/src/session-options.ts +++ b/src/session-options.ts @@ -4,10 +4,10 @@ import type { PduObject } from './pdu.ts'; import type { Result, VoidResult } from './result.ts'; import type { Session } from './session.ts'; import type { SmppLog } from './log.ts'; -import type { SmsIdFormats } from './sms-id.ts'; +import type { SmsIdFormat } from './sms-id.ts'; import type { Sms } from './sms.ts'; import type { Socket } from 'node:net'; -import { isSmsIdFormat } from './sms-id.ts'; +import { isSmsIdNotation, smsIdNotations, smsIdPlaces } from './sms-id.ts'; export type SessionEvents = { close: []; @@ -86,7 +86,7 @@ export type SessionOptions = { /** How long a drain waits for the requests already on the wire. 0 waits forever. */ shutdownTimeout?: number | undefined; /** The notation the peer writes message ids in, where it is not the one they are compared in. */ - smsIdFormat?: SmsIdFormats | undefined; + smsIdFormat?: SmsIdFormat | undefined; sock: Socket; /** This end's own identity, answered to the peer in place of the one it sent. */ systemId?: string | undefined; @@ -131,19 +131,31 @@ export function checkSessionOptions(options: CheckableOptions): VoidResult { } } - return checkSmsIdFormats(options.smsIdFormat); + return checkSmsIdFormat(options.smsIdFormat); } -function checkSmsIdFormats(smsIdFormat: CheckableOptions['smsIdFormat']): VoidResult { - const formats: [string, string | undefined][] = [ - ['receipt', smsIdFormat?.receipt], - ['submitResp', smsIdFormat?.submitResp], - ]; +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} - for (const [place, format] of formats) { - if (format !== undefined && !isSmsIdFormat(format)) { - return { err: new Error(`smsIdFormat.${place} must be decimal or hex, got ${format}`) }; - } +function checkSmsIdFormat(smsIdFormat: unknown): VoidResult { + if (smsIdFormat === undefined) return {}; + + if (!isRecord(smsIdFormat)) { + return { err: new Error('smsIdFormat names a notation per place, as { receipt, submitResp }') }; + } + + const allowed = smsIdNotations.join(' or '); + + for (const place of smsIdPlaces) { + const notation = smsIdFormat[place]; + + if (notation === undefined || isSmsIdNotation(notation)) continue; + + // String() throws on a null-prototype object, and this value is whatever the caller passed. + const got = typeof notation === 'string' ? notation : typeof notation; + + return { err: new Error(`smsIdFormat.${place} must be ${allowed}, got ${got}`) }; } return {}; @@ -157,5 +169,5 @@ export type CheckableOptions = { reassemblyTimeout?: number | undefined; responseTimeout?: number | undefined; shutdownTimeout?: number | undefined; - smsIdFormat?: { receipt?: string | undefined; submitResp?: string | undefined } | undefined; + smsIdFormat?: unknown; }; diff --git a/src/session.ts b/src/session.ts index 1107b64..95ca5f8 100644 --- a/src/session.ts +++ b/src/session.ts @@ -120,7 +120,7 @@ export class Session extends EventEmitter { maxReassembly: options.maxReassembly, onRequest: options.onRequest, reassemblyTimeout: options.reassemblyTimeout, - receiptIdFormat: options.smsIdFormat?.receipt, + receiptIdNotation: options.smsIdFormat?.receipt, session: this, systemId: options.systemId, }); @@ -210,7 +210,7 @@ export class Session extends EventEmitter { const sent = await submitSms({ log: this.log, reference: this.nextConcatReference(), - respIdFormat: this.options.smsIdFormat?.submitResp, + respIdNotation: this.options.smsIdFormat?.submitResp, send: input => this.send(input, options), }, sms); diff --git a/src/sms-id.ts b/src/sms-id.ts index 96d3d6a..6f54ef6 100644 --- a/src/sms-id.ts +++ b/src/sms-id.ts @@ -1,32 +1,37 @@ -/** The notation a peer writes message ids in. */ -export type SmsIdFormat = 'decimal' | 'hex'; - -/** The notation per place the peer writes an id. An omitted place is left as it arrived. */ -export type SmsIdFormats = { - receipt?: SmsIdFormat | undefined; - submitResp?: SmsIdFormat | undefined; -}; - -export function isSmsIdFormat(value: unknown): value is SmsIdFormat { - return value === 'decimal' || value === 'hex'; -} - -// SMPP 3.4 caps message_id at 64 octets, and BigInt on a longer string is a peer-controlled cost. -const maxIdLength = 64; - const notations = { decimal: { digits: /^[0-9]+$/, prefix: '' }, hex: { digits: /^[0-9a-f]+$/i, prefix: '0x' }, }; +/** The notation a peer writes message ids in. */ +export type SmsIdNotation = keyof typeof notations; + +/** The notation per place the peer writes an id. An omitted place is left as it arrived. */ +export type SmsIdFormat = { + receipt?: SmsIdNotation | undefined; + submitResp?: SmsIdNotation | undefined; +}; + +export const smsIdNotations: string[] = Object.keys(notations); + +export const smsIdPlaces: (keyof SmsIdFormat)[] = ['receipt', 'submitResp']; + +export function isSmsIdNotation(value: unknown): value is SmsIdNotation { + return typeof value === 'string' && Object.hasOwn(notations, value); +} + +// SMPP 3.4 caps message_id at 64 octets, and BigInt on a longer string is a peer-controlled cost. +const maxIdLength = 64; + /** * The id as a plain decimal value, so an SMSC that answers a submit in one notation and writes the - * receipt in another still correlates. An id the notation cannot read is left as it arrived. + * receipt in another still correlates. An id the notation cannot read is left as it arrived, which + * is what leaves a `-` id whole for DlrMerger. */ -export function normaliseSmsId(id: string, format: SmsIdFormat | undefined): string { - if (format === undefined || id.length > maxIdLength) return id; +export function normaliseSmsId(id: string, notation: SmsIdNotation | undefined): string { + if (notation === undefined || id.length > maxIdLength) return id; - const { digits, prefix } = notations[format]; + const { digits, prefix } = notations[notation]; return digits.test(id) ? BigInt(`${prefix}${id}`).toString(10) : id; } diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 98fee0b..3adf63b 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -872,6 +872,8 @@ describe('message id notation', () => { assert.ok(checked.err instanceof Error); assert.match(checked.err.message, /smsIdFormat\.receipt/); + // The shape todo.md sketched, which a caller without types would otherwise pass unnoticed. + assert.ok(checkSessionOptions({ smsIdFormat: 'hex' }).err instanceof Error); assert.equal(checkSessionOptions({ smsIdFormat: { submitResp: 'hex' } }).err, undefined); }); }); diff --git a/test/sms-id.test.ts b/test/sms-id.test.ts new file mode 100644 index 0000000..3259377 --- /dev/null +++ b/test/sms-id.test.ts @@ -0,0 +1,23 @@ +import assert from 'node:assert/strict'; +import test, { describe } from 'node:test'; +import { normaliseSmsId } from '../src/sms-id.ts'; + +describe('normaliseSmsId()', () => { + test('reads an id the length a message_id may be, and leaves a longer one alone', () => { + const padded = `0${'1'.repeat(63)}`; + const tooLong = `0${'1'.repeat(64)}`; + + assert.equal(normaliseSmsId(padded, 'decimal'), '1'.repeat(63)); + assert.equal(normaliseSmsId(tooLong, 'decimal'), tooLong); + }); + + test('leaves an id the notation cannot read as it arrived', () => { + assert.equal(normaliseSmsId('', 'hex'), ''); + assert.equal(normaliseSmsId('0x1f', 'hex'), '0x1f'); + assert.equal(normaliseSmsId('beef-1', 'hex'), 'beef-1', 'the segment convention stays whole'); + }); + + test('reads either case of a hexadecimal id', () => { + assert.equal(normaliseSmsId('1a2B', 'hex'), normaliseSmsId('1A2b', 'hex')); + }); +});