diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b74c7c..009b357 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,7 +66,9 @@ **A TLV input is keyed by its name, or by its decimal id where the table names none, and `tagId` is refused.** Write `{ 5142: { tagValue } }` for a vendor tag, not `{ vendor: { tagId: 5142, … } }`. A name and a `tagId` could disagree, and `{ message_state: { tagId: 5, … } }` went out as tag 5. - A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name. + A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and + so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of + `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index b21f6af..3d01adc 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,7 +32,8 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. - **A TLV is keyed by its name, or by its decimal id where the table names none**, and `tagId` on an input is refused: `{ 5142: { tagValue } }`, not `{ vendor: { tagId: 5142, tagValue } }`. - Unknown tags read back under the same decimal key. + Write `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back + under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`. - **The `error` event is `sessionError`**, and `serverError` on the server handle. - **`log`** takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods instead of a `larvitutils` one, and is silent by default: [README](README.md#logging). diff --git a/docs/decisions.md b/docs/decisions.md index b77ffaa..c48d82c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -434,9 +434,7 @@ rule and an index of the titles below. `writeParams()` ignores it, so an empty one, an absent one and one the wire cannot carry are the same input rather than three. A `data_coding` on a command that declares no such field is honoured the other way round, since it is `replace_sm`'s only way to name the alphabet its octets are in. - Every entry carrying the payload tag is resolved, by tag id rather - than by record key, since `tagIdOf()` lets a caller name it anything and a spelling that escaped - the guard would be a second spelling that disagrees about correctness. Rejected: refusing a string + Rejected: refusing a string `message_payload` outright and demanding octets, which contradicts `short_message` on the same PDU. Rejected: guarding every string-valued field against `data_coding`, which says nothing about them — a text field on the wire has an diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index c0eb97b..b3c6dcb 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -110,7 +110,7 @@ type ReadValue = Repeated>; type WriteValue = Specs[K] extends { multiple: true } ? ReadValue : WireValue extends number ? number : WireValue extends string ? number | string - : Buffer | number | string; + : Buffer | string; type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; @@ -122,22 +122,34 @@ export type Tlvs = { [K in TlvName]?: KnownTlv } & Record<`${number}`, Unknow export type Tlv = { [K in TlvName]: KnownTlv }[TlvName] | UnknownTlv; -type Alternate = keyof typeof alternates; - -type Canonical = K extends Alternate ? (typeof alternates)[K]['tag'] : K; - /** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ -export type TlvInputs = { [K in Alternate | TlvName]?: { tagValue: WriteValue> } } +export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } & Record<`${number}`, { tagValue: Buffer | number | string }>; -function tagIdOf(name: string, input: object): Result<{ tagId: number }> { +function isTlvInput(input: unknown): input is { tagValue: TlvValue } { + if (typeof input !== 'object' || input === null || !('tagValue' in input)) return false; + + const value = input.tagValue; + + if (!Array.isArray(value)) return Buffer.isBuffer(value) || typeof value === 'number' || typeof value === 'string'; + + return value.every(one => Buffer.isBuffer(one)) || value.every(one => typeof one === 'number'); +} + +function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { + if (!isTlvInput(input)) { + return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; + } + if ('tagId' in input) { return { err: new Error(`TLV "${name}": key it by its name, or a tag the table does not define by its decimal id, instead of giving a tagId`) }; } - const named = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + if (isTlvName(name)) return { tagId: specs[name].id, tagValue: input.tagValue }; - if (named) return { tagId: named.id }; + const alternate = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + + if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate.tag}, the name it reads back under`) }; if (!/^(0|[1-9]\d*)$/.test(name)) { return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; @@ -149,20 +161,22 @@ function tagIdOf(name: string, input: object): Result<{ tagId: number }> { const known = tlvsById[tagId]; - return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; + return known + ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } + : { tagId, tagValue: input.tagValue }; } /** Each TLV as its four octet header and the value the tag's own wire type writes. */ export function writeTlvs(inputs: TlvInputs | undefined): Result<{ chunks: Buffer[] }> { const chunks: Buffer[] = []; - for (const [name, input] of Object.entries(inputs ?? {})) { - const tag = tagIdOf(name, input); + for (const [name, input] of Object.entries(inputs ?? {})) { + const tag = entryOf(name, input); if (tag.err) return { err: tag.err }; const definition = tlvsById[tag.tagId]; - const values = occurrences(input.tagValue, definition?.multiple === true); + const values = occurrences(tag.tagValue, definition?.multiple === true); if (values.err) return { err: new Error(`TLV "${name}": ${values.err.message}`) }; diff --git a/test/pdu.test.ts b/test/pdu.test.ts index 5e6388d..3fb637a 100644 --- a/test/pdu.test.ts +++ b/test/pdu.test.ts @@ -392,7 +392,7 @@ describe('TLVs', () => { assert.deepEqual(unknown.tagValue, Buffer.from('blajfoo', 'ascii')); }); - test('keeps a binary TLV byte for byte through pduToObj and back', () => { + test('keeps a binary TLV byte for byte through pduToObj and back, copied off the chunk it arrived in', () => { const payload = Buffer.from('deadbeef00ff', 'hex'); const params = { destination_addr: '46709771337', @@ -400,16 +400,21 @@ describe('TLVs', () => { short_message: 'binary payload follows', source_addr: '46701113311', }; - const parsed = decode(encode({ + const pdu = encode({ cmdName: 'deliver_sm', params, seqNr: 7, tlvs: { message_payload: { tagValue: payload } }, - })); - const carried = parsed.tlvs.message_payload; + }); + const chunk = Buffer.alloc(64 * 1024); + + pdu.copy(chunk, 100); + + const carried = decode(chunk.subarray(100, 100 + pdu.length)).tlvs.message_payload; assert.ok(carried); assert.deepEqual(carried.tagValue, payload); + assert.notEqual(carried.tagValue.buffer, chunk.buffer, 'a TLV holds its own octets, not the chunk it arrived in'); const rebuilt = decode(encode({ cmdName: 'deliver_sm', @@ -442,6 +447,8 @@ describe('TLVs', () => { { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 65536: { tagValue: 'blajfoo' } } }), reason: /out of range/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { '05142': { tagValue: 'blajfoo' } } }), reason: /decimal id/ }, { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { 1063: { tagValue: 2 } } }), reason: /message_state/ }, + // @ts-expect-error the value goes in a { tagValue } wrapper + { built: objToPdu({ cmdName: 'deliver_sm', params, tlvs: { message_state: 2 } }), reason: /give it as \{ tagValue \}/ }, ]; for (const { built: { buffer, err }, reason } of refusals) { @@ -481,15 +488,17 @@ describe('TLVs', () => { assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]); }); - test('reads the failed areas of a broadcast_sm_resp as broadcast_area_identifier', () => { + test('writes and reads the failed areas of a broadcast_sm_resp as broadcast_area_identifier', () => { const areas = [Buffer.from('0001', 'hex'), Buffer.from('0002', 'hex')]; - const pduObj = decode(encode({ - cmdName: 'broadcast_sm_resp', - params: { message_id: '01a0d051-b588-76eb-a5c5-a8cb8b854e68' }, - tlvs: { failed_broadcast_area_identifier: { tagValue: areas } }, - })); + const params = { message_id: '01a0d051-b588-76eb-a5c5-a8cb8b854e68' }; + const pduObj = decode(encode({ cmdName: 'broadcast_sm_resp', params, tlvs: { broadcast_area_identifier: { tagValue: areas } } })); assert.deepEqual(pduObj.tlvs.broadcast_area_identifier?.tagValue, areas); + + // @ts-expect-error the alternate spelling is read back under the name, so only the name is written + const { err } = objToPdu({ cmdName: 'broadcast_sm_resp', params, tlvs: { failed_broadcast_area_identifier: { tagValue: areas } } }); + + assert.match(err?.message ?? '', /key it broadcast_area_identifier/); }); test('refuses a repeatable TLV given one value, and a lone TLV given several', () => { @@ -544,6 +553,8 @@ describe('TLVs', () => { assert.deepEqual([id, state, unknown], ['0199d8a4-5e2c-7b3f-9a61-c4e07f2d8b15', 2, Buffer.from('01', 'hex')]); + // @ts-expect-error an octet field takes no number, which would go out as its digits + assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { network_error_code: { tagValue: 5 } } })); // @ts-expect-error message_state is an integer assert.ok(objToPdu({ cmdName: 'deliver_sm', params: {}, tlvs: { message_state: { tagValue: 'ENROUTE' } } }).err); });