Skip to content

Commit 0725304

Browse files
Apollon77claude
andauthored
fix(general): decode TXT records as raw bytes per RFC 6763 §6 (#3671)
* fix(general): decode TXT records as raw bytes per RFC 6763 §6 decodeTxtRecord/encodeTxtRecord/TxtRecord now operate on Bytes[] so arbitrary 8-bit values (Thread BR meshcop xa, xp, at, pt, sb, dd) survive encode/decode losslessly. encodeTxtRecord and the TxtRecord factory accept (Bytes | string)[] so ASCII-only senders pass strings as before. Internal Matter consumers are unchanged: TXT values are still surfaced as Map<string, string> via DnssdName.parameters, which now splits each entry on byte 0x3D and decodes both halves with Bytes.toString. DnssdName.keyOf projects entries via Bytes.toHex for content-stable dedup. A Phase B follow-up will introduce a DnssdParameters façade exposing the raw bytes via .raw(key) for non-Matter consumers (Thread Border Router enrichment in matterjs-server #406). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(general): expose raw TXT bytes via DnssdParameters façade DnssdName.parameters and IpService.parameters now return a DnssdParameters instance — a ReadonlyMap<string, string> with a `.raw(key): Bytes` accessor that surfaces the original TXT byte sequence. Existing string consumers (DiscoveryData, CommissionableMdnsScanner, Peer) are unaffected because DnssdParameters implements ReadonlyMap<string, string>. The escape hatch enables external consumers to recover binary TXT values like the Thread MeshCoP xa, xp, at, pt, sb, dd fields (driver: matterjs-server #406 Thread Border Router enrichment). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(changelog): note TXT decode shape change and DnssdParameters.raw() Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(general): correct Uint8Array constructor argument order in Bytes.of Bytes.of swapped byteOffset and byteLength when wrapping a non-Uint8Array view, producing a slice into the wrong region of the underlying buffer. Latent because every existing call site passes a Uint8Array and hits the early-return branch — but Bytes.of accepts any AllowSharedBufferSource and the bug fires on DataView/Int8Array/etc. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(general): RFC 6763 §6.4/§6.5 conformance in DnssdName.parameters §6.4: Duplicate keys within or across TXT records now first-wins instead of the previous last-wins semantics from Map.set. §6.5: Zero-length TXT entries are now silently ignored on receive instead of producing a parameter with an empty key. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(general): post-greg-review polish - TxtRecord/encodeTxtRecord: document the (Bytes | string)[] dual shape so future readers don't unify the union and break ASCII senders. - DnssdName.parameters: ignore TXT entries with empty keys per RFC 6763 §6.4 (the spec requires "key MUST be at least one character"); add tests for empty-key and empty-value (key=) cases. - DnssdName.parameters: trim the RFC §6.4/§6.5 comments to one line each. - MdnsTest expectMessage: sort TXT entries by hex (lossless on binary) instead of round-tripping bytes → string → bytes; normalize string-array fixtures to Bytes[] before sorting so deep.equals matches actual decoded records. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(general): address Copilot review feedback on TXT record handling - encodeTxtRecord: warn and truncate to 255 bytes when an entry exceeds the RFC 6763 §6.1 length-octet limit, instead of silently wrapping via writeUInt8 and producing invalid wire data. - DnssdName.parameters: order TXT records newest-first (by installedAt) before applying RFC 6763 §6.4 first-wins so an updated record's keys take precedence over the not-yet-expired older copy. First-wins still applies to entries within each record in their wire order. - MdnsTest expectMessage sortKey: project TXT entries through Bytes.toHex (lossless) instead of Bytes.toString (lossy on binary), normalizing both string and Uint8Array fixtures to the same hex form. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(general): drop broken {@link key} in DnssdParameters.raw JSDoc Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 32affcd commit 0725304

12 files changed

Lines changed: 541 additions & 33 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ The main work (all changes without a GitHub username in brackets in the below li
2222

2323
- @matter/general
2424
- Breaking: Blob/File-related storage methods were removed from the normal Storage implementation
25+
- Breaking: `DnsCodec.decodeTxtRecord`/`encodeTxtRecord` and the `TxtRecord` factory now operate on `Bytes[]` per RFC 6763 §6 so binary TXT values (e.g. Thread MeshCoP `xa`, `xp`, `at`, `pt`, `sb`, `dd`) survive encode/decode losslessly; `encodeTxtRecord` and `TxtRecord` accept `(Bytes | string)[]` so ASCII-only senders are unchanged
2526
- Feature: Added a new `wal`-based storage engine (not yet the default) to optimize persistence
27+
- Feature: `DnssdName.parameters` and `IpService.parameters` now return a `DnssdParameters` instance — a `ReadonlyMap<string, string>` plus a `.raw(key): Bytes` accessor for binary TXT consumers (e.g. Thread Border Router enrichment)
2628
- Enhancement: Added locking to storage implementations to prevent concurrent access issues and data corruption
2729
- Enhancement: Split out Blob-Storage into its own `dir`-based BlobStorage implementation
2830
- Enhancement: Added Storage Migration logic that can generically migrate between different storage engines

packages/general/src/codec/DnsCodec.ts

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,21 @@ export const AAAARecord = (
5959
recordClass: DnsRecordClass.IN,
6060
flushCache,
6161
});
62+
63+
/**
64+
* Build a TXT {@link DnsRecord} (RFC 6763 §6).
65+
*
66+
* Accepts `(Bytes | string)[]` for ergonomic ASCII senders; entries are normalized to {@link Bytes} before storage so
67+
* downstream consumers always see the spec-correct binary shape.
68+
*/
6269
export const TxtRecord = (
6370
name: string,
64-
entries: string[],
71+
entries: (Bytes | string)[],
6572
ttl = DEFAULT_MDNS_TTL,
6673
flushCache = false,
67-
): DnsRecord<string[]> => ({
74+
): DnsRecord<Bytes[]> => ({
6875
name,
69-
value: entries,
76+
value: entries.map(e => (typeof e === "string" ? Bytes.fromString(e) : Bytes.of(e))),
7077
ttl,
7178
recordType: DnsRecordType.TXT,
7279
recordClass: DnsRecordClass.IN,
@@ -312,13 +319,14 @@ export class DnsCodec {
312319
return { priority, weight, port, target };
313320
}
314321

315-
static decodeTxtRecord(valueBytes: Bytes): string[] {
322+
static decodeTxtRecord(valueBytes: Bytes): Bytes[] {
316323
const reader = new DataReader(valueBytes);
317-
const result = new Array<string>();
324+
const result = new Array<Bytes>();
318325
let bytesRead = 0;
319326
while (bytesRead < valueBytes.byteLength) {
320327
const length = reader.readUInt8();
321-
result.push(reader.readUtf8String(length));
328+
// readByteArray returns a view; copy so retained entries don't alias the source buffer.
329+
result.push(reader.readByteArray(length).slice());
322330
bytesRead += length + 1;
323331
}
324332
return result;
@@ -410,7 +418,7 @@ export class DnsCodec {
410418
case DnsRecordType.SRV:
411419
return this.encodeSrvRecord(value as SrvRecordValue);
412420
case DnsRecordType.TXT:
413-
return this.encodeTxtRecord(value as string[]);
421+
return this.encodeTxtRecord(value as (Bytes | string)[]);
414422
case DnsRecordType.AAAA:
415423
return this.encodeAaaaRecord(value as string);
416424
case DnsRecordType.A:
@@ -431,12 +439,26 @@ export class DnsCodec {
431439
return ipv6ToBytes(ip);
432440
}
433441

434-
static encodeTxtRecord(entries: string[]) {
442+
/**
443+
* Encode TXT record entries (RFC 6763 §6).
444+
*
445+
* Accepts `(Bytes | string)[]` for ergonomic ASCII senders; the `string` arm exists to spare callers a per-entry
446+
* `Bytes.fromString` wrap.
447+
*/
448+
static encodeTxtRecord(entries: (Bytes | string)[]) {
435449
const writer = new DataWriter();
436450
entries.forEach(entry => {
437-
const entryData = Bytes.fromString(entry);
438-
writer.writeUInt8(entryData.byteLength);
439-
writer.writeByteArray(entryData);
451+
let bytes = typeof entry === "string" ? Bytes.fromString(entry) : Bytes.of(entry);
452+
if (bytes.byteLength > 0xff) {
453+
// RFC 6763 §6.1: TXT entries are length-prefixed by a single octet — silently wrapping the length
454+
// byte produces garbage on the wire, so warn loudly and truncate to the spec limit.
455+
logger.warn(
456+
`TXT record entry length ${bytes.byteLength} exceeds the RFC 6763 §6.1 limit of 255 bytes; truncating.`,
457+
);
458+
bytes = Bytes.of(bytes).subarray(0, 0xff);
459+
}
460+
writer.writeUInt8(bytes.byteLength);
461+
writer.writeByteArray(bytes);
440462
});
441463
return writer.toByteArray();
442464
}

packages/general/src/net/dns-sd/DnssdName.ts

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import { Logger } from "#log/Logger.js";
99
import { Time } from "#time/Time.js";
1010
import type { Timestamp } from "#time/Timestamp.js";
1111
import { Millis } from "#time/TimeUnit.js";
12+
import { Bytes } from "#util/Bytes.js";
1213
import { AsyncObserver, BasicObservable } from "#util/Observable.js";
1314
import { MaybePromise } from "#util/Promises.js";
1415
import type { DnssdNames } from "./DnssdNames.js";
16+
import { DnssdParameters } from "./DnssdParameters.js";
1517

1618
const logger = Logger.get("DnssdName");
1719

@@ -36,7 +38,7 @@ export class DnssdName extends BasicObservable<[changes: DnssdName.Changes], May
3638
#changes?: Map<string, { kind: "update" | "delete"; record: DnssdName.Record }>;
3739
#notified?: Promise<void>;
3840
#maybeDeleting?: Promise<void>;
39-
#parameters?: Map<string, string>;
41+
#parameters?: DnssdParameters;
4042
#dependencies?: Map<string, DnssdName>;
4143
#nullObserver?: () => void;
4244

@@ -66,22 +68,40 @@ export class DnssdName extends BasicObservable<[changes: DnssdName.Changes], May
6668
return this.#records.values();
6769
}
6870

69-
get parameters(): ReadonlyMap<string, string> {
71+
get parameters(): DnssdParameters {
7072
if (this.#parameters === undefined) {
71-
const parameters = (this.#parameters = new Map<string, string>());
73+
const raw = new Map<string, Bytes>();
74+
// Process newest TXT records first so an updated record's keys win over the not-yet-expired older copy;
75+
// first-wins (RFC 6763 §6.4) then applies to entries within each record in their wire order.
76+
const txtRecords = new Array<DnssdName.TextRecord>();
7277
for (const record of this.#records.values()) {
73-
if (record.recordType !== DnsRecordType.TXT) {
74-
continue;
78+
if (record.recordType === DnsRecordType.TXT) {
79+
txtRecords.push(record);
7580
}
76-
for (const entry of record.value as string[]) {
77-
const pos = entry.indexOf("=");
78-
if (pos === -1) {
79-
parameters.set(entry, "");
80-
} else {
81-
parameters.set(entry.slice(0, pos), entry.slice(pos + 1));
81+
}
82+
txtRecords.sort((a, b) => b.installedAt - a.installedAt);
83+
for (const record of txtRecords) {
84+
for (const entry of record.value) {
85+
const bytes = Bytes.of(entry);
86+
// RFC 6763 §6.5: ignore zero-length entry.
87+
if (bytes.byteLength === 0) {
88+
continue;
89+
}
90+
// 0x3D = '='. RFC 6763 §6.4: split on the first '=' (later '=' bytes, e.g. base64 padding, belong to the value).
91+
const eqIndex = bytes.indexOf(0x3d);
92+
// RFC 6763 §6.4: ignore entry with empty key.
93+
if (eqIndex === 0) {
94+
continue;
95+
}
96+
const key = eqIndex === -1 ? Bytes.toString(bytes) : Bytes.toString(bytes.subarray(0, eqIndex));
97+
// RFC 6763 §6.4: first occurrence wins on duplicates.
98+
if (raw.has(key)) {
99+
continue;
82100
}
101+
raw.set(key, eqIndex === -1 ? new Uint8Array(0) : bytes.subarray(eqIndex + 1));
83102
}
84103
}
104+
this.#parameters = new DnssdParameters(raw);
85105
}
86106
return this.#parameters;
87107
}
@@ -259,8 +279,9 @@ function keyOf(record: DnsRecord): string | undefined {
259279

260280
case DnsRecordType.TXT:
261281
if (Array.isArray(record.value)) {
262-
// Spread before sort: keyOf must not mutate record.value (parameters recompute relies on wire order)
263-
return `${record.recordType} ${[...record.value].sort().join(" ")}`;
282+
const keys = (record.value as Bytes[]).map(entry => Bytes.toHex(entry));
283+
keys.sort();
284+
return `${record.recordType} ${keys.join(" ")}`;
264285
}
265286
break;
266287
}
@@ -307,7 +328,7 @@ export namespace DnssdName {
307328
recordType: DnsRecordType.SRV;
308329
}
309330

310-
export interface TextRecord extends DnsRecord<string[]>, Expiration {
331+
export interface TextRecord extends DnsRecord<Bytes[]>, Expiration {
311332
recordType: DnsRecordType.TXT;
312333
}
313334

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/**
2+
* @license
3+
* Copyright 2022-2026 Matter.js Authors
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { Bytes } from "#util/Bytes.js";
8+
9+
/**
10+
* Parsed key/value pairs from a DNS-SD service's TXT records.
11+
*
12+
* Implements {@link ReadonlyMap}<string, string> with UTF-8 decoded values. Use {@link raw} for the original byte
13+
* sequence — required for binary TXT fields such as the Thread MeshCoP `xa`, `xp`, `at`, `pt`, `sb`, and `dd` keys
14+
* (RFC 6763 §6 — TXT values are opaque binary data; non-UTF-8 bytes mojibake to U+FFFD via the string accessors).
15+
*/
16+
export class DnssdParameters implements ReadonlyMap<string, string> {
17+
readonly #raw: ReadonlyMap<string, Bytes>;
18+
19+
constructor(raw: ReadonlyMap<string, Bytes>) {
20+
this.#raw = raw;
21+
}
22+
23+
get(key: string): string | undefined {
24+
const v = this.#raw.get(key);
25+
return v === undefined ? undefined : Bytes.toString(v);
26+
}
27+
28+
/**
29+
* The original bytes for the given key, or `undefined` if absent.
30+
*/
31+
raw(key: string): Bytes | undefined {
32+
return this.#raw.get(key);
33+
}
34+
35+
has(key: string): boolean {
36+
return this.#raw.has(key);
37+
}
38+
39+
get size(): number {
40+
return this.#raw.size;
41+
}
42+
43+
keys(): MapIterator<string> {
44+
return this.#raw.keys();
45+
}
46+
47+
*values(): MapIterator<string> {
48+
for (const v of this.#raw.values()) {
49+
yield Bytes.toString(v);
50+
}
51+
}
52+
53+
*entries(): MapIterator<[string, string]> {
54+
for (const [k, v] of this.#raw) {
55+
yield [k, Bytes.toString(v)];
56+
}
57+
}
58+
59+
[Symbol.iterator](): MapIterator<[string, string]> {
60+
return this.entries();
61+
}
62+
63+
forEach(cb: (value: string, key: string, map: ReadonlyMap<string, string>) => void, thisArg?: unknown): void {
64+
for (const [k, v] of this.entries()) {
65+
cb.call(thisArg, v, k, this);
66+
}
67+
}
68+
}

packages/general/src/net/dns-sd/IpService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { Abort } from "#util/Abort.js";
1414
import { AsyncObservable, AsyncObservableValue, ObserverGroup } from "#util/Observable.js";
1515
import { DnssdName } from "./DnssdName.js";
1616
import { DnssdNames } from "./DnssdNames.js";
17+
import { DnssdParameters } from "./DnssdParameters.js";
1718
import { IpServiceStatus } from "./IpServiceStatus.js";
1819

1920
/**
@@ -95,7 +96,7 @@ export class IpService {
9596
/**
9697
* Values from TXT records.
9798
*/
98-
get parameters(): ReadonlyMap<string, string> {
99+
get parameters(): DnssdParameters {
99100
return this.#name.parameters;
100101
}
101102

packages/general/src/net/dns-sd/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
export * from "./DnssdName.js";
88
export * from "./DnssdNames.js";
9+
export * from "./DnssdParameters.js";
910
export * from "./DnssdSolicitor.js";
1011
export * from "./IpService.js";
1112
export * from "./IpServiceResolution.js";

packages/general/src/util/Bytes.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export namespace Bytes {
7777
}
7878

7979
if (ArrayBuffer.isView(source)) {
80-
return new Uint8Array(source.buffer, source.byteLength, source.byteOffset);
80+
return new Uint8Array(source.buffer, source.byteOffset, source.byteLength);
8181
}
8282

8383
return new Uint8Array(source);

packages/general/test/codec/DnsCodecTest.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
} from "#codec/DnsCodec.js";
1717
import { Seconds } from "#time/TimeUnit.js";
1818
import { Bytes } from "#util/Bytes.js";
19-
import { DataReader } from "#util/index.js";
19+
import { DataReader, DataWriter } from "#util/index.js";
2020

2121
const DNS_RESPONSE: DnsMessage = {
2222
transactionId: 0,
@@ -115,7 +115,7 @@ const DECODED_WITH_PRIVATE_TYPE = {
115115
"RI=0900669ED4F3C8043FCD77A39EEBD96F672A",
116116
"PH=36",
117117
"PI=",
118-
],
118+
].map(Bytes.fromString),
119119
},
120120
{
121121
flushCache: false,
@@ -276,6 +276,33 @@ describe("DnsCodec", () => {
276276
}
277277
});
278278

279+
describe("encode and decode TXT records", () => {
280+
it("round-trips a TXT entry containing high bytes losslessly", () => {
281+
// xa = 0x5AAF359C0501A1B0 — real Apple HomePod border-router extended MAC
282+
const rawXa = Bytes.fromHex("5aaf359c0501a1b0");
283+
const entry = Bytes.concat(Bytes.fromString("xa="), rawXa);
284+
285+
const writer = new DataWriter();
286+
writer.writeUInt8(entry.byteLength);
287+
writer.writeByteArray(entry);
288+
const wire = writer.toByteArray();
289+
290+
const decoded = DnsCodec.decodeTxtRecord(wire);
291+
expect(decoded).to.have.length(1);
292+
expect(Bytes.areEqual(decoded[0], entry)).to.equal(true);
293+
294+
const reEncoded = DnsCodec.encodeTxtRecord(decoded);
295+
expect(Bytes.areEqual(reEncoded, wire)).to.equal(true);
296+
});
297+
298+
it("truncates entries longer than 255 bytes per RFC 6763 §6.1", () => {
299+
const oversize = new Uint8Array(300).fill(0x41);
300+
const wire = DnsCodec.encodeTxtRecord([oversize]);
301+
expect(wire[0]).equal(0xff);
302+
expect(wire.byteLength).equal(0xff + 1);
303+
});
304+
});
305+
279306
describe("encode and decode AAAA records", () => {
280307
it("encodes and decodes AAAA fe80 record", () => {
281308
const encoded = DnsCodec.encodeAaaaRecord("fe80::9580:b733:6f54:9f43");

0 commit comments

Comments
 (0)