WIP: Read and write every copy of a TLV SMPP lets repeat, and drop the dormant tlvMap #26

Closed
lilleman wants to merge 3 commits from tlv-multiple into main
12 changed files with 120 additions and 52 deletions
Showing only changes of commit 224acca7d0 - Show all commits
+5
View File
@@ -37,6 +37,11 @@
response command`. response command`.
- `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its
other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message.
- `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and
`broadcast_error_status` read as a list of every copy the peer sent, where only the last was kept,
and are written from a list. A single value for one of them, or a list for any other tag, is
refused.
- `tlvMap` is gone from the `broadcast_sm_resp` command definition. Nothing read it.
## 0.5.0 ## 0.5.0
+3
View File
@@ -70,6 +70,9 @@ have for these:
- Binary TLVs (`message_payload`, `network_error_code`, `callback_num` and the rest) were parsed into - 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 round trip corrupted them. a hex string and written back as the ASCII of that string, so every round trip corrupted them.
They are `Buffer`s in both directions now; drop any hex encoding of your own. They are `Buffer`s in both directions now; drop any hex encoding of your own.
- A repeated `callback_num`, `callback_num_atag`, `callback_num_pres_ind`,
`broadcast_area_identifier` or `broadcast_error_status` kept only its last copy. Each of those tags
is a list now, read and written, even where one copy arrives.
- A body carried in the `message_payload` TLV was ignored, so the message arrived empty, and a - A body carried in the `message_payload` TLV was ignored, so the message arrived empty, and a
`data_sm` was answered `ESME_RINVCMDID`, so a receipt thrown on one was lost silently. Both reach `data_sm` was answered `ESME_RINVCMDID`, so a receipt thrown on one was lost silently. Both reach
the application now: a receipt as `dlr`, answered for you, and a message as `sms` for you to answer. the application now: a receipt as `dlr`, answered for you, and a message as `sms` for you to answer.
+2
View File
@@ -608,6 +608,8 @@ if (isCommand(pduObj, 'submit_sm')) {
and hands back the UDH where the PDU carries one. and hands back the UDH where the PDU carries one.
- `concatOf(pduObj)`: the `part`, `total` and `reference` a PDU declares and the `spelling` that - `concatOf(pduObj)`: the `part`, `total` and `reference` a PDU declares and the `spelling` that
carried them, `'udh'` or `'sar'`, or `undefined` for a whole message. carried them, `'udh'` or `'sar'`, or `undefined` for a whole message.
- A tag SMPP lets repeat, marked `multiple` in `tlvs` (`callback_num`, `broadcast_area_identifier`
and three more), is a list of every copy in wire order, and `objToPdu()` takes it as one.
- `messageClassOf(dataCoding)`: `0` for the flash class, `1`, `2` and `3` for the ME-, SIM- and - `messageClassOf(dataCoding)`: `0` for the flash class, `1`, `2` and `3` for the ME-, SIM- and
TE-specific ones, `undefined` where that `data_coding`'s coding group carries no class. TE-specific ones, `undefined` where that `data_coding`'s coding group carries no class.
-2
View File
@@ -4,7 +4,6 @@ import { buffer, cstring, dest_address_array, int8, unsuccess_sme_array } from '
type CommandSpec = { type CommandSpec = {
id: number; id: number;
params?: Record<string, WireType>; params?: Record<string, WireType>;
tlvMap?: Record<string, string>;
}; };
const bindParams = { const bindParams = {
@@ -59,7 +58,6 @@ const specs = {
broadcast_sm_resp: { broadcast_sm_resp: {
id: 0x80000111, id: 0x80000111,
params: { message_id: cstring }, params: { message_id: cstring },
tlvMap: { broadcast_area_identifier: 'failed_broadcast_area_identifier' },
}, },
cancel_broadcast_sm: { cancel_broadcast_sm: {
id: 0x00000113, id: 0x00000113,
+61 -27
View File
@@ -1,16 +1,19 @@
import type { ParamValue, WireType } from './types.ts'; import type { WireType } from './types.ts';
import type { Result } from '../result.ts'; import type { Result } from '../result.ts';
import { tlv } from './types.ts'; import { tlv } from './types.ts';
/** What one TLV on the wire reads as. */
export type TlvScalar = Buffer | number | string;
export type TlvDefinition = { export type TlvDefinition = {
id: number; id: number;
multiple?: boolean; multiple?: boolean;
tag: string; tag: string;
type: WireType; type: WireType<TlvScalar>;
}; };
/** The constraint keys every definition to its own name, so a `tag` that drifts fails to compile. */ /** The constraint keys every definition to its own name, so a `tag` that drifts fails to compile. */
const tlvSpecs = <T extends { [K in keyof T]: { id: number; multiple?: boolean; tag: K; type: WireType } }>( const tlvSpecs = <T extends { [K in keyof T]: { id: number; multiple?: boolean; tag: K; type: WireType<TlvScalar> } }>(
definitions: T, definitions: T,
): T => definitions; ): T => definitions;
@@ -98,18 +101,21 @@ for (const definition of Object.values<TlvDefinition>(specs)) {
} }
/** Fallback for tags this table does not know: keep the raw octets. */ /** Fallback for tags this table does not know: keep the raw octets. */
export const tlvDefault: WireType = tlv.buffer; export const tlvDefault: WireType<TlvScalar> = tlv.buffer;
/** A list for a tag its definition marks `multiple`, one copy per list entry, and a single value otherwise. */
export type TlvValue = TlvScalar | TlvScalar[];
export type Tlv = { export type Tlv = {
tagId: number; tagId: number;
tagName: string | undefined; tagName: string | undefined;
tagValue: ParamValue; tagValue: TlvValue;
}; };
export type TlvInput = { export type TlvInput = {
/** Resolved from the record key; pass it for a tag the TLV table does not define. */ /** Resolved from the record key; pass it for a tag the TLV table does not define. */
tagId?: number | undefined; tagId?: number | undefined;
tagValue: ParamValue; tagValue: TlvValue;
}; };
export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> { export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }> {
@@ -126,6 +132,45 @@ export function tagIdOf(name: string, input: TlvInput): Result<{ tagId: number }
return { tagId }; return { tagId };
} }
function copiesOf(name: string, definition: TlvDefinition | undefined, value: TlvValue): Result<{ copies: TlvScalar[] }> {
const multiple = definition?.multiple === true;
if (Array.isArray(value) && multiple) return { copies: value };
if (!Array.isArray(value) && !multiple) return { copies: [value] };
return {
err: new Error(multiple
? `TLV "${name}": the tag may repeat, so give its value as a list`
: `TLV "${name}": the tag may not repeat, so give its value alone rather than as a list`),
};
}
function writeTlv(name: string, tagId: number, type: WireType<TlvScalar>, value: TlvScalar): Result<{ chunk: Buffer }> {
const sized = type.size(value);
if (sized.err) {
return { err: new Error(`TLV "${name}": ${sized.err.message}`) };
}
if (sized.size > 0xffff) {
return { err: new Error(`TLV "${name}": ${String(sized.size)} octets overflow the two octet length`) };
}
const chunk = Buffer.alloc(sized.size + 4);
chunk.writeUInt16BE(tagId, 0);
chunk.writeUInt16BE(sized.size, 2);
const written = type.write(value, chunk, 4);
if (written.err) {
return { err: new Error(`TLV "${name}": ${written.err.message}`) };
}
return { chunk };
}
/** Each TLV as its four octet header and the value the tag's own wire type writes. */ /** Each TLV as its four octet header and the value the tag's own wire type writes. */
export function writeTlvs(inputs: Record<string, TlvInput> | undefined): Result<{ chunks: Buffer[] }> { export function writeTlvs(inputs: Record<string, TlvInput> | undefined): Result<{ chunks: Buffer[] }> {
const chunks: Buffer[] = []; const chunks: Buffer[] = [];
@@ -135,29 +180,18 @@ export function writeTlvs(inputs: Record<string, TlvInput> | undefined): Result<
if (tag.err) return { err: tag.err }; if (tag.err) return { err: tag.err };
const type = tlvsById[tag.tagId]?.type ?? tlvDefault; const definition = tlvsById[tag.tagId];
const sized = type.size(input.tagValue); const listed = copiesOf(name, definition, input.tagValue);
if (sized.err) { if (listed.err) return { err: listed.err };
return { err: new Error(`TLV "${name}": ${sized.err.message}`) };
for (const value of listed.copies) {
const written = writeTlv(name, tag.tagId, definition?.type ?? tlvDefault, value);
if (written.err) return { err: written.err };
chunks.push(written.chunk);
} }
if (sized.size > 0xffff) {
return { err: new Error(`TLV "${name}": ${String(sized.size)} octets overflow the two octet length`) };
}
const chunk = Buffer.alloc(sized.size + 4);
chunk.writeUInt16BE(tag.tagId, 0);
chunk.writeUInt16BE(sized.size, 2);
const written = type.write(input.tagValue, chunk, 4);
if (written.err) {
return { err: new Error(`TLV "${name}": ${written.err.message}`) };
}
chunks.push(chunk);
} }
return { chunks }; return { chunks };
+4 -4
View File
@@ -1,7 +1,7 @@
import type { MessageState } from './defs/constants.ts'; import type { MessageState } from './defs/constants.ts';
import type { ParamValue } from './defs/types.ts';
import type { PduObject } from './pdu.ts'; import type { PduObject } from './pdu.ts';
import type { SmsIdFormat } from './sms-id.ts'; import type { SmsIdFormat } from './sms-id.ts';
import type { TlvValue } from './defs/tlvs.ts';
import { consts, constsById, hasUdh, messageTypeOf } from './defs/constants.ts'; import { consts, constsById, hasUdh, messageTypeOf } from './defs/constants.ts';
import { encodings } from './defs/encodings.ts'; import { encodings } from './defs/encodings.ts';
import { messageOctets } from './message-body.ts'; import { messageOctets } from './message-body.ts';
@@ -150,7 +150,7 @@ const smeMessageTypes: readonly number[] = [
consts.ESM_CLASS.USER_ACKNOWLEDGEMENT, consts.ESM_CLASS.USER_ACKNOWLEDGEMENT,
]; ];
function nonEmptyText(value: ParamValue | undefined): string | undefined { function nonEmptyText(value: TlvValue | undefined): string | undefined {
return typeof value === 'string' && value !== '' ? value : undefined; return typeof value === 'string' && value !== '' ? value : undefined;
} }
@@ -178,7 +178,7 @@ function receiptBody(pduObj: PduObject): string {
} }
function receiptId( function receiptId(
tlvId: ParamValue | undefined, tlvId: TlvValue | undefined,
receipt: Receipt | undefined, receipt: Receipt | undefined,
format: SmsIdFormat, format: SmsIdFormat,
): string | undefined { ): string | undefined {
@@ -198,7 +198,7 @@ function isMessageState(name: string | undefined): name is MessageState {
/** The state TLV wins where it names a state we know; an unnameable one leaves the body to say. */ /** The state TLV wins where it names a state we know; an unnameable one leaves the body to say. */
function receiptStatus( function receiptStatus(
tlvState: ParamValue | undefined, tlvState: TlvValue | undefined,
receipt: Receipt | undefined, receipt: Receipt | undefined,
): { statusId: number; statusMsg: MessageState | undefined } { ): { statusId: number; statusMsg: MessageState | undefined } {
const scraped = receiptStates[receipt?.stat?.toUpperCase() ?? '']; const scraped = receiptStates[receipt?.stat?.toUpperCase() ?? ''];
+1 -1
View File
@@ -67,7 +67,7 @@ export type { ErrorName } from './defs/errors.ts';
export type { PduObject, PduObjectInput, TlvInput } from './pdu.ts'; export type { PduObject, PduObjectInput, TlvInput } from './pdu.ts';
export type { PduHeader } from './pdu-refusal.ts'; export type { PduHeader } from './pdu-refusal.ts';
export type { SplitOptions } from './message.ts'; export type { SplitOptions } from './message.ts';
export type { Tlv, TlvDefinition, TlvName } from './defs/tlvs.ts'; export type { Tlv, TlvDefinition, TlvName, TlvScalar, TlvValue } from './defs/tlvs.ts';
export type { DestAddress, ParamValue, UnsuccessSme, WireType } from './defs/types.ts'; export type { DestAddress, ParamValue, UnsuccessSme, WireType } from './defs/types.ts';
/** The spec tables, grouped the way `larvitsmpp.defs` was in 0.4.0. */ /** The spec tables, grouped the way `larvitsmpp.defs` was in 0.4.0. */
+11 -6
View File
@@ -3,7 +3,7 @@ import type { ErrorName } from './defs/errors.ts';
import type { ParamValue } from './defs/types.ts'; import type { ParamValue } from './defs/types.ts';
import type { PduHeader } from './pdu-refusal.ts'; import type { PduHeader } from './pdu-refusal.ts';
import type { Result, VoidResult } from './result.ts'; import type { Result, VoidResult } from './result.ts';
import type { Tlv, TlvInput } from './defs/tlvs.ts'; import type { Tlv, TlvDefinition, TlvInput, TlvScalar, TlvValue } from './defs/tlvs.ts';
import { PduRefusedError, framingRefusal } from './pdu-refusal.ts'; import { PduRefusedError, framingRefusal } from './pdu-refusal.ts';
import { cmds, commandNameById, respNameFor } from './defs/commands.ts'; import { cmds, commandNameById, respNameFor } from './defs/commands.ts';
import { hasUdh } from './defs/constants.ts'; import { hasUdh } from './defs/constants.ts';
@@ -262,6 +262,13 @@ export function objToPdu<C extends CommandName>(obj: PduObjectInput<C>): Result<
); );
} }
/** A repeatable tag gathers every copy the peer sent; any other keeps the last. */
function tlvValue(definition: TlvDefinition | undefined, earlier: Tlv | undefined, value: TlvScalar): TlvValue {
if (definition?.multiple !== true) return value;
return [...Array.isArray(earlier?.tagValue) ? earlier.tagValue : [], value];
}
function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record<string, Tlv> }> { function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: Record<string, Tlv> }> {
const tlvs: Record<string, Tlv> = {}; const tlvs: Record<string, Tlv> = {};
let offset = start; let offset = start;
@@ -279,11 +286,9 @@ function parseTlvs(pdu: Buffer, start: number): Result<{ offset: number; tlvs: R
if (read.err) return { err: read.err }; if (read.err) return { err: read.err };
tlvs[definition?.tag ?? tagId.toString()] = { const name = definition?.tag ?? tagId.toString();
tagId,
tagName: definition?.tag, tlvs[name] = { tagId, tagName: definition?.tag, tagValue: tlvValue(definition, tlvs[name], read.value) };
tagValue: read.value,
};
offset += 4 + tagLength; offset += 4 + tagLength;
} }
+7 -4
View File
@@ -2,7 +2,7 @@ import type { Concat } from './concat.ts';
import type { ParamValue } from './defs/types.ts'; import type { ParamValue } from './defs/types.ts';
import type { PduObject } from './pdu.ts'; import type { PduObject } from './pdu.ts';
import type { SmppLog } from './log.ts'; import type { SmppLog } from './log.ts';
import type { Tlv } from './defs/tlvs.ts'; import type { Tlv, TlvScalar } from './defs/tlvs.ts';
import { ExpiringGroups } from './expiring-groups.ts'; import { ExpiringGroups } from './expiring-groups.ts';
import { decodeMessage } from './message.ts'; import { decodeMessage } from './message.ts';
import { messageOctets } from './message-body.ts'; import { messageOctets } from './message-body.ts';
@@ -52,6 +52,10 @@ type Group = {
total: number; total: number;
}; };
function copied(value: TlvScalar): TlvScalar {
return Buffer.isBuffer(value) ? Buffer.from(value) : value;
}
/** Wire reads hand back views, so retaining one segment would pin the whole PDU it arrived in. */ /** Wire reads hand back views, so retaining one segment would pin the whole PDU it arrived in. */
function detach(pduObj: PduObject): PduObject { function detach(pduObj: PduObject): PduObject {
const params: Record<string, ParamValue> = {}; const params: Record<string, ParamValue> = {};
@@ -62,9 +66,7 @@ function detach(pduObj: PduObject): PduObject {
} }
for (const [name, tlv] of Object.entries(pduObj.tlvs)) { for (const [name, tlv] of Object.entries(pduObj.tlvs)) {
tlvs[name] = Buffer.isBuffer(tlv.tagValue) tlvs[name] = { ...tlv, tagValue: Array.isArray(tlv.tagValue) ? tlv.tagValue.map(copied) : copied(tlv.tagValue) };
? { ...tlv, tagValue: Buffer.from(tlv.tagValue) }
: tlv;
} }
// short_message holds the same octets wherever it was not decoded, so one copy covers both. // short_message holds the same octets wherever it was not decoded, so one copy covers both.
@@ -78,6 +80,7 @@ function detach(pduObj: PduObject): PduObject {
// A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU.
function sizeOf(value: unknown): number { function sizeOf(value: unknown): number {
if (Buffer.isBuffer(value)) return value.length; if (Buffer.isBuffer(value)) return value.length;
if (Array.isArray(value)) return value.reduce<number>((sum, copy) => sum + sizeOf(copy), 0);
return typeof value === 'string' ? value.length : 0; return typeof value === 'string' ? value.length : 0;
} }
+25
View File
@@ -420,6 +420,31 @@ describe('TLVs', () => {
assert.deepEqual(rebuilt.tlvs.message_payload?.tagValue, payload); assert.deepEqual(rebuilt.tlvs.message_payload?.tagValue, payload);
}); });
test('reads every copy of a tag SMPP lets repeat, in wire order, and writes them back', () => {
const numbers = [Buffer.from('46701113311', 'latin1'), Buffer.from('46709771337', 'latin1')];
const pduObj = decode(encode({
cmdName: 'submit_sm',
params: { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' },
tlvs: { callback_num: { tagValue: numbers }, callback_num_pres_ind: { tagValue: [1] } },
}));
assert.deepEqual(pduObj.tlvs.callback_num, { tagId: 0x0381, tagName: 'callback_num', tagValue: numbers });
assert.deepEqual(pduObj.tlvs.callback_num_pres_ind?.tagValue, [1]);
});
test('refuses a single value for a tag SMPP lets repeat, and a list for one it does not', () => {
const params = { destination_addr: '46709771337', short_message: 'hi', source_addr: '46701113311' };
const single = objToPdu({
cmdName: 'submit_sm',
params,
tlvs: { callback_num: { tagValue: Buffer.from('46701113311', 'latin1') } },
});
const list = objToPdu({ cmdName: 'submit_sm', params, tlvs: { source_port: { tagValue: [1234] } } });
assert.match(single.err?.message ?? '', /TLV "callback_num": .*list/);
assert.match(list.err?.message ?? '', /TLV "source_port": .*list/);
});
test('takes the tag id from the record key when the caller gives none', () => { test('takes the tag id from the record key when the caller gives none', () => {
const pduObj = decode(encode({ const pduObj = decode(encode({
cmdName: 'deliver_sm', cmdName: 'deliver_sm',
+1 -1
View File
@@ -2963,7 +2963,7 @@ describe('message id notation', () => {
const [dlr, pduObj] = await reported; const [dlr, pduObj] = await reported;
assert.equal(dlr.smsId, sent.smsIds[0]); assert.equal(dlr.smsId, sent.smsIds[0]);
assert.equal(paramText(pduObj.tlvs.receipted_message_id?.tagValue), '1a2b'); assert.equal(pduObj.tlvs.receipted_message_id?.tagValue, '1a2b');
}); });
test('leaves the segment ids of a multipart send to merge as they are', async t => { test('leaves the segment ids of a multipart send to merge as they are', async t => {
-7
View File
@@ -189,13 +189,6 @@ and is also what the panel ranked hardest — two methods, one answer.
### Correctness, ahead of everything below ### Correctness, ahead of everything below
- [ ] **Read `multiple` in `parseTlvs()` and `writeTlvs()`, or delete it and `tlvMap`.** Five TLVs
declare `multiple: true` (`callback_num`, `callback_num_atag`, `callback_num_pres_ind`,
`broadcast_area_identifier`, `broadcast_error_status`) and nothing reads it; `parseTlvs()` keys
by tag name, so a peer sending two `callback_num` TLVs silently keeps the last. `tlvMap` on
`broadcast_sm_resp` is declared, set once and read nowhere. This is the "Dormant filters" row
of the 0.4.0 defect table in a new spelling — metadata that reads as a guarantee.
- [ ] **Test that a multipart send which errors never fires `messageDlr`.** Goal 2 now says so and - [ ] **Test that a multipart send which errors never fires `messageDlr`.** Goal 2 now says so and
README promises it; `session-extras.test.ts` covers a drop *after* the send, not one during it. README promises it; `session-extras.test.ts` covers a drop *after* the send, not one during it.