Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use correct DnsRecord types for ipv4 addresses #409

Merged
merged 2 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 20 additions & 22 deletions packages/matter.js/src/mdns/MdnsBroadcaster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Fabric } from "../fabric/Fabric.js";
import { Logger } from "../log/Logger.js";
import { Network } from "../net/Network.js";
import { TypeFromPartialBitSchema } from "../schema/BitmapSchema.js";
import { isIPv4 } from "../util/Ip.js";
import { isIPv4, isIPv6 } from "../util/Ip.js";
import {
MATTER_COMMISSIONER_SERVICE_QNAME,
MATTER_COMMISSION_SERVICE_QNAME,
Expand Down Expand Up @@ -92,6 +92,22 @@ export class MdnsBroadcaster {
}
}

private getIpRecords(hostname: string, ips: string[]) {
const records = new Array<DnsRecord<any>>();
ips.forEach(ip => {
if (isIPv6(ip)) {
records.push(AAAARecord(hostname, ip));
} else if (isIPv4(ip)) {
if (this.enableIpv4) {
records.push(ARecord(hostname, ip));
}
} else {
logger.warn(`Unknown IP address type: ${ip}`);
}
});
return records;
}

/** Set the Broadcaster data to announce a device ready for commissioning in a special mode */
async setCommissionMode(
announcedNetPort: number,
Expand Down Expand Up @@ -170,13 +186,7 @@ export class MdnsBroadcaster {
`PI=${pairingInstructions}` /* Pairing Instruction */,
]),
];
ips.forEach(ip => {
if (isIPv4(ip) && this.enableIpv4) {
records.push(ARecord(hostname, ip));
} else {
records.push(AAAARecord(hostname, ip));
}
});
records.push(...this.getIpRecords(hostname, ips));
return records;
});
}
Expand Down Expand Up @@ -240,13 +250,7 @@ export class MdnsBroadcaster {
];
records.push(...fabricRecords);
});
ips.forEach(ip => {
if (isIPv4(ip) && this.enableIpv4) {
records.push(ARecord(hostname, ip));
} else {
records.push(AAAARecord(hostname, ip));
}
});
records.push(...this.getIpRecords(hostname, ips));
return records;
});
}
Expand Down Expand Up @@ -302,13 +306,7 @@ export class MdnsBroadcaster {
records.push(PtrRecord(deviceTypeQname, deviceQname));
}

ips.forEach(ip => {
if (isIPv4(ip) && this.enableIpv4) {
records.push(ARecord(hostname, ip));
} else {
records.push(AAAARecord(hostname, ip));
}
});
records.push(...this.getIpRecords(hostname, ips));
return records;
});
}
Expand Down
16 changes: 11 additions & 5 deletions packages/matter.js/test/mdns/MdnsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ const PORT3 = 5542;
const OPERATIONAL_ID = ByteArray.fromHex("0000000000000018");
const NODE_ID = NodeId(BigInt(1));

[false, true].forEach(testIpv4Enabled => {
const serverIps = testIpv4Enabled ? [SERVER_IPv4, SERVER_IPv6] : [SERVER_IPv6];
[
{ serverHasIpv4Addresses: true, testIpv4Enabled: true },
{ serverHasIpv4Addresses: true, testIpv4Enabled: false },
{ serverHasIpv4Addresses: false, testIpv4Enabled: false },
].forEach(({ serverHasIpv4Addresses, testIpv4Enabled }) => {
const serverIps = serverHasIpv4Addresses ? [SERVER_IPv4, SERVER_IPv6] : [SERVER_IPv6];
const clientIps = testIpv4Enabled ? [CLIENT_IPv4] : [CLIENT_IPv6];
const serverNetwork = new NetworkFake(SERVER_MAC, serverIps);
const clientNetwork = new NetworkFake(CLIENT_MAC, clientIps);
Expand All @@ -47,7 +51,7 @@ const NODE_ID = NodeId(BigInt(1));
value: "fe80::e777:4f5e:c61e:7314",
},
];
if (testIpv4Enabled) {
if (testIpv4Enabled && serverHasIpv4Addresses) {
IPDnsRecords.unshift({
flushCache: false,
name: "00B0D063C2260000.local",
Expand All @@ -60,12 +64,14 @@ const NODE_ID = NodeId(BigInt(1));

const IPIntegrationResultsPort1 = [{ ip: `${SERVER_IPv6}%fakeInterface`, port: PORT, type: "udp" }];
const IPIntegrationResultsPort2 = [{ ip: `${SERVER_IPv6}%fakeInterface`, port: PORT2, type: "udp" }];
if (testIpv4Enabled) {
if (testIpv4Enabled && serverHasIpv4Addresses) {
IPIntegrationResultsPort1.push({ ip: SERVER_IPv4, port: PORT, type: "udp" });
IPIntegrationResultsPort2.push({ ip: SERVER_IPv4, port: PORT2, type: "udp" });
}

describe(`MDNS Scanner and Broadcaster ${testIpv4Enabled ? "with" : "without"} IPv4`, () => {
describe(`MDNS Scanner and Broadcaster ${testIpv4Enabled ? "with" : "without"} IPv4 (and Ipv4 ${
serverHasIpv4Addresses ? "" : "not "
}provided)`, () => {
let broadcaster: MdnsBroadcaster;
let scanner: MdnsScanner;
let scannerChannel: UdpChannel;
Expand Down