diff --git a/AGENTS.md b/AGENTS.md index 27eee63..24a2f3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,9 @@ These are not preferences. Breaking one is a defect. 3. **No `error` event.** Node makes an unhandled `error` event throw, which would break rule 1. Sessions emit `sessionError`, servers emit `serverError`. 4. **No casts, no non-null assertions.** `as`, `as unknown as` and `!` are all banned. Parse untyped - input once through a type guard at the boundary; everything past it is typed. + input once through a type guard at the boundary; everything past it is typed. `noUncheckedIndexedAccess` + is on, so every lookup into a record or buffer is `T | undefined` until you handle it — that is the + point, not an obstacle to route around. ## Architecture @@ -40,12 +42,12 @@ src/ sms.ts The live handle emitted as the 'sms' event (sendResp/sendDlr) message.ts Encoding detection, splitting, bit counting, SMPP date formatting pdu.ts pduToObj / objToPdu / pduReturn — synchronous, result-returning + result.ts Result — the shape every fallible call returns defs/ commands.ts The 33 commands, their ids and ordered parameter lists constants.ts consts + constsById (TON, NPI, ENCODING, MESSAGE_STATE, …) - encodings.ts GSM 03.38, LATIN1, UCS2 and detection + encodings.ts GSM 03.38, LATIN1, UCS2, detection, data_coding resolution errors.ts errors + errorsById (ESME_*) - filters.ts Per-field encode/decode hooks (time, message, callback_num, …) tlvs.ts TLV definitions, tlvsById types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays ``` @@ -96,11 +98,16 @@ implementation (see todo.md). | Text-only DLRs refused | `deliver_sm` without both `message_state` and `receipted_message_id` TLVs is rejected with `ESME_RINVTLVSTREAM`, so Kannel-style receipts are unusable | | Unbounded reassembly | Incomplete long-SMS groups are capped by nothing and swept only when other traffic arrives, after 24 hours | | Dead DLR aggregation | `longSmsDlrs` is allocated to merge per-segment receipts and then never used | +| Trailing NULL truncation | `types.buffer.size()` subtracts one whenever the value's last octet is `0x00`, so the PDU is allocated one octet short while `sm_length` still reports the full length. Any UCS2 message ending in a character like U+4E00 or U+3000 goes out corrupt | +| Dormant filters | `defs.filters` is declared on commands and TLVs but never invoked anywhere. Dropped in the rewrite; SMPP time formatting is exported as `smppTime` instead | +| Unchecked reads | Wire reads index straight into the buffer, so a short or malformed PDU throws out of the codec. Reads are bounds-checked and return results now | +| Unrangechecked writes | Integer params are handed to `writeUInt8`/`writeUInt16BE` unvalidated, so an out-of-range value throws from inside Node | ## Conventions -- Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant - (see the wire-order note above). +- Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant. + Two deliberate exceptions: command parameters are in wire order (above), and the `errors` and TLV + tables are ordered by their numeric id so they can be diffed against the spec and gaps stay visible. - Comments are the exception, not the default — see the root `CLAUDE.md` rules. Do not write file preambles or restate what the code says. - Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders. diff --git a/README.md b/README.md index ba03071..08e0412 100644 --- a/README.md +++ b/README.md @@ -166,9 +166,11 @@ commands the codec knows, not just the four the session handles natively. `{ userData }`. - **Renamed options:** `enqLinkTiming` → `enquireLinkInterval`, server `timeout` → `idleTimeout`. - **`larvitsmpp.utils` is gone.** Its contents are named exports: `bitCount`, `decodeMessage`, - `encodeMessage`, `objToPdu`, `pduReturn`, `pduToObj`, `smppDate`, `splitMessage`. The PDU codec is + `encodeMessage`, `objToPdu`, `pduReturn`, `pduToObj`, `smppDate`, `smppTime`, `splitMessage`. The PDU codec is synchronous and returns `{ err, pduObj }` / `{ err, buffer }`. - **`pduObj.isResp()` is now the standalone `isResp(pduObj)`.** +- **`defs.filters` is gone.** It was declared on every command and TLV but never invoked, so it did + nothing. SMPP time formatting, the one part worth keeping, is exported as `smppTime`. - **The `error` event is `sessionError`** (and `serverError` on the server handle). - **`log`** takes a [`@larvit/log`](https://www.npmjs.com/package/@larvit/log) instance instead of a `larvitutils` one, and is silent by default. @@ -188,6 +190,10 @@ have worked around any of these, remove the workaround: - Alphanumeric senders were sent with TON 1 (international) instead of TON 5. - Delivery receipts carrying only the standard receipt text, with no TLVs — what Kannel and several other SMSCs send — were rejected outright. They are now parsed. +- 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. +- Short or malformed PDUs threw out of the codec instead of being reported as a parse failure. ## Development diff --git a/todo.md b/todo.md index 86872a7..b1b5083 100644 --- a/todo.md +++ b/todo.md @@ -107,7 +107,8 @@ smpp.on('session', session => { - **Named exports only.** No default export. `defs` stays as a grouped export alongside the individual tables (`cmds`, `consts`, `encodings`, `errors`, `tlvs`, `types`, and the `*ById` maps). - **`utils` is gone.** Its contents are named exports: `bitCount`, `decodeMessage`, `encodeMessage`, - `objToPdu`, `pduReturn`, `pduToObj`, `smppDate`, `splitMessage`. + `objToPdu`, `pduReturn`, `pduToObj`, `smppDate`, `smppTime`, `splitMessage`. +- **`defs.filters` is gone** — it never did anything. `smppTime` replaces the one useful part. - **The PDU codec is synchronous** and returns `{ err?, pduObj? }` / `{ err?, buffer? }`. - **Low-level surface stays public**, including `session.sock`, `session.send()` and `session.sendReturn()` — that is how the other 29 commands are reached. @@ -117,29 +118,32 @@ smpp.on('session', session => { ### 1. Definition tables — `src/defs/` +All done, with tests in `test/encodings.test.ts`, `test/types.test.ts` and `test/commands.test.ts`. + - [x] `constants.ts` — `consts` + `constsById`. -- [x] `errors.ts` — all `ESME_*` codes plus `errorsById`. -- [ ] `types.ts` — wire types `int8`, `int16`, `int32`, `string`, `cstring`, `buffer`, - `dest_address_array`, `unsuccess_sme_array`, and the `tlv` variants. Each is - `{ default, read(buffer, offset, length?), size(value), write(value, buffer, offset) }`. - `read` must be bounds-checked and return a result rather than throwing. -- [ ] `encodings.ts` — GSM 03.38 (`ASCII`), `LATIN1`, `UCS2`, `FLASH` (alias of ASCII) and `detect`. - Drop `iconv-lite`: LATIN1 is `Buffer.from(str, 'latin1')`, UCS2 is `Buffer.from(str, - 'utf16le').swap16()` (copy before swapping, and reject odd-length input on decode). -- [ ] `filters.ts` — `time`, `message`, `billing_identification`, `broadcast_area_identifier`, - `broadcast_content_type`, `broadcast_frequency_interval`, `callback_num`, `callback_num_atag`. -- [ ] `tlvs.ts` — the 67 TLV definitions plus `tlvsById` and the two aliases - (`alert_on_msg_delivery`, `failed_broadcast_area_identifier`). -- [ ] `commands.ts` — all 33 commands with ids and **wire-ordered** parameter lists, plus `cmdsById`. +- [x] `errors.ts` — all `ESME_*` codes plus `errorsById`, `isErrorName`, `errorNameById`. +- [x] `types.ts` — wire types. `read` is bounds-checked and reports `bytesRead`, so no caller has to + re-derive a length that could disagree with what was written. `size` and `write` validate their + input and return results. +- [x] `encodings.ts` — GSM 03.38, LATIN1, UCS2, FLASH, `detect` and `encodingByDataCoding`. + `iconv-lite` is gone; Buffer does it natively. +- [x] `tlvs.ts` — 67 TLV definitions, `tlvsById`, and the two aliases. +- [x] `commands.ts` — all 33 commands, wire-ordered, plus `cmdsById` and the `PduParams` / + `PduParamsInput` per-command types. +- [x] `filters.ts` — **not ported.** `defs.filters` was declared on commands and TLVs but never + invoked anywhere in 0.4.0. The one piece that is genuinely needed, SMPP time formatting, is + task 2's `smppTime`. ### 2. Message helpers — `src/message.ts` - [ ] `bitCount(msg, encoding?)`, `encodeMessage`, `decodeMessage`, `smppDate`, `splitMessage`. +- [ ] `smppTime.encode(value)` / `smppTime.decode(value)` — absolute and relative SMPP time formats, + replacing the dormant `filters.time`. Used by `validityPeriod` and `scheduleDeliveryTime`. - [ ] **Fix:** segments are 134 GSM characters or 67 UCS2 characters, so that segment + 6-byte UDH is exactly 140 octets. 0.4.0 produces 152/66. - [ ] **Fix:** `smppDate` must add 1 to `getMonth()` and zero-pad correctly. -- [ ] **Fix:** LATIN1 must actually decode — resolve `data_coding` to a concrete encoding, not to - whichever alias happens to sort last. +- [ ] **Fix:** `decodeMessage` must resolve the alphabet through `encodingByDataCoding` (already + written and tested) rather than scanning the alias table, which is what broke LATIN1. - [ ] The concatenation reference counter is per session, not module-global. `splitMessage` therefore takes the reference as an argument instead of owning a counter. @@ -148,6 +152,8 @@ smpp.on('session', session => { - [ ] `pduToObj(buffer)` → `{ err?, pduObj? }`, `objToPdu(obj)` → `{ err?, buffer? }`, `pduReturn(pdu, status?, params?, tlvs?)` → `{ err?, buffer? }`, `isResp(pduObj)`. - [ ] Keep the trailing-NULL-octet retry for `short_message` that 0.4.0 has — real peers send it. + It is now an explicit decision in the parser: `types.buffer.size()` no longer silently drops a + trailing `0x00`, because that corrupted every UCS2 message ending in one (see AGENTS.md). - [ ] Guard `cmdLength` against a maximum before allocating, so a hostile peer cannot ask for a 4 GiB buffer. 0.4.0 has no such guard. - [ ] Per-command typed params: `pduToObj` returns a union discriminated on `cmdName`, and