Skip to content

Commit 4099eef

Browse files
thebenternthebentern
committed
Add comprehensive tests for compression and decompression boundaries, including remarks handling and edge cases
1 parent edf1948 commit 4099eef

File tree

6 files changed

+328
-0
lines changed

6 files changed

+328
-0
lines changed

‎kotlin/src/test/kotlin/org/meshtastic/tak/CompressionTest.kt‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,116 @@ class CompressionTest {
4040
"(XML: ${xml.length}B, proto: ${result.protobufSize}B)")
4141
}
4242

43+
// -- compressWithRemarksFallback boundary & outcome tests ----------------
44+
//
45+
// These tests pin down the four outcomes of compressWithRemarksFallbackDetailed
46+
// (fit-as-is / fit-after-strip / dropped-with-remarks / dropped-no-remarks)
47+
// and the <=-inclusive MTU boundary. They use synthetic packets built
48+
// directly from TakPacketV2Data rather than fixture XML, so the thresholds
49+
// are deterministic regardless of future fixture or dictionary changes.
50+
51+
private fun makePliWithRemarks(remarks: String): TakPacketV2Data = TakPacketV2Data(
52+
cotTypeId = CotTypeMapper.COTTYPE_A_F_G_U_C,
53+
how = CotTypeMapper.COTHOW_M_G,
54+
callsign = "TESTER",
55+
latitudeI = 340522000,
56+
longitudeI = -1182437000,
57+
altitude = 100,
58+
uid = "testnode",
59+
remarks = remarks,
60+
payload = TakPacketV2Data.Payload.Pli(true),
61+
)
62+
63+
@Test
64+
fun `fallback returns full payload when under limit with no strip`() {
65+
val packet = makePliWithRemarks("")
66+
val result = compressor.compressWithRemarksFallbackDetailed(packet, maxWireBytes = 500)
67+
assertNotNull(result.wirePayload, "expected a wire payload under a generous limit")
68+
assertFalse(result.remarksStripped, "no remarks => no strip should happen")
69+
assertTrue(result.fits)
70+
}
71+
72+
@Test
73+
fun `fallback strips remarks when oversized with remarks but fits without`() {
74+
val packet = makePliWithRemarks("x".repeat(500))
75+
val fullSize = compressor.compress(packet).size
76+
val strippedSize = compressor.compress(packet.copy(remarks = "")).size
77+
assertTrue(fullSize > strippedSize,
78+
"test setup: padded remarks must grow the wire payload (full=$fullSize stripped=$strippedSize)")
79+
80+
// Limit sits between the two sizes — stripping must save the packet.
81+
val limit = strippedSize + (fullSize - strippedSize) / 2
82+
val result = compressor.compressWithRemarksFallbackDetailed(packet, limit)
83+
assertNotNull(result.wirePayload, "stripping should have produced a fit")
84+
assertTrue(result.remarksStripped, "remarksStripped flag should be true")
85+
assertTrue(result.wirePayload!!.size <= limit)
86+
// Backward-compat wrapper must agree with the detailed variant.
87+
val legacy = compressor.compressWithRemarksFallback(packet, limit)
88+
assertNotNull(legacy)
89+
assertTrue(legacy!!.contentEquals(result.wirePayload!!))
90+
}
91+
92+
@Test
93+
fun `fallback drops when even stripped payload exceeds limit`() {
94+
val packet = makePliWithRemarks("x".repeat(500))
95+
val strippedSize = compressor.compress(packet.copy(remarks = "")).size
96+
97+
// Limit is below even the stripped size — nothing can save this packet.
98+
val result = compressor.compressWithRemarksFallbackDetailed(packet, strippedSize - 1)
99+
assertNull(result.wirePayload)
100+
assertTrue(result.remarksStripped,
101+
"stripping was attempted even though it didn't help")
102+
assertFalse(result.fits)
103+
assertNull(compressor.compressWithRemarksFallback(packet, strippedSize - 1))
104+
}
105+
106+
@Test
107+
fun `fallback drops when packet has no remarks to strip`() {
108+
val packet = makePliWithRemarks("")
109+
val fullSize = compressor.compress(packet).size
110+
111+
// Limit is 1 byte below the natural compressed size. Packet has no
112+
// remarks, so stripping is a no-op and the caller must drop it.
113+
val result = compressor.compressWithRemarksFallbackDetailed(packet, fullSize - 1)
114+
assertNull(result.wirePayload)
115+
assertFalse(result.remarksStripped,
116+
"no remarks => stripping should not be attempted")
117+
}
118+
119+
@Test
120+
fun `fallback maxWireBytes boundary is inclusive`() {
121+
// Pins the `<=` in compressWithRemarksFallbackDetailed. The boundary
122+
// case (limit == actual size) MUST succeed; limit-1 MUST drop (no
123+
// remarks to strip). This is the off-by-one guard for item #16 from
124+
// the audit — independent of the real 237B LoRa MTU.
125+
val packet = makePliWithRemarks("")
126+
val naturalSize = compressor.compress(packet).size
127+
128+
val atLimit = compressor.compressWithRemarksFallbackDetailed(packet, maxWireBytes = naturalSize)
129+
assertNotNull(atLimit.wirePayload, "<= must accept the exact boundary")
130+
assertEquals(naturalSize, atLimit.wirePayload!!.size)
131+
assertFalse(atLimit.remarksStripped)
132+
133+
val belowLimit = compressor.compressWithRemarksFallbackDetailed(packet, maxWireBytes = naturalSize - 1)
134+
assertNull(belowLimit.wirePayload, "1 byte under boundary must drop")
135+
}
136+
137+
@Test
138+
fun `fallback accepts every fixture at the real LoRa MTU`() {
139+
// Sanity check: every real fixture survives the full fallback path at
140+
// the canonical 237B MTU. Worst-case fixture (drawing_telestration)
141+
// is 212B, so all of them should come back with remarksStripped=false.
142+
for (fixture in TestFixtures.filenames) {
143+
val xml = TestFixtures.loadFixture(fixture)
144+
val packet = parser.parse(xml)
145+
val result = compressor.compressWithRemarksFallbackDetailed(packet, maxWireBytes = LORA_MTU)
146+
assertNotNull(result.wirePayload, "$fixture: should fit under LoRa MTU")
147+
assertTrue(result.wirePayload!!.size <= LORA_MTU, "$fixture: wire size within MTU")
148+
}
149+
}
150+
151+
// -- end fallback tests --------------------------------------------------
152+
43153
@Test
44154
fun `compression achieves meaningful ratio`() {
45155
// At least 3x compression on average across all fixtures

‎kotlin/src/test/kotlin/org/meshtastic/tak/CotTypeMapperTest.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,25 @@ class CotTypeMapperTest {
7070
assertFalse(CotTypeMapper.isAircraftString("a-f-S"))
7171
}
7272

73+
@Test
74+
fun `aircraft string classification handles edge cases`() {
75+
// Fewer than 3 atoms -> always non-aircraft (guards size >= 3)
76+
assertFalse(CotTypeMapper.isAircraftString(""), "empty string")
77+
assertFalse(CotTypeMapper.isAircraftString("A"), "single atom 'A'")
78+
assertFalse(CotTypeMapper.isAircraftString("a"), "single non-A atom")
79+
assertFalse(CotTypeMapper.isAircraftString("a-f"), "2 atoms")
80+
81+
// 'A' present but NOT at index 2 -> non-aircraft
82+
assertFalse(CotTypeMapper.isAircraftString("A-f-G"), "A at index 0")
83+
assertFalse(CotTypeMapper.isAircraftString("a-A-G"), "A at index 1")
84+
assertFalse(CotTypeMapper.isAircraftString("a---A"), "A at index 3 after empty atoms")
85+
assertFalse(CotTypeMapper.isAircraftString("a-f-G-A"), "A at index 3")
86+
87+
// Exactly 3 atoms with A at index 2 -> minimal valid aircraft type
88+
assertTrue(CotTypeMapper.isAircraftString("a-f-A"), "minimal 3-atom aircraft")
89+
assertTrue(CotTypeMapper.isAircraftString("a-h-A"), "hostile 3-atom aircraft")
90+
}
91+
7392
@Test
7493
fun `how strings map correctly`() {
7594
assertEquals(CotTypeMapper.COTHOW_H_E, CotTypeMapper.howToEnum("h-e"))

‎kotlin/src/test/kotlin/org/meshtastic/tak/MalformedInputTest.kt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,46 @@ class MalformedInputTest {
9898
fun `rejects decompression bomb`() {
9999
assertThrows<Exception> { compressor.decompress(loadMalformed("decompression_bomb.bin")) }
100100
}
101+
102+
// -- Decompression size-cap boundary tests (audit item #19) --------------
103+
//
104+
// The existing decompression_bomb.bin fixture proves "> 4096 rejects" for
105+
// a dict-compressed payload via the zstd library's max_output_size guard.
106+
// These tests pin the boundary on the 0xFF uncompressed path — the only
107+
// branch where TakCompressor enforces the cap itself — with synthetic
108+
// wire payloads of exactly 4096 and 4097 bytes.
109+
110+
@Test
111+
fun `uncompressed payload over MAX_DECOMPRESSED_SIZE is rejected`() {
112+
// [0xFF] + 4097 bytes of anything -> size check MUST fire before
113+
// the bytes are handed to the protobuf parser.
114+
val wire = ByteArray(1 + TakCompressor.MAX_DECOMPRESSED_SIZE + 1)
115+
wire[0] = 0xFF.toByte()
116+
val ex = assertThrows<IllegalArgumentException> { compressor.decompress(wire) }
117+
assertTrue(
118+
ex.message?.contains("exceeds limit") == true,
119+
"expected 'exceeds limit' in error message, got: ${ex.message}",
120+
)
121+
}
122+
123+
@Test
124+
fun `uncompressed payload at MAX_DECOMPRESSED_SIZE passes size guard`() {
125+
// [0xFF] + exactly 4096 bytes. The size check is `> MAX_DECOMPRESSED_SIZE`
126+
// so 4096 bytes is within the limit. 4096 zero bytes is NOT valid
127+
// protobuf (field tag 0 is reserved), so the call will still throw —
128+
// but the failure must come from the downstream protobuf parse step,
129+
// NOT from the size guard. Verified by asserting the error message
130+
// does not mention the size limit.
131+
val wire = ByteArray(1 + TakCompressor.MAX_DECOMPRESSED_SIZE)
132+
wire[0] = 0xFF.toByte()
133+
try {
134+
compressor.decompress(wire)
135+
// If it somehow parses successfully, that's fine — no size error.
136+
} catch (e: Exception) {
137+
assertFalse(
138+
e.message?.contains("exceeds limit") == true,
139+
"size check fired at the exact boundary: ${e.message}",
140+
)
141+
}
142+
}
101143
}

‎typescript/tests/compression.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,26 @@ import { FIXTURES, loadFixtureXml } from "./helpers.js";
66
const LORA_MTU = 237;
77
const compressor = new TakCompressor();
88

9+
// Minimal synthetic-packet seed for the fallback tests. Using a real fixture
10+
// (`pli_basic.xml`) avoids reconstructing every optional field by hand; the
11+
// tests then mutate `remarks` directly to control wire size.
12+
const SYNTHETIC_XML = `<?xml version="1.0" encoding="UTF-8"?>
13+
<event version="2.0" uid="testnode" type="a-f-G-U-C" how="m-g"
14+
time="2026-03-15T14:22:10Z" start="2026-03-15T14:22:10Z"
15+
stale="2026-03-15T14:22:55Z">
16+
<point lat="37.7749" lon="-122.4194" hae="100" ce="4.9" le="9999999"/>
17+
<detail>
18+
<contact callsign="TESTER"/>
19+
<uid Droid="testnode"/>
20+
</detail>
21+
</event>`;
22+
23+
function makePacketWithRemarks(remarks: string): Record<string, unknown> {
24+
const pkt = parseCotXml(SYNTHETIC_XML) as Record<string, unknown>;
25+
pkt.remarks = remarks;
26+
return pkt;
27+
}
28+
929
describe("Compression", () => {
1030
it.each(FIXTURES)("fits under LoRa MTU: %s", async (fixture) => {
1131
const xml = loadFixtureXml(fixture);
@@ -28,3 +48,87 @@ describe("Compression", () => {
2848
expect(ratio).toBeGreaterThanOrEqual(3.0);
2949
});
3050
});
51+
52+
describe("compressWithRemarksFallback", () => {
53+
// These tests pin down the four outcomes of compressWithRemarksFallbackDetailed
54+
// (fit-as-is / fit-after-strip / dropped-with-remarks / dropped-no-remarks)
55+
// and the <=-inclusive MTU boundary. They use a synthetic packet built from
56+
// a tiny seed XML rather than real fixtures, so the thresholds are
57+
// deterministic regardless of future fixture or dictionary changes.
58+
59+
it("returns full payload when under limit with no strip", async () => {
60+
const pkt = makePacketWithRemarks("");
61+
const result = await compressor.compressWithRemarksFallbackDetailed(pkt, 500);
62+
expect(result.wirePayload).not.toBeNull();
63+
expect(result.remarksStripped).toBe(false);
64+
});
65+
66+
it("strips remarks when oversized with remarks but fits without", async () => {
67+
const pkt = makePacketWithRemarks("x".repeat(500));
68+
const full = await compressor.compress(pkt);
69+
const stripped = await compressor.compress({ ...pkt, remarks: "" });
70+
expect(full.length).toBeGreaterThan(stripped.length);
71+
72+
// Limit sits between the two sizes — stripping must save the packet.
73+
const limit = stripped.length + Math.floor((full.length - stripped.length) / 2);
74+
const result = await compressor.compressWithRemarksFallbackDetailed(pkt, limit);
75+
expect(result.wirePayload).not.toBeNull();
76+
expect(result.remarksStripped).toBe(true);
77+
expect(result.wirePayload!.length).toBeLessThanOrEqual(limit);
78+
79+
// Backward-compat wrapper must agree with the detailed variant.
80+
const legacy = await compressor.compressWithRemarksFallback(pkt, limit);
81+
expect(legacy).not.toBeNull();
82+
expect(legacy!.equals(result.wirePayload!)).toBe(true);
83+
});
84+
85+
it("drops when even stripped payload exceeds limit", async () => {
86+
const pkt = makePacketWithRemarks("x".repeat(500));
87+
const stripped = await compressor.compress({ ...pkt, remarks: "" });
88+
89+
// Limit is below even the stripped size — nothing can save this packet.
90+
const result = await compressor.compressWithRemarksFallbackDetailed(pkt, stripped.length - 1);
91+
expect(result.wirePayload).toBeNull();
92+
expect(result.remarksStripped).toBe(true);
93+
94+
const legacy = await compressor.compressWithRemarksFallback(pkt, stripped.length - 1);
95+
expect(legacy).toBeNull();
96+
});
97+
98+
it("drops when packet has no remarks to strip", async () => {
99+
const pkt = makePacketWithRemarks("");
100+
const full = await compressor.compress(pkt);
101+
102+
const result = await compressor.compressWithRemarksFallbackDetailed(pkt, full.length - 1);
103+
expect(result.wirePayload).toBeNull();
104+
expect(result.remarksStripped).toBe(false);
105+
});
106+
107+
it("maxWireBytes boundary is inclusive", async () => {
108+
// Pins the `<=` check. The boundary case (limit == actual size) MUST
109+
// succeed; limit-1 MUST drop (no remarks to strip).
110+
const pkt = makePacketWithRemarks("");
111+
const natural = await compressor.compress(pkt);
112+
113+
const atLimit = await compressor.compressWithRemarksFallbackDetailed(pkt, natural.length);
114+
expect(atLimit.wirePayload).not.toBeNull();
115+
expect(atLimit.wirePayload!.length).toBe(natural.length);
116+
expect(atLimit.remarksStripped).toBe(false);
117+
118+
const belowLimit = await compressor.compressWithRemarksFallbackDetailed(pkt, natural.length - 1);
119+
expect(belowLimit.wirePayload).toBeNull();
120+
});
121+
122+
it("accepts every fixture at the real LoRa MTU", async () => {
123+
// Sanity check: every real fixture survives the full fallback path at
124+
// the canonical 237B MTU. Worst-case is drawing_telestration at 212B,
125+
// so all of them should come back with remarksStripped=false.
126+
for (const fixture of FIXTURES) {
127+
const xml = loadFixtureXml(fixture);
128+
const pkt = parseCotXml(xml);
129+
const result = await compressor.compressWithRemarksFallbackDetailed(pkt, LORA_MTU);
130+
expect(result.wirePayload, `${fixture}: should fit under LoRa MTU`).not.toBeNull();
131+
expect(result.wirePayload!.length).toBeLessThanOrEqual(LORA_MTU);
132+
}
133+
});
134+
});

‎typescript/tests/cotTypeMapper.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,24 @@ describe("CotTypeMapper", () => {
2727
expect(isAircraftString("a-f-S")).toBe(false);
2828
});
2929

30+
it("classifies aircraft string edge cases correctly", () => {
31+
// Fewer than 3 atoms -> always non-aircraft (guards atoms.length >= 3)
32+
expect(isAircraftString("")).toBe(false);
33+
expect(isAircraftString("A")).toBe(false);
34+
expect(isAircraftString("a")).toBe(false);
35+
expect(isAircraftString("a-f")).toBe(false);
36+
37+
// 'A' present but NOT at index 2 -> non-aircraft
38+
expect(isAircraftString("A-f-G")).toBe(false); // index 0
39+
expect(isAircraftString("a-A-G")).toBe(false); // index 1
40+
expect(isAircraftString("a---A")).toBe(false); // index 3 after empty atoms
41+
expect(isAircraftString("a-f-G-A")).toBe(false); // index 3
42+
43+
// Exactly 3 atoms with A at index 2 -> minimal valid aircraft type
44+
expect(isAircraftString("a-f-A")).toBe(true);
45+
expect(isAircraftString("a-h-A")).toBe(true);
46+
});
47+
3048
it("round-trips type enum <-> string", () => {
3149
for (let i = 1; i <= 75; i++) {
3250
const s = typeToString(i);

‎typescript/tests/malformedInput.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,39 @@ describe("Malformed Input", () => {
6969
it("rejects decompression bomb", async () => {
7070
await expect(compressor.decompress(load("decompression_bomb.bin"))).rejects.toThrow();
7171
});
72+
73+
// -- Decompression size-cap boundary tests (audit item #19) ---------------
74+
//
75+
// The existing decompression_bomb.bin fixture proves "> 4096 rejects" for
76+
// a dict-compressed payload via the zstd library's max_output_size guard.
77+
// These tests pin the boundary on the 0xFF uncompressed path — the only
78+
// branch where TakCompressor enforces the cap itself — with synthetic
79+
// wire payloads of exactly 4096 and 4097 bytes.
80+
81+
const MAX_DECOMPRESSED_SIZE = 4096;
82+
83+
it("rejects uncompressed payload over MAX_DECOMPRESSED_SIZE", async () => {
84+
// [0xFF] + 4097 bytes of anything -> size check MUST fire before
85+
// the bytes are handed to the protobuf parser.
86+
const wire = Buffer.alloc(1 + MAX_DECOMPRESSED_SIZE + 1);
87+
wire[0] = 0xff;
88+
await expect(compressor.decompress(wire)).rejects.toThrow(/exceeds limit/);
89+
});
90+
91+
it("accepts uncompressed payload at MAX_DECOMPRESSED_SIZE (size guard inclusive)", async () => {
92+
// [0xFF] + exactly 4096 bytes. The size check is `> MAX_DECOMPRESSED_SIZE`
93+
// so 4096 bytes is within the limit. 4096 zero bytes is NOT valid
94+
// protobuf (field tag 0 is reserved), so the call will still throw —
95+
// but the failure must come from the downstream protobuf parse step,
96+
// NOT from the size guard.
97+
const wire = Buffer.alloc(1 + MAX_DECOMPRESSED_SIZE);
98+
wire[0] = 0xff;
99+
try {
100+
await compressor.decompress(wire);
101+
// If it somehow parses successfully, that's fine — no size error.
102+
} catch (e: unknown) {
103+
const msg = e instanceof Error ? e.message : String(e);
104+
expect(msg, `size check fired at the exact boundary: ${msg}`).not.toMatch(/exceeds limit/);
105+
}
106+
});
72107
});

0 commit comments

Comments
 (0)