diff --git a/src/helpers/NetworkLink.cpp b/src/helpers/NetworkLink.cpp index e8bc73c80..4fde2d142 100644 --- a/src/helpers/NetworkLink.cpp +++ b/src/helpers/NetworkLink.cpp @@ -37,6 +37,7 @@ namespace { // Trailing empty slots are dropped to keep remote replies short; "-" marks an // empty slot before a populated one, or a list with nothing in it. static void formatDnsList(char* out, size_t out_size, const ip_addr_t* servers, int count) { + if (out == nullptr || out_size == 0) return; int last = count - 1; while (last >= 0 && ip_addr_isany_val(servers[last])) last--; out[0] = '\0'; @@ -158,16 +159,20 @@ class NetworkLinkBase : public NetworkLink { // that ever changes.) static esp_err_t snapshotDnsOnTcpipThread(void* ctx) { NetworkLinkBase* self = static_cast(ctx); + ip_addr_t servers[kDnsServers] = {}; bool any = false; for (int i = 0; i < kDnsServers; i++) { const ip_addr_t* server = dns_getserver(i); - self->_dns_snapshot[i] = (server != nullptr) ? *server : *IP_ADDR_ANY; - if (!ip_addr_isany_val(self->_dns_snapshot[i])) any = true; + servers[i] = (server != nullptr) ? *server : *IP_ADDR_ANY; + if (!ip_addr_isany_val(servers[i])) any = true; } // A lease that carried no resolver at all (static configuration, or a // server that offered none) must not be remembered as "this medium's DNS is // nothing" — restoring that would wipe a working resolver for no gain. - if (any) self->_dns_captured = true; + if (any) { + for (int i = 0; i < kDnsServers; i++) self->_dns_snapshot[i] = servers[i]; + self->_dns_captured = true; + } return ESP_OK; } diff --git a/test/fixtures/network_dns.cpp b/test/fixtures/network_dns.cpp new file mode 100644 index 000000000..a15b5019d --- /dev/null +++ b/test/fixtures/network_dns.cpp @@ -0,0 +1,282 @@ +// The marked DNS methods and shared base are copied verbatim from production. +// This fixture replaces only SDK dispatch, address conversion, and link I/O. +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static void require(bool condition, const char* message) { + if (!condition) throw std::runtime_error(message); +} +constexpr int DNS_MAX_SERVERS = 3; +struct ip_addr_t { + std::array text{}; + unsigned type = 0; + bool operator==(const ip_addr_t& other) const { + return type == other.type && text == other.text; + } +}; +static ip_addr_t address(const char* text, unsigned type = 4) { + ip_addr_t value; + if (*text) { + require(strlen(text) < value.text.size(), "fixture address too long"); + strcpy(value.text.data(), text); + value.type = type; + } + return value; +} +static const ip_addr_t empty_address; +#define IP_ADDR_ANY (&empty_address) +static bool ip_addr_isany_val(const ip_addr_t& value) { return !value.text[0]; } +static char* ipaddr_ntoa_r(const ip_addr_t* value, char* out, int size) { + if (size <= 0 || strlen(value->text.data()) >= size_t(size)) return nullptr; + strcpy(out, value->text.data()); + return out; +} +using esp_err_t = int; +constexpr esp_err_t ESP_OK = 0; +static bool on_tcpip_thread = false; +static unsigned tcpip_dispatches = 0; +static std::array global_dns{}; +static std::array null_dns{}; +static std::vector dns_reads, dns_writes; +static esp_err_t esp_netif_tcpip_exec(esp_err_t (*callback)(void*), void* context) { + require(!on_tcpip_thread, "nested TCP/IP dispatch"); + ++tcpip_dispatches; + on_tcpip_thread = true; + const esp_err_t result = callback(context); + on_tcpip_thread = false; + return result; +} +static const ip_addr_t* dns_getserver(int index) { + require(on_tcpip_thread, "DNS read escaped TCP/IP thread"); + dns_reads.push_back(index); + return null_dns.at(index) ? nullptr : &global_dns.at(index); +} +static void dns_setserver(int index, const ip_addr_t* value) { + require(on_tcpip_thread, "DNS write escaped TCP/IP thread"); + dns_writes.push_back(index); + global_dns.at(index) = *value; +} +struct LinkEndpoint { + IPAddress gateway; + IPAddress gatewayIP() const { return gateway; } +} WiFi, CH390; + +@HELPERS@ +@BASE@ + +// Unchanged interface operations are irrelevant to DNS and have no SDK effects. +struct TestLink : NetworkLinkBase { + const char* mediumName() const override { return "test"; } + NetworkMedium medium() const override { return NetworkMedium::None; } + const char* statusName() const override { return "test"; } + int statusCode() const override { return 0; } + bool configValid(const char*) const override { return true; } + void setHostname(const char*) override {} + bool begin(const char*, const char*) override { return false; } + NetworkTransition maintain(uint32_t, uint8_t) override { return NetworkTransition::None; } + bool isConnected() const override { return false; } + IPAddress localIP() const override { return IPAddress(); } + int rssi() const override { return INT_MIN; } + bool resolveHost(const char*, IPAddress&) const override { return false; } + void formatDiagnostics(char*, size_t) const override {} +}; +struct WiFiNetworkLink : TestLink { @WiFiNetworkLink@ }; +struct EthernetNetworkLink : TestLink { @EthernetNetworkLink@ }; +struct AutomaticNetworkLink : TestLink { + std::atomic _selected{NetworkMedium::None}; + WiFiNetworkLink _wifi; + EthernetNetworkLink _ethernet; + @AutomaticNetworkLink@ +}; +static AutomaticNetworkLink automatic; +static unsigned link_lookups = 0; +NetworkLink& activeNetworkLink() { ++link_lookups; return automatic; } +static bool get(const char* config, char* reply) { + @GETTER@ else { return false; } + return true; +} + +static void restore_scenario() { + WiFiNetworkLink one, all; + const std::array complete{{ + address("1.1.1.1"), address("8.8.8.8"), address("2001:db8::53", 6)}}; + const std::array single{{address("9.9.9.9"), {}, {}}}; + global_dns = complete; + all.snapshotDns(); + global_dns = single; + one.snapshotDns(); + dns_writes.clear(); + all.restoreDns(); + require(global_dns == complete, "DNS restore lost a populated third slot or typed address"); + require(dns_writes == std::vector({0, 1, 2}), "DNS restore did not write every slot"); + dns_writes.clear(); + one.restoreDns(); + require(global_dns == single, "DNS restore retained another medium's empty-slot fallbacks"); + require(dns_writes == std::vector({0, 1, 2}), "DNS restore omitted empty slots"); + require(tcpip_dispatches == 4, "DNS snapshots/restores bypassed thread dispatch"); +} + +static void never_leased_scenario() { + WiFiNetworkLink never; + const std::array complete{{address("1.1.1.1"), address("8.8.8.8"), address("9.9.9.9")}}; + global_dns = complete; + never.restoreDns(); + require(global_dns == complete && dns_writes.empty(), "never-leased medium wiped live DNS"); + char lease[64] = {}; + never.formatLeaseDns(lease, sizeof(lease)); + require(std::string(lease) == "none", "never-leased DNS diagnostic is misleading"); + global_dns = {}; + never.snapshotDns(); + global_dns = complete; + never.restoreDns(); + require(global_dns == complete && dns_writes.empty(), "resolver-free lease wiped live DNS"); + never.formatLeaseDns(lease, sizeof(lease)); + require(std::string(lease) == "none", "resolver-free lease invented captured DNS"); +} + +static void renewal_scenario() { + WiFiNetworkLink link; + const std::array third_only{{{}, {}, address("9.9.9.9")}}; + const std::array complete{{address("1.1.1.1"), address("8.8.8.8"), address("9.9.9.9")}}; + const std::array partial{{{}, address("10.10.10.10"), {}}}; + global_dns = third_only; + link.snapshotDns(); + char lease[64] = {}; + link.formatLeaseDns(lease, sizeof(lease)); + require(std::string(lease) == "-,-,9.9.9.9", "third-slot-only lease was not captured"); + global_dns = complete; + link.restoreDns(); + require(global_dns == third_only, "third-slot-only DNS restore lost its resolver"); + global_dns = complete; + link.snapshotDns(); + global_dns = partial; + link.snapshotDns(); + global_dns = complete; + link.restoreDns(); + require(global_dns == partial, "valid DNS renewal retained obsolete resolver slots"); + global_dns = {}; + link.snapshotDns(); + global_dns = complete; + link.restoreDns(); + require(global_dns == partial, "DNS-free renewal wiped the last valid lease"); + link.formatLeaseDns(lease, sizeof(lease)); + require(std::string(lease) == "-,10.10.10.10", "DNS-free renewal diagnostic lost the last valid lease"); +} + +static void prepare_automatic() { + WiFi.gateway = IPAddress("10.0.0.1"); + CH390.gateway = IPAddress("192.168.1.1"); + global_dns = {{address("1.1.1.1"), {}, {}}}; + automatic._wifi.snapshotDns(); + global_dns = {{address("8.8.8.8"), {}, address("9.9.9.9")}}; + automatic._ethernet.snapshotDns(); + global_dns = {{address("4.4.4.4"), {}, {}}}; + automatic._selected = NetworkMedium::Ethernet; +} + +static void format_scenario() { + char list[64] = {}; + std::array servers{}; + formatDnsList(list, sizeof(list), servers.data(), 3); + require(std::string(list) == "-", "empty resolver list differs from documented sentinel"); + servers = {{address("1.1.1.1"), {}, {}}}; + formatDnsList(list, sizeof(list), servers.data(), 3); + require(std::string(list) == "1.1.1.1", "trailing empty resolver slots were retained"); + servers[2] = address("9.9.9.9"); + formatDnsList(list, sizeof(list), servers.data(), 3); + require(std::string(list) == "1.1.1.1,-,9.9.9.9", "interior empty resolver slot was dropped"); + global_dns = servers; + null_dns[0] = true; + formatCurrentDnsServers(list, sizeof(list)); + require(std::string(list) == "-,-,9.9.9.9", "null lwIP resolver was not treated as empty"); + null_dns = {}; + + prepare_automatic(); + const size_t writes_before = dns_writes.size(); + char reply[256] = {}; + const NetworkLink& wifi = automatic._wifi; + wifi.formatDns(reply, sizeof(reply)); + require(std::string(reply) == "> dns:4.4.4.4 gw:10.0.0.1 lease:1.1.1.1", "WiFi DNS interface lost its own lease/gateway"); + const NetworkLink& ethernet = automatic._ethernet; + ethernet.formatDns(reply, sizeof(reply)); + require(std::string(reply) == "> dns:4.4.4.4 gw:192.168.1.1 lease:8.8.8.8,-,9.9.9.9", "Ethernet DNS interface lost its own lease/gateway"); + const NetworkLink& selected = automatic; + selected.formatDns(reply, sizeof(reply)); + require(std::string(reply) == "> dns:4.4.4.4 selected:ethernet gw:192.168.1.1\neth-lease:8.8.8.8,-,9.9.9.9 wifi-lease:1.1.1.1", "automatic DNS omitted global or medium lease data"); + automatic._selected = NetworkMedium::WiFi; + selected.formatDns(reply, sizeof(reply)); + require(strstr(reply, "selected:wifi gw:10.0.0.1"), "automatic WiFi selection used the Ethernet gateway"); + automatic._selected = NetworkMedium::None; + selected.formatDns(reply, sizeof(reply)); + require(strstr(reply, "selected:none gw:0.0.0.0"), "unselected diagnostic invented a gateway"); + + // All three lists can exceed the real CLI's 160-byte buffer. Truncation is + // allowed; writing beyond the declared capacity or losing NUL is not. + global_dns = {{address("255.255.255.255"), address("255.255.255.255"), address("255.255.255.255")}}; + automatic._wifi.snapshotDns(); + automatic._ethernet.snapshotDns(); + automatic._selected = NetworkMedium::Ethernet; + CH390.gateway = IPAddress("255.255.255.255"); + selected.formatDns(reply, sizeof(reply)); + require(strlen(reply) == 207, "long DNS diagnostic fixture no longer exercises truncation"); + const std::string full(reply); + for (size_t size : {size_t(0), size_t(1), size_t(2), size_t(10), size_t(64), size_t(160)}) { + std::array bounded; + bounded.fill(0xA5); + char* out = reinterpret_cast(bounded.data() + 8); + selected.formatDns(out, size); + for (size_t i = 0; i < 8; ++i) require(bounded[i] == 0xA5, "DNS formatter wrote before destination"); + for (size_t i = 8 + size; i < bounded.size(); ++i) require(bounded[i] == 0xA5, "DNS formatter overran declared capacity"); + if (size) require(std::string(out) == full.substr(0, size - 1), "bounded DNS reply lost NUL or changed prefix"); + } + require(dns_writes.size() == writes_before, "read-only DNS diagnostics changed resolver state"); +} + +static void zero_scenario() { + char canary = 'Z'; + formatDnsList(&canary, 0, global_dns.data(), 3); + require(canary == 'Z', "zero-size DNS formatter wrote to its destination"); + formatDnsList(nullptr, 0, global_dns.data(), 3); + formatDnsList(nullptr, 7, global_dns.data(), 3); +} + +static void getter_scenario() { + prepare_automatic(); + struct Reply { unsigned char before[8]; char text[160]; unsigned char after[8]; } reply; + memset(&reply, 0xA5, sizeof(reply)); + require(get("link.dns", reply.text), "observer DNS getter was not recognized"); + require(link_lookups == 1 && strstr(reply.text, "> dns:4.4.4.4 selected:ethernet"), "observer DNS getter did not use the live NetworkLink interface"); + for (unsigned char c : reply.before) require(c == 0xA5, "observer DNS getter underflowed reply"); + for (unsigned char c : reply.after) require(c == 0xA5, "observer DNS getter overflowed reply"); + const std::string original(reply.text); + for (const char* rejected : {"link.dnsX", "LINK.DNS", "link.dns ", "wifi.status"}) { + require(!get(rejected, reply.text), "observer DNS getter accepted another command"); + require(std::string(reply.text) == original && link_lookups == 1, "rejected DNS command changed reply or queried the link"); + } +} + +int main(int argc, char** argv) { + try { + require(argc == 2, "missing DNS scenario"); + const std::string scenario(argv[1]); + if (scenario == "restore") restore_scenario(); + else if (scenario == "never-leased") never_leased_scenario(); + else if (scenario == "renewal") renewal_scenario(); + else if (scenario == "format") format_scenario(); + else if (scenario == "zero") zero_scenario(); + else if (scenario == "getter") getter_scenario(); + else throw std::runtime_error("unknown DNS scenario"); + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } +} diff --git a/test/test_cli_settings_contract.py b/test/test_cli_settings_contract.py index 9bd20c210..ad6915ae1 100644 --- a/test/test_cli_settings_contract.py +++ b/test/test_cli_settings_contract.py @@ -63,6 +63,7 @@ COMMON_QUERY_EXCEPTIONS = { 'wifi.pwd': 'Setter is in the observer or standalone-WiFi delegate', } OBSERVER_QUERY_EXCEPTIONS = { + 'link.dns': 'Live resolver list, gateway, and retained per-medium lease diagnostics', 'mqtt.config.valid': 'Configuration validation result', 'mqtt.enabled': 'Stored in common NodePrefs; set mqtt.enabled is handled by CommonCLI', 'mqtt.ntp.diag': 'NTP diagnostics', diff --git a/test/test_network_dns.py b/test/test_network_dns.py new file mode 100644 index 000000000..0119d6354 --- /dev/null +++ b/test/test_network_dns.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +"""Execute production DNS snapshots, formatters, and CLI wiring without a network. + +Only lwIP dispatch/address conversion and physical-link endpoints are mocked. +The real NetworkLink interface, complete shared base, and each DNS formatter +are compiled together, including the observer getter copied from production. +""" +from pathlib import Path +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/network_dns.cpp" + +IP_ADDRESS = r''' +#pragma once +#include +class IPAddress { + std::string text; + public: + IPAddress() : text("0.0.0.0") {} + explicit IPAddress(const char* value) : text(value) {} + std::string toString() const { return text; } +}; +''' + + +def production_harness(): + network = (ROOT / "src/helpers/NetworkLink.cpp").read_text(encoding="utf-8") + helpers = network[network.index("namespace {") + len("namespace {"): + network.index("class NetworkLinkBase")] + base = extract_braced(network, "class NetworkLinkBase") + ";" + code = FIXTURE.read_text(encoding="ascii").replace("@HELPERS@", helpers) + code = code.replace("@BASE@", base) + for name in ("WiFiNetworkLink", "EthernetNetworkLink", "AutomaticNetworkLink"): + implementation = extract_braced(network, "class " + name) + methods = extract_braced(implementation, "void formatDns(") + if name != "AutomaticNetworkLink": + methods += "\n" + extract_braced(implementation, "IPAddress gatewayIP()") + code = code.replace("@" + name + "@", methods) + cli = (ROOT / "src/helpers/CommonCLI_Observer.cpp").read_text(encoding="utf-8") + getter = extract_braced(cli, 'if (strcmp(config, "link.dns") == 0)') + return code.replace("@GETTER@", getter) + + +class NetworkDnsTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if cls.compiler is None: + raise unittest.SkipTest("a host C++17 compiler is required") + cls.temporary = tempfile.TemporaryDirectory(prefix="meshcore-network-dns-") + cls.addClassCleanup(cls.temporary.cleanup) + cls.directory = Path(cls.temporary.name) + (cls.directory / "Arduino.h").write_text("#pragma once\n", encoding="ascii") + (cls.directory / "IPAddress.h").write_text(IP_ADDRESS, encoding="ascii") + cls.source = production_harness() + cls.binary = cls.compile(cls.source, "production") + + @classmethod + def compile(cls, source, name): + cpp = cls.directory / (name + ".cpp") + binary = cls.directory / name + cpp.write_text(source, encoding="utf-8") + sanitizers = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie"] if sys.platform.startswith("linux") else []) + result = subprocess.run([ + cls.compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", *sanitizers, + "-DESP_PLATFORM=1", "-I", str(cls.directory), "-I", str(ROOT / "src"), + str(cpp), "-o", str(binary)], capture_output=True, text=True, timeout=60) + if result.returncode: + raise AssertionError(result.stdout + result.stderr) + return binary + + def run_scenario(self, scenario, binary=None): + result = subprocess.run([str(binary or self.binary), scenario], + capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_three_slot_snapshot_restore_and_empty_slot_clearing(self): + self.run_scenario("restore") + + def test_never_leased_medium_preserves_current_global_resolvers(self): + self.run_scenario("never-leased") + + def test_renewals_keep_last_valid_lease_and_replace_obsolete_slots(self): + self.run_scenario("renewal") + + def test_dns_lists_and_all_network_diagnostics_are_bounded(self): + self.run_scenario("format") + + def test_zero_capacity_and_null_destination_do_not_write(self): + self.run_scenario("zero") + + def test_observer_getter_uses_real_interface_and_exact_command(self): + self.run_scenario("getter") + cli = (ROOT / "src/helpers/CommonCLI_Observer.cpp").read_text(encoding="utf-8") + self.assertIn('#ifdef ESP_PLATFORM\n#include "NetworkLink.h"', cli) + self.assertIn('memcmp(config, "wifi.status", 11)', cli) + + def test_webconfig_mock_exposes_the_read_only_dns_getter(self): + sys.path.insert(0, str(ROOT / "scripts")) + try: + import webconfig_mock_server as portal + finally: + sys.path.pop(0) + self.assertEqual(portal.cli_get({}, "link.dns"), + (True, "> dns:192.168.1.1 gw:192.168.1.1 lease:192.168.1.1")) + page = (ROOT / "webui/index.html").read_text(encoding="utf-8") + self.assertRegex(page, r'\["link\.dns","[^"\n]+",1\]') + + def test_old_snapshot_and_restore_callbacks_lose_resolvers(self): + snapshot = extract_braced(self.source, "static esp_err_t snapshotDnsOnTcpipThread(") + # Reproduce the old write-before-validation callback. The retained + # self-copy on a valid lease has no effect; an empty renewal overwrites + # the snapshot before the captured flag can reject it. + old_snapshot = snapshot.replace("ip_addr_t servers[kDnsServers] = {};", "") + old_snapshot = old_snapshot.replace("servers[i]", "self->_dns_snapshot[i]") + controls = ( + (self.source.replace("kDnsServers = DNS_MAX_SERVERS;", "kDnsServers = 2;"), "restore"), + (self.source.replace("dns_setserver(i, &self->_dns_snapshot[i]);", + "if (!ip_addr_isany_val(self->_dns_snapshot[i])) " + "dns_setserver(i, &self->_dns_snapshot[i]);"), "restore"), + (self.source.replace(snapshot, old_snapshot), "renewal"), + ) + for index, (broken, scenario) in enumerate(controls): + with self.subTest(control=index): + self.assertNotEqual(broken, self.source) + binary = self.compile(broken, "broken-" + str(index)) + result = subprocess.run([str(binary), scenario], capture_output=True, + text=True, timeout=10) + self.assertNotEqual(result.returncode, 0) + self.assertIn("DNS-free renewal" if scenario == "renewal" else "DNS restore", + result.stderr) + + +if __name__ == "__main__": + unittest.main()