Record wire-format defects and drop the dormant filters layer

This commit is contained in:
2026-08-25 16:20:25 +02:00
parent 4299a8cc35
commit 79c9df79c6
3 changed files with 41 additions and 22 deletions
+12 -5
View File
@@ -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. 3. **No `error` event.** Node makes an unhandled `error` event throw, which would break rule 1.
Sessions emit `sessionError`, servers emit `serverError`. Sessions emit `sessionError`, servers emit `serverError`.
4. **No casts, no non-null assertions.** `as`, `as unknown as` and `!` are all banned. Parse untyped 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 ## Architecture
@@ -40,12 +42,12 @@ src/
sms.ts The live handle emitted as the 'sms' event (sendResp/sendDlr) sms.ts The live handle emitted as the 'sms' event (sendResp/sendDlr)
message.ts Encoding detection, splitting, bit counting, SMPP date formatting message.ts Encoding detection, splitting, bit counting, SMPP date formatting
pdu.ts pduToObj / objToPdu / pduReturn — synchronous, result-returning pdu.ts pduToObj / objToPdu / pduReturn — synchronous, result-returning
result.ts Result<T> — the shape every fallible call returns
defs/ defs/
commands.ts The 33 commands, their ids and ordered parameter lists commands.ts The 33 commands, their ids and ordered parameter lists
constants.ts consts + constsById (TON, NPI, ENCODING, MESSAGE_STATE, …) 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_*) errors.ts errors + errorsById (ESME_*)
filters.ts Per-field encode/decode hooks (time, message, callback_num, …)
tlvs.ts TLV definitions, tlvsById tlvs.ts TLV definitions, tlvsById
types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays 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 | | 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 | | 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 | | 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 ## Conventions
- Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant - Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant.
(see the wire-order note above). 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 - Comments are the exception, not the default — see the root `CLAUDE.md` rules. Do not write file
preambles or restate what the code says. preambles or restate what the code says.
- Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders. - Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders.
+7 -1
View File
@@ -166,9 +166,11 @@ commands the codec knows, not just the four the session handles natively.
`{ userData }`. `{ userData }`.
- **Renamed options:** `enqLinkTiming``enquireLinkInterval`, server `timeout``idleTimeout`. - **Renamed options:** `enqLinkTiming``enquireLinkInterval`, server `timeout``idleTimeout`.
- **`larvitsmpp.utils` is gone.** Its contents are named exports: `bitCount`, `decodeMessage`, - **`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 }`. synchronous and returns `{ err, pduObj }` / `{ err, buffer }`.
- **`pduObj.isResp()` is now the standalone `isResp(pduObj)`.** - **`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). - **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 - **`log`** takes a [`@larvit/log`](https://www.npmjs.com/package/@larvit/log) instance instead of a
`larvitutils` one, and is silent by default. `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. - 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 - 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. 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 ## Development
+22 -16
View File
@@ -107,7 +107,8 @@ smpp.on('session', session => {
- **Named exports only.** No default export. `defs` stays as a grouped export alongside the - **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). individual tables (`cmds`, `consts`, `encodings`, `errors`, `tlvs`, `types`, and the `*ById` maps).
- **`utils` is gone.** Its contents are named exports: `bitCount`, `decodeMessage`, `encodeMessage`, - **`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? }`. - **The PDU codec is synchronous** and returns `{ err?, pduObj? }` / `{ err?, buffer? }`.
- **Low-level surface stays public**, including `session.sock`, `session.send()` and - **Low-level surface stays public**, including `session.sock`, `session.send()` and
`session.sendReturn()` — that is how the other 29 commands are reached. `session.sendReturn()` — that is how the other 29 commands are reached.
@@ -117,29 +118,32 @@ smpp.on('session', session => {
### 1. Definition tables — `src/defs/` ### 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] `constants.ts``consts` + `constsById`.
- [x] `errors.ts` — all `ESME_*` codes plus `errorsById`. - [x] `errors.ts` — all `ESME_*` codes plus `errorsById`, `isErrorName`, `errorNameById`.
- [ ] `types.ts` — wire types `int8`, `int16`, `int32`, `string`, `cstring`, `buffer`, - [x] `types.ts` — wire types. `read` is bounds-checked and reports `bytesRead`, so no caller has to
`dest_address_array`, `unsuccess_sme_array`, and the `tlv` variants. Each is re-derive a length that could disagree with what was written. `size` and `write` validate their
`{ default, read(buffer, offset, length?), size(value), write(value, buffer, offset) }`. input and return results.
`read` must be bounds-checked and return a result rather than throwing. - [x] `encodings.ts` — GSM 03.38, LATIN1, UCS2, FLASH, `detect` and `encodingByDataCoding`.
- [ ] `encodings.ts` — GSM 03.38 (`ASCII`), `LATIN1`, `UCS2`, `FLASH` (alias of ASCII) and `detect`. `iconv-lite` is gone; Buffer does it natively.
Drop `iconv-lite`: LATIN1 is `Buffer.from(str, 'latin1')`, UCS2 is `Buffer.from(str, - [x] `tlvs.ts` — 67 TLV definitions, `tlvsById`, and the two aliases.
'utf16le').swap16()` (copy before swapping, and reject odd-length input on decode). - [x] `commands.ts` — all 33 commands, wire-ordered, plus `cmdsById` and the `PduParams<C>` /
- [ ] `filters.ts``time`, `message`, `billing_identification`, `broadcast_area_identifier`, `PduParamsInput<C>` per-command types.
`broadcast_content_type`, `broadcast_frequency_interval`, `callback_num`, `callback_num_atag`. - [x] `filters.ts`**not ported.** `defs.filters` was declared on commands and TLVs but never
- [ ] `tlvs.ts` — the 67 TLV definitions plus `tlvsById` and the two aliases invoked anywhere in 0.4.0. The one piece that is genuinely needed, SMPP time formatting, is
(`alert_on_msg_delivery`, `failed_broadcast_area_identifier`). task 2's `smppTime`.
- [ ] `commands.ts` — all 33 commands with ids and **wire-ordered** parameter lists, plus `cmdsById`.
### 2. Message helpers — `src/message.ts` ### 2. Message helpers — `src/message.ts`
- [ ] `bitCount(msg, encoding?)`, `encodeMessage`, `decodeMessage`, `smppDate`, `splitMessage`. - [ ] `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 - [ ] **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. is exactly 140 octets. 0.4.0 produces 152/66.
- [ ] **Fix:** `smppDate` must add 1 to `getMonth()` and zero-pad correctly. - [ ] **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 - [ ] **Fix:** `decodeMessage` must resolve the alphabet through `encodingByDataCoding` (already
whichever alias happens to sort last. 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 - [ ] The concatenation reference counter is per session, not module-global. `splitMessage` therefore
takes the reference as an argument instead of owning a counter. 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? }`, - [ ] `pduToObj(buffer)``{ err?, pduObj? }`, `objToPdu(obj)``{ err?, buffer? }`,
`pduReturn(pdu, status?, params?, tlvs?)``{ err?, buffer? }`, `isResp(pduObj)`. `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. - [ ] 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 - [ ] 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. buffer. 0.4.0 has no such guard.
- [ ] Per-command typed params: `pduToObj` returns a union discriminated on `cmdName`, and - [ ] Per-command typed params: `pduToObj` returns a union discriminated on `cmdName`, and