From 4d42212b9ecdfe5a44e77832aa588046acf9f6ff Mon Sep 17 00:00:00 2001 From: Sergii Bogomolov Date: Thu, 9 Jul 2026 03:11:27 +0200 Subject: [PATCH 1/3] test: consolidate duplicated matrices and drop dead test surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config and application suites re-instantiated one shared template's behavior per protocol by hand-copying near-identical cases. Replace those with value-parameterized suites over {wol, mdns, ssdp}, so every protocol stays deliberately covered — robust to the template ever being split apart — from one test source: - config duplicate-rule matrix: one TEST_P suite × 3 protocols (the port-dependent corners stay WoL-only); the per-protocol smoke tests fold into it. ConfigCount fails loudly on an unhandled protocol. - application invalid-socket tests: one TEST_P over {protocol × source /target}; drop the SSDP rollback duplicate (the clear() is one unconditional path already pinned by the mDNS case). - udp_socket: the five moved-from/closed guards become one table test. Dead surface removed: the unused LoopbackReceiver helper. Two would-be cuts kept as sharper tests instead: the mixed-case MAC parse now asserts every byte (pins per-nibble, not per-byte, case folding), and the static-array logger name is retargeted to a non-NUL-terminated array (the one StaticStringLength branch a literal can't reach). --- tests/application_test.cpp | 163 +++++------- tests/config_test.cpp | 490 +++++++++---------------------------- tests/logger_test.cpp | 13 +- tests/mac_address_test.cpp | 14 +- tests/test_helpers.h | 40 --- tests/udp_socket_test.cpp | 55 ++--- 6 files changed, 213 insertions(+), 562 deletions(-) diff --git a/tests/application_test.cpp b/tests/application_test.cpp index b72d978..56004f7 100644 --- a/tests/application_test.cpp +++ b/tests/application_test.cpp @@ -218,36 +218,76 @@ TEST_F(ApplicationTest, CreatesDistinctSocketsForDistinctInterfaces) { EXPECT_EQ(dispatcher_->RegistrationCount(), 2); // two distinct source fds watched } -TEST_F(ApplicationTest, FailsWhenSourceSocketInvalid) { - ConfigureSocket("bad-src", {.valid = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeWolConfig("tv", "bad-src", "dst", {9})) - .Build(); +// Which side of the reflector (source or target) gets the invalid socket. +enum class SocketRole { Source, Target }; + +using ReflectorConfigFactory = Config (*)(std::string_view source_if, std::string_view target_if); + +struct InvalidSocketParam { + std::string_view protocol; + ReflectorConfigFactory make_config; + SocketRole role; + + // gtest prints each param to annotate --gtest_list_tests output. Without a printer it falls back + // to RawBytesPrinter, which formats the struct's trailing padding -- uninitialised, so memcheck + // (the Valgrind gate) flags it. A hidden friend is found by gtest via ADL and reads only the + // initialised fields. + friend void PrintTo(const InvalidSocketParam& param, std::ostream* os) { + *os << param.protocol << (param.role == SocketRole::Source ? "/source" : "/target"); + } +}; - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); +// Value-parameterized over {protocol, source-or-target}: ConfigureReflectors's invalid-socket +// branch (Application::ConfigureReflectors in application.cpp) is one code path shared by every +// protocol via the ReflectorType/ConfigType template parameters, so this replaces six near- +// identical TEST_Fs (one pair per protocol) with a single test body instantiated six times — every +// protocol stays covered even if the template is later split apart. +class ApplicationInvalidSocketTest : public ApplicationTest, + public ::testing::WithParamInterface { +public: + static Config WolReflectorConfig(std::string_view source_if, std::string_view target_if) { + return TestConfigBuilder{}.Add(MakeWolConfig("tv", source_if, target_if, {9})).Build(); + } - EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("bad-src"), std::string::npos) << output; // the log names the source interface -} + static Config MdnsReflectorConfig(std::string_view source_if, std::string_view target_if) { + return TestConfigBuilder{}.Add(MakeMdnsConfig("cast", source_if, target_if)).Build(); + } + + static Config SsdpReflectorConfig(std::string_view source_if, std::string_view target_if) { + return TestConfigBuilder{}.Add(MakeSsdpConfig("cast", source_if, target_if)).Build(); + } +}; -TEST_F(ApplicationTest, FailsWhenTargetSocketInvalid) { - ConfigureSocket("bad-dst", {.valid = false}); +TEST_P(ApplicationInvalidSocketTest, FailsAndLogsTheInterface) { + const auto& param = GetParam(); + const bool bad_is_source = param.role == SocketRole::Source; + const std::string_view bad_if = bad_is_source ? "bad-src" : "bad-dst"; + ConfigureSocket(bad_if, {.valid = false}); auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeWolConfig("tv", "src", "bad-dst", {9})) - .Build(); + const Config config = bad_is_source ? param.make_config(bad_if, "dst") : param.make_config("src", bad_if); const std::string output = CaptureStdout([&] { EXPECT_FALSE(app.Configure(config)); }); EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("bad-dst"), std::string::npos) << output; // the log names the target interface + EXPECT_NE(output.find(param.protocol), std::string::npos) << output; // the log names the protocol + EXPECT_NE(output.find(bad_if), std::string::npos) << output; // and the invalid interface } +INSTANTIATE_TEST_SUITE_P(Protocols, ApplicationInvalidSocketTest, + ::testing::Values( + InvalidSocketParam{"wol", &ApplicationInvalidSocketTest::WolReflectorConfig, SocketRole::Source}, + InvalidSocketParam{"wol", &ApplicationInvalidSocketTest::WolReflectorConfig, SocketRole::Target}, + InvalidSocketParam{"mdns", &ApplicationInvalidSocketTest::MdnsReflectorConfig, SocketRole::Source}, + InvalidSocketParam{"mdns", &ApplicationInvalidSocketTest::MdnsReflectorConfig, SocketRole::Target}, + InvalidSocketParam{"ssdp", &ApplicationInvalidSocketTest::SsdpReflectorConfig, SocketRole::Source}, + InvalidSocketParam{"ssdp", &ApplicationInvalidSocketTest::SsdpReflectorConfig, SocketRole::Target}), + [](const ::testing::TestParamInfo& param_info) { + return std::string{param_info.param.protocol} + + (param_info.param.role == SocketRole::Source ? "Source" : "Target"); + }); + TEST_F(ApplicationTest, FailsWhenReflectorSetupFails) { // The target socket is valid but can't originate IPv4, so the (IPv4) reflector fails to // initialize even though both socket checks pass. @@ -315,37 +355,6 @@ TEST_F(ApplicationTest, SharesSocketsBetweenWolAndMdns) { EXPECT_EQ(dispatcher_->RegistrationCount(), 2); } -TEST_F(ApplicationTest, FailsWhenMdnsSourceSocketInvalid) { - ConfigureSocket("bad-src", {.valid = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeMdnsConfig("cast", "bad-src", "dst")) - .Build(); - - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); - - EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("mdns"), std::string::npos) << output; // the log names the protocol - EXPECT_NE(output.find("bad-src"), std::string::npos) << output; // and the source interface -} - -TEST_F(ApplicationTest, FailsWhenMdnsTargetSocketInvalid) { - ConfigureSocket("bad-dst", {.valid = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeMdnsConfig("cast", "src", "bad-dst")) - .Build(); - - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); - - EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("bad-dst"), std::string::npos) << output; // the log names the target interface -} - TEST_F(ApplicationTest, FailsWhenMdnsReflectorSetupFails) { // mDNS needs the family sendable on both interfaces; the IPv4 config can't be reflected when // the target can't originate IPv4, so the reflector fails to initialize. @@ -417,37 +426,6 @@ TEST_F(ApplicationTest, SharesSocketsAcrossAllProtocols) { EXPECT_EQ(dispatcher_->RegistrationCount(), 2); } -TEST_F(ApplicationTest, FailsWhenSsdpSourceSocketInvalid) { - ConfigureSocket("bad-src", {.valid = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeSsdpConfig("cast", "bad-src", "dst")) - .Build(); - - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); - - EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("ssdp"), std::string::npos) << output; // the log names the protocol - EXPECT_NE(output.find("bad-src"), std::string::npos) << output; // and the source interface -} - -TEST_F(ApplicationTest, FailsWhenSsdpTargetSocketInvalid) { - ConfigureSocket("bad-dst", {.valid = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeSsdpConfig("cast", "src", "bad-dst")) - .Build(); - - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); - - EXPECT_EQ(ReflectorCount(app), 0); - EXPECT_NE(output.find("bad-dst"), std::string::npos) << output; // the log names the target interface -} - TEST_F(ApplicationTest, FailsWhenSsdpReflectorSetupFails) { // SSDP needs the family sendable on both interfaces; the IPv4 config can't be reflected when the // target can't originate IPv4, so the reflector fails to initialize. @@ -464,24 +442,6 @@ TEST_F(ApplicationTest, FailsWhenSsdpReflectorSetupFails) { EXPECT_EQ(ReflectorCount(app), 0) << output; } -TEST_F(ApplicationTest, ClearsEarlierReflectorsWhenSsdpFails) { - // SSDP is configured last; a source that can't originate IPv4 breaks SSDP (which reflects in - // both directions) but not the WoL reflector wired before it. Configure is transactional, so the - // earlier success is rolled back, leaving nothing wired. - ConfigureSocket("src", {.can_send_v4 = false}); - auto app = MakeApp(); - const Config config = TestConfigBuilder{} - .Add(MakeWolConfig("tv", "src", "dst", {9})) - .Add(MakeSsdpConfig("cast", "src", "dst")) - .Build(); - - const std::string output = CaptureStdout([&] { - EXPECT_FALSE(app.Configure(config)); - }); - - EXPECT_EQ(ReflectorCount(app), 0) << output; // the WoL reflector wired earlier is rolled back -} - TEST_F(ApplicationTest, SubscribesToTheAddressMonitor) { auto app = MakeApp(); @@ -529,8 +489,11 @@ TEST_F(ApplicationTest, NotifiesReflectorsWhenAnInterfaceChanges) { Iface("dst")->SetHasSource(IpAddress::Family::V6, true); // a v6 address appears on the target const std::string output = CaptureStdout([&] { monitor_->FireChange(9); }); - EXPECT_NE(output.find(std::format("Starting {} reflection", IpAddress::Family::V6)), - std::string::npos) << output; + // Pin only the stable tail of family_capability.cpp's wording, not the full + // "Starting {} reflection: ..." sentence: e2e/run.py's ADDR_CHANGE_EXPECTED_ERROR pins the + // complementary "a source address is no longer available" tail for the loss case, so keeping + // to just the tail here means a prefix wording change doesn't need fixing in three places. + EXPECT_NE(output.find("a source address is available"), std::string::npos) << output; } TEST_F(ApplicationTest, RefreshesAllInterfacesOnOverflowSignal) { diff --git a/tests/config_test.cpp b/tests/config_test.cpp index 4f54278..9bd1232 100644 --- a/tests/config_test.cpp +++ b/tests/config_test.cpp @@ -827,210 +827,157 @@ wol = true })).has_value()); } -TEST(ConfigTest, RejectsDuplicateMacSourceTargetTriple) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] +// --- Cross-protocol duplicate-rule matrix --- +// AppendConfig implements duplicate detection once, but each protocol is a separate +// instantiation over its own config vector: the matrix runs against every protocol so each +// protocol's contract stays pinned even if the shared template is ever split apart. The +// port-dependent corners are WoL-only and live in their own section further down. +class ConfigDedupMatrixTest : public ::testing::TestWithParam { +protected: + // Two-entry TOML enabling the parameterized protocol on both entries; `a` and `b` carry each + // entry's remaining fields (mac / interfaces / address_family) as raw TOML lines. + [[nodiscard]] std::string TwoEntries(std::string_view a, std::string_view b) const { + return std::format("[reflectors.a]{1}\n{0} = true\n\n[reflectors.b]{2}\n{0} = true\n", + GetParam(), a, b); + } + + [[nodiscard]] size_t ConfigCount(const Config& config) const { + if (GetParam() == "wol") { + return config.WolConfigs().size(); + } + if (GetParam() == "mdns") { + return config.MdnsConfigs().size(); + } + if (GetParam() == "ssdp") { + return config.SsdpConfigs().size(); + } + // A protocol added to the instantiation must extend this dispatch, not silently count some + // other vector and pass. + ADD_FAILURE() << "ConfigCount has no case for protocol \"" << GetParam() << "\""; + return 0; + } + + // The pair must be rejected as this protocol's duplicate — the message names the protocol, + // which also pins AppendConfig's error text (WoL runs its ports-mentioning branch, the other + // protocols the ports-less one). + void ExpectDuplicate(std::string_view a, std::string_view b) const { + const auto config = Config::FromString(TwoEntries(a, b)); + ASSERT_FALSE(config.has_value()); + EXPECT_NE(config.error().Message().find(std::format("duplicate {} rule", GetParam())), + std::string::npos) << config.error().Message(); + } + + void ExpectBothAccepted(std::string_view a, std::string_view b) const { + const auto config = Config::FromString(TwoEntries(a, b)); + ASSERT_TRUE(config.has_value()) << config.error().Message(); + EXPECT_EQ(ConfigCount(*config), 2u); + } +}; + +INSTANTIATE_TEST_SUITE_P(Protocols, ConfigDedupMatrixTest, + ::testing::Values(std::string_view{"wol"}, std::string_view{"mdns"}, std::string_view{"ssdp"}), + [](const ::testing::TestParamInfo& param_info) { return std::string{param_info.param}; }); + +TEST_P(ConfigDedupMatrixTest, RejectsDuplicateMacSourceTargetTriple) { + ExpectDuplicate(R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth1" -wol = true - -[reflectors.b] +target_if = "eth1")", R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth1" -wol = true -)").has_value()); +target_if = "eth1")"); } -TEST(ConfigTest, RejectsDuplicateUnfilteredSourceTargetRule) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] +TEST_P(ConfigDedupMatrixTest, RejectsDuplicateUnfilteredSourceTargetRule) { + ExpectDuplicate(R"( source_if = "eth0" -target_if = "eth1" -wol = true - -[reflectors.b] +target_if = "eth1")", R"( source_if = "eth0" -target_if = "eth1" -wol = true -)").has_value()); +target_if = "eth1")"); } -TEST(ConfigTest, RejectsSpecificRuleOverlappedByUnfilteredRule) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] +TEST_P(ConfigDedupMatrixTest, RejectsSpecificRuleOverlappedByUnfilteredRule) { + ExpectDuplicate(R"( source_if = "eth0" -target_if = "eth1" -wol = true - -[reflectors.b] +target_if = "eth1")", R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth1" -wol = true -)").has_value()); +target_if = "eth1")"); } -TEST(ConfigTest, AcceptsSameMacWithDifferentTargets) { - const auto config = Config::FromString(R"( -[reflectors.a] +// "default" handles IPv4 too, so it overlaps an ipv4-only rule on the same triple. +TEST_P(ConfigDedupMatrixTest, RejectsOverlappingRuleWhenDefaultCoversIpv4) { + ExpectDuplicate(R"( mac = "00:11:22:33:44:55" source_if = "eth0" target_if = "eth1" -wol = true - -[reflectors.b] +address_family = "default")", R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth2" -wol = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->WolConfigs().size(), 2); +target_if = "eth1" +address_family = "ipv4")"); } -TEST(ConfigTest, AcceptsOverlappingMacSelectionWithDisjointPorts) { - const auto config = Config::FromString(R"( -[reflectors.a] +TEST_P(ConfigDedupMatrixTest, RejectsDuplicateIpv6OnlyRules) { + // Two ipv6-only rules on the same triple collide via the IPv6 disjunct of AddressFamiliesOverlap + // (the IPv4 disjunct is false for both) -- the mirror of the default/ipv4 case above. + ExpectDuplicate(R"( source_if = "eth0" target_if = "eth1" -wol = true -wol_ports = [7] - -[reflectors.b] -mac = "00:11:22:33:44:55" +address_family = "ipv6")", R"( source_if = "eth0" target_if = "eth1" -wol = true -wol_ports = [9] -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->WolConfigs().size(), 2); +address_family = "ipv6")"); } -TEST(ConfigTest, AcceptsOverlappingMacSelectionWithDifferentSources) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "eth0" -target_if = "eth1" -wol = true - -[reflectors.b] +TEST_P(ConfigDedupMatrixTest, AcceptsDistinctConcreteMacs) { + // Two different concrete MACs on the same source/target do not collide: MacSelectionsOverlap + // is false only when both are set and unequal. + ExpectBothAccepted(R"( mac = "00:11:22:33:44:55" -source_if = "eth2" -target_if = "eth1" -wol = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->WolConfigs().size(), 2); -} - -TEST(ConfigTest, AcceptsOverlappingMacSelectionWithDifferentTargets) { - const auto config = Config::FromString(R"( -[reflectors.a] source_if = "eth0" -target_if = "eth1" -wol = true - -[reflectors.b] -mac = "00:11:22:33:44:55" +target_if = "eth1")", R"( +mac = "00:11:22:33:44:66" source_if = "eth0" -target_if = "eth2" -wol = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->WolConfigs().size(), 2); +target_if = "eth1")"); } -// An ipv4-only and an ipv6-only rule never handle the same packet, so an otherwise identical pair -// is not a duplicate — it is just the long form of one "dual" rule. -TEST(ConfigTest, AcceptsIdenticalRuleWithDisjointAddressFamilies) { - const auto config = Config::FromString(R"( -[reflectors.a] -mac = "00:11:22:33:44:55" +TEST_P(ConfigDedupMatrixTest, AcceptsOverlappingMacSelectionWithDifferentSources) { + ExpectBothAccepted(R"( source_if = "eth0" -target_if = "eth1" -wol = true -address_family = "ipv4" - -[reflectors.b] +target_if = "eth1")", R"( mac = "00:11:22:33:44:55" -source_if = "eth0" -target_if = "eth1" -wol = true -address_family = "ipv6" -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->WolConfigs().size(), 2); +source_if = "eth2" +target_if = "eth1")"); } -// "default" handles IPv4 too, so it overlaps an ipv4-only rule on the same triple. -TEST(ConfigTest, RejectsOverlappingRuleWhenDefaultCoversIpv4) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] +TEST_P(ConfigDedupMatrixTest, AcceptsSameMacWithDifferentTargets) { + ExpectBothAccepted(R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth1" -wol = true -address_family = "default" - -[reflectors.b] +target_if = "eth1")", R"( mac = "00:11:22:33:44:55" source_if = "eth0" -target_if = "eth1" -wol = true -address_family = "ipv4" -)").has_value()); +target_if = "eth2")"); } -TEST(ConfigTest, RejectsDuplicateIpv6OnlyRules) { - // Two ipv6-only rules on the same triple collide via the IPv6 disjunct of AddressFamiliesOverlap - // (the IPv4 disjunct is false for both) -- the mirror of the default/ipv4 case above. - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] +// An ipv4-only and an ipv6-only rule never handle the same packet, so an otherwise identical pair +// is not a duplicate — it is just the long form of one "dual" rule. +TEST_P(ConfigDedupMatrixTest, AcceptsIdenticalRuleWithDisjointAddressFamilies) { + ExpectBothAccepted(R"( +mac = "00:11:22:33:44:55" source_if = "eth0" target_if = "eth1" -wol = true -address_family = "ipv6" - -[reflectors.b] +address_family = "ipv4")", R"( +mac = "00:11:22:33:44:55" source_if = "eth0" target_if = "eth1" -wol = true -address_family = "ipv6" -)").has_value()); +address_family = "ipv6")"); } -// --- per-protocol dedup across entries (mDNS / SSDP) and cross-protocol independence --- +// --- cross-protocol independence: the same rule under different protocols never collides --- -TEST(ConfigTest, RejectsDuplicateMdnsRuleAcrossEntries) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true -[reflectors.b] -source_if = "lan" -target_if = "iot" -mdns = true -)").has_value()); -} - -TEST(ConfigTest, RejectsDuplicateSsdpRuleAcrossEntries) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true - -[reflectors.b] -source_if = "lan" -target_if = "iot" -ssdp = true -)").has_value()); -} TEST(ConfigTest, AcceptsOverlappingDifferentProtocolsAcrossEntries) { // Same source/target, but one entry does WoL and the other mDNS — different protocol vectors, @@ -1295,23 +1242,22 @@ ssdp = true )").has_value()); } -// --- WoL dedup: branches not exercised by the migrated matrix --- +// --- WoL dedup: the port-dependent branches the cross-protocol matrix cannot reach --- -TEST(ConfigTest, AcceptsDistinctConcreteMacsSameTriple) { - // Two different concrete MACs on the same source/target/ports do not collide: MacSelectionsOverlap - // is false only when both are set and unequal. +TEST(ConfigTest, AcceptsOverlappingMacSelectionWithDisjointPorts) { const auto config = Config::FromString(R"( [reflectors.a] -mac = "00:11:22:33:44:55" source_if = "eth0" target_if = "eth1" wol = true +wol_ports = [7] [reflectors.b] -mac = "00:11:22:33:44:66" +mac = "00:11:22:33:44:55" source_if = "eth0" target_if = "eth1" wol = true +wol_ports = [9] )"); ASSERT_TRUE(config.has_value()) << config.error().Message(); EXPECT_EQ(config->WolConfigs().size(), 2); @@ -1319,8 +1265,8 @@ wol = true TEST(ConfigTest, RejectsPartialPortOverlap) { // Sharing a single port (9) is enough to collide — PortsOverlap is any-shared-element, not set - // equality. - EXPECT_FALSE(Config::FromString(R"( + // equality. The error runs AppendConfig's WoL-only branch, whose message lists the ports. + const auto config = Config::FromString(R"( [reflectors.a] source_if = "eth0" target_if = "eth1" @@ -1332,242 +1278,24 @@ source_if = "eth0" target_if = "eth1" wol = true wol_ports = [9, 40000] -)").has_value()); -} - -// --- mDNS dedup matrix (AppendConfig dedups mdns_configs independently of WoL) --- - -TEST(ConfigTest, RejectsMdnsDuplicateMacTriple) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -mdns = true - -[reflectors.b] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -mdns = true -)").has_value()); -} - -TEST(ConfigTest, RejectsMdnsSpecificOverlappedByUnfiltered) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true - -[reflectors.b] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -mdns = true -)").has_value()); -} - -TEST(ConfigTest, AcceptsMdnsDistinctMacs) { - const auto config = Config::FromString(R"( -[reflectors.a] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -mdns = true - -[reflectors.b] -mac = "00:11:22:33:44:66" -source_if = "lan" -target_if = "iot" -mdns = true )"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->MdnsConfigs().size(), 2); + ASSERT_FALSE(config.has_value()); + EXPECT_NE(config.error().Message().find("ports"), std::string::npos) << config.error().Message(); } -TEST(ConfigTest, AcceptsMdnsDifferentSources) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true -[reflectors.b] -source_if = "lan2" -target_if = "iot" -mdns = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->MdnsConfigs().size(), 2); -} -TEST(ConfigTest, AcceptsMdnsDifferentTargets) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true -[reflectors.b] -source_if = "lan" -target_if = "iot2" -mdns = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->MdnsConfigs().size(), 2); -} -TEST(ConfigTest, AcceptsMdnsDisjointAddressFamilies) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true -address_family = "ipv4" -[reflectors.b] -source_if = "lan" -target_if = "iot" -mdns = true -address_family = "ipv6" -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->MdnsConfigs().size(), 2); -} -TEST(ConfigTest, RejectsMdnsDefaultOverlapsIpv4) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -mdns = true -address_family = "default" -[reflectors.b] -source_if = "lan" -target_if = "iot" -mdns = true -address_family = "ipv4" -)").has_value()); -} -// --- SSDP dedup matrix (AppendConfig dedups ssdp_configs independently of WoL / mDNS) --- -TEST(ConfigTest, RejectsSsdpDuplicateMacTriple) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -ssdp = true -[reflectors.b] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -ssdp = true -)").has_value()); -} - -TEST(ConfigTest, RejectsSsdpSpecificOverlappedByUnfiltered) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true - -[reflectors.b] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -ssdp = true -)").has_value()); -} - -TEST(ConfigTest, AcceptsSsdpDistinctMacs) { - const auto config = Config::FromString(R"( -[reflectors.a] -mac = "00:11:22:33:44:55" -source_if = "lan" -target_if = "iot" -ssdp = true - -[reflectors.b] -mac = "00:11:22:33:44:66" -source_if = "lan" -target_if = "iot" -ssdp = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->SsdpConfigs().size(), 2); -} - -TEST(ConfigTest, AcceptsSsdpDifferentSources) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true - -[reflectors.b] -source_if = "lan2" -target_if = "iot" -ssdp = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->SsdpConfigs().size(), 2); -} -TEST(ConfigTest, AcceptsSsdpDifferentTargets) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true -[reflectors.b] -source_if = "lan" -target_if = "iot2" -ssdp = true -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->SsdpConfigs().size(), 2); -} -TEST(ConfigTest, AcceptsSsdpDisjointAddressFamilies) { - const auto config = Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true -address_family = "ipv4" - -[reflectors.b] -source_if = "lan" -target_if = "iot" -ssdp = true -address_family = "ipv6" -)"); - ASSERT_TRUE(config.has_value()) << config.error().Message(); - EXPECT_EQ(config->SsdpConfigs().size(), 2); -} - -TEST(ConfigTest, RejectsSsdpDefaultOverlapsIpv4) { - EXPECT_FALSE(Config::FromString(R"( -[reflectors.a] -source_if = "lan" -target_if = "iot" -ssdp = true -address_family = "default" - -[reflectors.b] -source_if = "lan" -target_if = "iot" -ssdp = true -address_family = "ipv4" -)").has_value()); -} // --- SSDP dial flag: struct-level Verify + formatter, and the TOML parse --- diff --git a/tests/logger_test.cpp b/tests/logger_test.cpp index ddd2da4..cdeeab3 100644 --- a/tests/logger_test.cpp +++ b/tests/logger_test.cpp @@ -63,16 +63,19 @@ TEST(LoggerTest, StaticLiteralNameAppearsInOutput) { EXPECT_NE(output.find("message from static literal logger"), std::string::npos) << output; } -TEST(LoggerTest, StaticArrayNameAppearsInOutput) { +TEST(LoggerTest, NonNullTerminatedArrayNameAppearsInOutput) { + // A char array without a trailing NUL: StaticStringLength takes its `: N` branch (a terminated + // array and any string literal take `N - 1`), so the whole extent is the name — the one case + // that distinguishes the array ctor from the literal ctor. const ScopedMinLogLevel level{LogLevel::Info}; - Logger logger{STATIC_ARRAY_NAME}; + constexpr char name[] = {'N', 'o', 'N', 'u', 'l'}; + Logger logger{name}; const std::string output = CaptureStdout([&] { - logger.Info("message from static array logger"); + logger.Info("message from unterminated array logger"); }); - EXPECT_NE(output.find(STATIC_ARRAY_NAME), std::string::npos) << output; - EXPECT_NE(output.find("message from static array logger"), std::string::npos) << output; + EXPECT_NE(output.find("[NoNul]"), std::string::npos) << output; } TEST(LoggerTest, DynamicTemporaryNameAppearsInOutput) { diff --git a/tests/mac_address_test.cpp b/tests/mac_address_test.cpp index cdef513..66ebc32 100644 --- a/tests/mac_address_test.cpp +++ b/tests/mac_address_test.cpp @@ -32,8 +32,18 @@ TEST(MacAddressTest, ParsesUppercaseAddress) { EXPECT_EQ(bytes[5], std::byte{0xbe}); } -TEST(MacAddressTest, ParsesMixedCaseAddress) { - EXPECT_TRUE(MacAddress::FromString("aA:bB:cC:dD:eE:fF").has_value()); +TEST(MacAddressTest, ParsesMixedCaseWithinAByte) { + // Both nibbles of each byte differ in case (aA, bB, ...) — the lowercase/uppercase tests use + // uniform case per byte, so only this pins that case-folding is per nibble, not per byte. + const auto mac = MacAddress::FromString("aA:bB:cC:dD:eE:fF"); + ASSERT_TRUE(mac.has_value()); + const auto& bytes = mac->Bytes(); + EXPECT_EQ(bytes[0], std::byte{0xaa}); + EXPECT_EQ(bytes[1], std::byte{0xbb}); + EXPECT_EQ(bytes[2], std::byte{0xcc}); + EXPECT_EQ(bytes[3], std::byte{0xdd}); + EXPECT_EQ(bytes[4], std::byte{0xee}); + EXPECT_EQ(bytes[5], std::byte{0xff}); } TEST(MacAddressTest, DefaultInitializesToZero) { diff --git a/tests/test_helpers.h b/tests/test_helpers.h index eeb808f..f5532d3 100644 --- a/tests/test_helpers.h +++ b/tests/test_helpers.h @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -25,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -392,42 +390,4 @@ struct TestCaptureSocket { } }; -struct LoopbackReceiver { - UdpSocket socket; - PacketRecorder recorder; - - explicit LoopbackReceiver(uint16_t port, IpAddress::Family family) : socket{family} { - BindLoopback(socket, port); - } - - [[nodiscard]] uint16_t Port() const { return BoundPort(socket); } - - bool PollOnce(std::chrono::milliseconds timeout) { - pollfd pfd{.fd = socket.Fd(), .events = POLLIN, .revents = 0}; - const auto ready = ::poll(&pfd, 1, static_cast(timeout.count())); - if (ready <= 0) { - return false; - } - - std::vector buffer(64 * 1024); - sockaddr_storage source{}; - socklen_t source_size = sizeof(source); - const auto bytes = ::recvfrom(socket.Fd(), buffer.data(), buffer.size(), 0, - reinterpret_cast(&source), &source_size); - if (bytes <= 0) { - return false; - } - const auto source_ip = IpAddress::FromSockaddr(reinterpret_cast(&source)); - Packet packet{ - .header = PacketHeader{ - .source = {source_ip.value_or(LoopbackFor(socket.AddressFamily()))}, - .dest = {LoopbackFor(socket.AddressFamily())}, - }, - .payload = std::span{buffer.data(), static_cast(bytes)}, - }; - recorder.OnPacket(packet); - return true; - } -}; - } // namespace reflector diff --git a/tests/udp_socket_test.cpp b/tests/udp_socket_test.cpp index 48f5f5d..e8748a1 100644 --- a/tests/udp_socket_test.cpp +++ b/tests/udp_socket_test.cpp @@ -156,33 +156,29 @@ TEST(UdpSocketTest, SetBroadcastOnValidSocketSucceeds) { EXPECT_TRUE(sock.SetBroadcast(true)); } -TEST(UdpSocketTest, SetBroadcastOnMovedFromSocketFails) { +TEST(UdpSocketTest, OperationsOnMovedFromSocketFail) { UdpSocket src{IpAddress::Family::V4}; UdpSocket dst{std::move(src)}; - - EXPECT_FALSE(src.SetBroadcast(true)); // NOLINT(bugprone-use-after-move) -} - -TEST(UdpSocketTest, SetReuseAddrOnMovedFromSocketFails) { - UdpSocket src{IpAddress::Family::V4}; - UdpSocket dst{std::move(src)}; - - EXPECT_FALSE(src.SetReuseAddr(true)); // NOLINT(bugprone-use-after-move) -} - -TEST(UdpSocketTest, BindOnMovedFromSocketFails) { - UdpSocket src{IpAddress::Family::V4}; - UdpSocket dst{std::move(src)}; - - EXPECT_FALSE(src.Bind(0)); // NOLINT(bugprone-use-after-move) -} - -TEST(UdpSocketTest, SendToOnMovedFromSocketFails) { - UdpSocket src{IpAddress::Family::V4}; - UdpSocket dst{std::move(src)}; - - const std::array payload{std::byte{0x01}}; - EXPECT_FALSE(src.SendTo(payload, {IpAddress::LoopbackV4(), 9})); // NOLINT(bugprone-use-after-move) + ASSERT_FALSE(src.IsValid()); // NOLINT(bugprone-use-after-move) + + struct Op { + const char* name; + bool (*run)(UdpSocket&); + }; + // SetBroadcast, SetReuseAddr, Bind, and SendTo all share the single IsValid() guard. + const std::array ops{{ + {"SetBroadcast", [](UdpSocket& s) { return s.SetBroadcast(true); }}, + {"SetReuseAddr", [](UdpSocket& s) { return s.SetReuseAddr(true); }}, + {"Bind", [](UdpSocket& s) { return s.Bind(0); }}, + {"SendTo", + [](UdpSocket& s) { + return s.SendTo(std::array{std::byte{0x01}}, {IpAddress::LoopbackV4(), 9}); + }}, + }}; + + for (const auto& op : ops) { + EXPECT_FALSE(op.run(src)) << op.name; // NOLINT(bugprone-use-after-move) + } } TEST(UdpSocketTest, CloseInvalidatesSocket) { @@ -201,15 +197,6 @@ TEST(UdpSocketTest, CloseIsIdempotent) { EXPECT_FALSE(sock.IsValid()); } -TEST(UdpSocketTest, OperationsOnClosedSocketFail) { - UdpSocket sock{IpAddress::Family::V4}; - sock.Close(); - - EXPECT_FALSE(sock.SetBroadcast(true)); - EXPECT_FALSE(sock.SetReuseAddr(true)); - EXPECT_FALSE(sock.Bind(0)); -} - TEST(UdpSocketTest, SetV6OnlySucceedsOnV6Socket) { UdpSocket sock{IpAddress::Family::V6}; ASSERT_TRUE(sock.IsValid()); From a957ea7e32f349b54f59d3893f040fb0bbfe7875 Mon Sep 17 00:00:00 2001 From: Sergii Bogomolov Date: Thu, 9 Jul 2026 03:21:43 +0200 Subject: [PATCH 2/3] test: cover the BSD getifaddrs helpers via a detail:: split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macOS/FreeBSD resolver's pure helpers had no coverage at any layer — only the whole resolver, which needs a real interface. Split the testable logic into detail:: functions (mirroring SelectSourceAddresses) and unit-test them: - MacFromLinkSockaddr (was ReadMac): reads the MAC at the name-length- dependent sdl_data offset; tested for the offset skip, the no-address (alen 0) case, and refusing a MAC that runs past sa_len. - Ipv6SourceFlagsUsable: the SIOCGIFAFLAG_IN6 bitmask decision extracted from IsUsableIpv6 (the ioctl wrapper keeps calling it); tested for a usable address and each unusable flag bit. - CanonicalizeLinkLocalV6: the KAME embedded-scope-id clearing; tested that it zeroes only bytes 2-3 and leaves a canonical address alone. Behavior-preserving — the resolver calls the same code through the new seams. Tests are BSD-lane only (the helpers are non-Linux). --- src/reflector/interface_address.cpp | 77 ++++++++++++------------ src/reflector/interface_address.h | 20 +++++++ tests/interface_address_test.cpp | 91 +++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 37 deletions(-) diff --git a/src/reflector/interface_address.cpp b/src/reflector/interface_address.cpp index 16bdad5..72bbd62 100644 --- a/src/reflector/interface_address.cpp +++ b/src/reflector/interface_address.cpp @@ -259,27 +259,10 @@ void ResolveViaNetlink(unsigned index, InterfaceAddresses& result) noexcept { #else -// The interface's link-layer MAC from a sockaddr_dl, or a default (all-zero) MacAddress if it -// has none. Reads straight out of the kernel-provided sockaddr rather than copying the whole -// (variable-length) sockaddr into a fixed local: the MAC sits at a name-dependent offset in -// sdl_data that a long interface name could push past any local sockaddr_dl. Byte access -// through std::byte* is alignment- and aliasing-safe. -MacAddress ReadMac(const ifaddrs& ifa) noexcept { - const auto* bytes = reinterpret_cast(ifa.ifa_addr); - const auto sa_len = static_cast(ifa.ifa_addr->sa_len); - const auto name_len = std::to_integer(bytes[offsetof(sockaddr_dl, sdl_nlen)]); - const auto addr_len = std::to_integer(bytes[offsetof(sockaddr_dl, sdl_alen)]); - const auto mac_offset = offsetof(sockaddr_dl, sdl_data) + name_len; - if (addr_len != MAC_SIZE || mac_offset + MAC_SIZE > sa_len) { - return {}; - } - return MacAddress::FromBytes(std::span{bytes + mac_offset, MAC_SIZE}); -} - -// True only if the kernel confirms the IPv6 address is a valid source — not tentative, -// deprecated, duplicated, etc. Queried via SIOCGIFAFLAG_IN6 since getifaddrs doesn't surface -// per-address flags. If the ioctl fails — most likely because the address was removed between -// enumeration and now — we can't confirm it's usable, so we skip it rather than risk a +// True only if the kernel confirms the IPv6 address is a valid source. Queried via SIOCGIFAFLAG_IN6 +// since getifaddrs doesn't surface per-address flags; the usable/unusable decision itself is +// detail::Ipv6SourceFlagsUsable. If the ioctl fails — most likely because the address was removed +// between enumeration and now — we can't confirm it's usable, so we skip it rather than risk a // tentative or stale source. bool IsUsableIpv6(int inet6_fd, const char* interface, const sockaddr_in6& sin6) noexcept { in6_ifreq request{}; @@ -289,20 +272,7 @@ bool IsUsableIpv6(int inet6_fd, const char* interface, const sockaddr_in6& sin6) GetLogger().Warning("Cannot query IPv6 address flags on interface \"{}\": {}", interface, Error::FromErrno()); return false; } - constexpr int unusable = IN6_IFF_TENTATIVE | IN6_IFF_DUPLICATED | IN6_IFF_DETACHED - | IN6_IFF_DEPRECATED | IN6_IFF_ANYCAST; - return (request.ifr_ifru.ifru_flags6 & unusable) == 0; -} - -// BSD/KAME embeds the scope id (interface index) in bytes 2-3 of a link-local sin6_addr; -// clear it so the on-wire source is the canonical fe80::. macOS-only: Linux's -// netlink returns canonical link-local addresses, so the same clearing there would be a no-op -// at best and corrupt a non-conforming address at worst. -IpAddress CanonicalizeLinkLocal(const IpAddress& address) noexcept { - auto bytes = address.Bytes(); - bytes[2] = std::byte{0}; - bytes[3] = std::byte{0}; - return IpAddress::FromV6Bytes(bytes); + return detail::Ipv6SourceFlagsUsable(request.ifr_ifru.ifru_flags6); } void ResolveViaGetifaddrs(std::string_view interface, InterfaceAddresses& result) noexcept { @@ -344,14 +314,14 @@ void ResolveViaGetifaddrs(std::string_view interface, InterfaceAddresses& result } if (auto address = IpAddress::FromSockaddr(ifa->ifa_addr); address) { if (address->IsLinkLocal()) { - address = CanonicalizeLinkLocal(*address); + address = detail::CanonicalizeLinkLocalV6(*address); } candidates.push_back(*address); } break; } case AF_LINK: - result.mac = ReadMac(*ifa); + result.mac = detail::MacFromLinkSockaddr(*ifa->ifa_addr); break; default: break; @@ -378,6 +348,39 @@ void SelectSourceAddresses(std::span candidates, InterfaceAddre } } +#if !defined(__linux__) + +// Reads straight out of the kernel-provided sockaddr rather than copying the whole (variable-length) +// sockaddr into a fixed local: the MAC sits at a name-length-dependent offset in sdl_data that a long +// interface name could push past any local sockaddr_dl. Byte access through std::byte* is alignment- +// and aliasing-safe. +MacAddress MacFromLinkSockaddr(const sockaddr& link_addr) noexcept { + const auto* bytes = reinterpret_cast(&link_addr); + const auto sa_len = static_cast(link_addr.sa_len); + const auto name_len = std::to_integer(bytes[offsetof(sockaddr_dl, sdl_nlen)]); + const auto addr_len = std::to_integer(bytes[offsetof(sockaddr_dl, sdl_alen)]); + const auto mac_offset = offsetof(sockaddr_dl, sdl_data) + name_len; + if (addr_len != MAC_SIZE || mac_offset + MAC_SIZE > sa_len) { + return {}; + } + return MacAddress::FromBytes(std::span{bytes + mac_offset, MAC_SIZE}); +} + +bool Ipv6SourceFlagsUsable(int flags6) noexcept { + constexpr int unusable = IN6_IFF_TENTATIVE | IN6_IFF_DUPLICATED | IN6_IFF_DETACHED + | IN6_IFF_DEPRECATED | IN6_IFF_ANYCAST; + return (flags6 & unusable) == 0; +} + +IpAddress CanonicalizeLinkLocalV6(const IpAddress& address) noexcept { + auto bytes = address.Bytes(); + bytes[2] = std::byte{0}; + bytes[3] = std::byte{0}; + return IpAddress::FromV6Bytes(bytes); +} + +#endif + } // namespace detail #if defined(__linux__) diff --git a/src/reflector/interface_address.h b/src/reflector/interface_address.h index 4a24739..aac3170 100644 --- a/src/reflector/interface_address.h +++ b/src/reflector/interface_address.h @@ -7,6 +7,7 @@ #include #include #include +#include namespace reflector { @@ -30,6 +31,25 @@ namespace detail { // without a real interface that happens to carry the right address mix. void SelectSourceAddresses(std::span candidates, InterfaceAddresses& result) noexcept; +#if !defined(__linux__) +// The BSD getifaddrs resolver's pure helpers, split out for unit testing (the resolver itself needs +// a real interface). `link_addr` is an AF_LINK sockaddr (a sockaddr_dl). + +// The interface's link-layer MAC read from an AF_LINK sockaddr_dl, or all-zero if it carries none +// (addr_len != 6) or the address would run past the sockaddr's sa_len. Reads by byte offset — the +// MAC sits at a name-length-dependent offset a long interface name could push past a fixed local. +[[nodiscard]] MacAddress MacFromLinkSockaddr(const sockaddr& link_addr) noexcept; + +// Whether the IN6_IFF_* flags (from SIOCGIFAFLAG_IN6) mark the address a usable source — not +// tentative, duplicated, detached, deprecated, or anycast. +[[nodiscard]] bool Ipv6SourceFlagsUsable(int flags6) noexcept; + +// A KAME link-local sin6_addr with the embedded scope id (interface index in bytes 2-3) cleared to +// the canonical fe80:: on-wire form. macOS/FreeBSD only — Linux netlink already yields canonical +// addresses, so the same clearing there could corrupt a non-conforming one. +[[nodiscard]] IpAddress CanonicalizeLinkLocalV6(const IpAddress& address) noexcept; +#endif + } // namespace detail // Resolves an interface's MAC and per-family source addresses; fields are left empty for diff --git a/tests/interface_address_test.cpp b/tests/interface_address_test.cpp index dc8dc3c..c65bbcb 100644 --- a/tests/interface_address_test.cpp +++ b/tests/interface_address_test.cpp @@ -13,6 +13,15 @@ #if defined(__linux__) #include #include +#else +#include +#include +#include +#include +#include +#include +#include +#include #endif namespace { @@ -233,4 +242,86 @@ TEST(SelectSourceAddressesTest, EmptyCandidatesSelectNothing) { EXPECT_FALSE(selected.v6.has_value()); } +#if !defined(__linux__) + +// --- BSD getifaddrs pure helpers (the resolver itself needs a real interface) --- + +namespace { + +// An AF_LINK sockaddr_dl carrying `name` then `mac` in sdl_data, with sdl_len (the sa_len the reader +// bounds against) set to `sa_len`. Short inputs fit sdl_data, so a plain sockaddr_dl backs it. +sockaddr_dl MakeLinkSockaddr(std::string_view name, std::span mac, uint8_t sa_len) { + sockaddr_dl sdl{}; + sdl.sdl_family = AF_LINK; + sdl.sdl_nlen = static_cast(name.size()); + sdl.sdl_alen = static_cast(mac.size()); + sdl.sdl_len = sa_len; + std::memcpy(sdl.sdl_data, name.data(), name.size()); + std::memcpy(sdl.sdl_data + name.size(), mac.data(), mac.size()); + return sdl; +} + +constexpr uint8_t LinkSaLen(size_t name_len, size_t mac_len) { + return static_cast(offsetof(sockaddr_dl, sdl_data) + name_len + mac_len); +} + +} // namespace + +TEST(MacFromLinkSockaddrTest, ReadsMacAtTheNameDependentOffset) { + // The MAC sits after the 3-char name in sdl_data; a correct read skips the name. + const std::array mac{std::byte{0x02}, std::byte{0x11}, std::byte{0x22}, + std::byte{0x33}, std::byte{0x44}, std::byte{0x55}}; + const auto sdl = MakeLinkSockaddr("en0", mac, LinkSaLen(3, 6)); + + const auto parsed = detail::MacFromLinkSockaddr(reinterpret_cast(sdl)); + EXPECT_EQ(parsed, *MacAddress::FromString("02:11:22:33:44:55")); +} + +TEST(MacFromLinkSockaddrTest, YieldsZeroWhenTheLinkCarriesNoAddress) { + // A loopback-style AF_LINK entry has sdl_alen 0 (no hardware address) -> all-zero MAC. + const auto sdl = MakeLinkSockaddr("lo0", {}, LinkSaLen(3, 0)); + + EXPECT_EQ(detail::MacFromLinkSockaddr(reinterpret_cast(sdl)), MacAddress{}); +} + +TEST(MacFromLinkSockaddrTest, YieldsZeroWhenTheMacWouldRunPastSaLen) { + // sdl_alen claims 6 but sa_len stops one byte short of the MAC's end: refuse rather than over-read. + const std::array mac{std::byte{0x02}, std::byte{0x11}, std::byte{0x22}, + std::byte{0x33}, std::byte{0x44}, std::byte{0x55}}; + const auto sdl = MakeLinkSockaddr("en0", mac, LinkSaLen(3, 6) - 1); + + EXPECT_EQ(detail::MacFromLinkSockaddr(reinterpret_cast(sdl)), MacAddress{}); +} + +TEST(Ipv6SourceFlagsUsableTest, AcceptsAUsableAddress) { + EXPECT_TRUE(detail::Ipv6SourceFlagsUsable(0)); + EXPECT_TRUE(detail::Ipv6SourceFlagsUsable(IN6_IFF_AUTOCONF)); // a benign flag, not in the reject set +} + +TEST(Ipv6SourceFlagsUsableTest, RejectsEachUnusableFlag) { + for (const int flag : {IN6_IFF_TENTATIVE, IN6_IFF_DUPLICATED, IN6_IFF_DETACHED, + IN6_IFF_DEPRECATED, IN6_IFF_ANYCAST}) { + EXPECT_FALSE(detail::Ipv6SourceFlagsUsable(flag)) << "flag " << flag; + EXPECT_FALSE(detail::Ipv6SourceFlagsUsable(flag | IN6_IFF_AUTOCONF)) << "flag " << flag; + } +} + +TEST(CanonicalizeLinkLocalV6Test, ClearsTheEmbeddedKameScopeId) { + // KAME stashes the interface index in bytes 2-3 of a link-local sin6_addr; canonicalizing zeroes + // just those, leaving fe80:: and the interface id intact. + auto bytes = IpAddress::FromString("fe80::1")->Bytes(); + bytes[2] = std::byte{0x00}; + bytes[3] = std::byte{0x05}; // scope id 5 embedded KAME-style + const auto canonical = detail::CanonicalizeLinkLocalV6(IpAddress::FromV6Bytes(bytes)); + + EXPECT_EQ(canonical, *IpAddress::FromString("fe80::1")); +} + +TEST(CanonicalizeLinkLocalV6Test, LeavesACanonicalAddressUnchanged) { + const auto canonical = *IpAddress::FromString("fe80::1"); + EXPECT_EQ(detail::CanonicalizeLinkLocalV6(canonical), canonical); +} + +#endif + } // namespace reflector From 7db98de83963310f96ebb69ef624f92d3b3a0e9c Mon Sep 17 00:00:00 2001 From: Sergii Bogomolov Date: Thu, 9 Jul 2026 04:44:02 +0200 Subject: [PATCH 3/3] test: cover error and edge paths surfaced by the sweep The sweep found live code with no test at any layer. Add narrowly scoped cases, each pinned to a specific production line: - dispatchers: the DrainReadableFd per-poll cap stops one bursty socket from starving the loop, then resumes on the next drain. - SSDP: the session cap is global across groups, a repeat interface change with no capability change is a no-op, a transient bring-up failure on a later group retries, and DIAL still forwards the response Location when the listener mint fails. - DIAL proxy: rediscovery and a spurious accept edge both refresh last_active past the original grace deadline; the closed guard stops minting for a later header once a prior one aborted; an interface change to a different live address drops and re-mints listeners. - raw socket: send fails cleanly when the frame can't be built; RejectsInvalidInterface now runs on both platforms; family-fd reuse on join (RequiresRoot). - config: env tags differing only by case both canonicalize to one name and are rejected. http: a Content-Length that overflows size_t is rejected. - failed-registration rollback: three paths that acquire an fd, fail to register it with a dispatcher, and must release it -- Application's signal-wakeup pipe, DefaultAddressMonitor::Start (Watch fails -> the socket is closed so IsValid() reports false), and EventLoopDispatcher ::Register (kernel-interest programming fails -> the map entry is erased so no fd is stranded; epoll-lane only). None was covered; a leaked fd is invisible to LeakSanitizer. Captured-output assertions match the log level or a short stable token, not full message text. TestCaptureSocket sizes its socketpair buffers so a full drain burst can be staged before the first read. --- tests/application_test.cpp | 15 +++ tests/config_test.cpp | 15 +++ tests/default_address_monitor_test.cpp | 13 +++ tests/default_packet_dispatcher_test.cpp | 27 +++++ tests/dial_proxy_test.cpp | 127 +++++++++++++++++++++++ tests/event_loop_dispatcher_test.cpp | 21 ++++ tests/http_message_test.cpp | 12 +++ tests/raw_socket_test.cpp | 66 ++++++++++-- tests/ssdp_reflector_test.cpp | 109 +++++++++++++++++++ tests/test_helpers.h | 6 ++ 10 files changed, 403 insertions(+), 8 deletions(-) diff --git a/tests/application_test.cpp b/tests/application_test.cpp index 56004f7..d2e938c 100644 --- a/tests/application_test.cpp +++ b/tests/application_test.cpp @@ -39,6 +39,7 @@ class ApplicationTest : public ::testing::Test { // The signal-wakeup self-pipe read fd (friendship isn't inherited, so the TEST_F body reaches the // private member through this fixture accessor, like the counts above). static int WakeupReadFd(const Application& app) { return app.wakeup_read_.Get(); } + static int WakeupWriteFd(const Application& app) { return app.wakeup_write_.Get(); } // Per-interface settings the factories stamp onto the Interface and socket when first // created. Interfaces with no entry get the defaults (valid, can send both families, @@ -570,4 +571,18 @@ TEST_F(ApplicationTest, SignalWakeupRegistersAndDrains) { EXPECT_TRUE(IsWouldBlockErrno(errno)); } +// If the dispatcher can't watch the read end, PrepareSignalWakeup rolls back: both pipe fds are +// closed (they would otherwise leak -- LeakSanitizer doesn't track fds) and it returns -1 so main() +// falls back to poll-interval-bounded shutdown rather than a live but unwatched pipe. +TEST_F(ApplicationTest, SignalWakeupRollsBackWhenRegistrationFails) { + auto app = MakeApp(); + const size_t before = dispatcher_->RegistrationCount(); + dispatcher_->fail_registers_remaining = 1; // the wakeup pipe is the next (and only) Register here + + EXPECT_EQ(app.PrepareSignalWakeup(), -1); + EXPECT_EQ(dispatcher_->RegistrationCount(), before); // nothing left watched + EXPECT_EQ(WakeupReadFd(app), -1); // read end closed on rollback + EXPECT_EQ(WakeupWriteFd(app), -1); // write end closed on rollback +} + } // namespace reflector diff --git a/tests/config_test.cpp b/tests/config_test.cpp index 9bd1232..d44d0c2 100644 --- a/tests/config_test.cpp +++ b/tests/config_test.cpp @@ -1758,6 +1758,21 @@ TEST(ConfigTest, EnvRejectsDuplicateNameAcrossTags) { EXPECT_FALSE(config.has_value()); } +TEST(ConfigTest, EnvRejectsTagsDifferingOnlyByCase) { + // The tag map keys on the raw tag (case-sensitive), so TV and tv are two groups — but both + // canonicalize to the reflector name "tv", so the second is rejected as a duplicate rather than + // silently becoming a separate reflector. + const auto config = Config::Load(std::nullopt, Env({ + {"REFLECTOR_TV_SOURCE_IF", "eth0"}, + {"REFLECTOR_TV_TARGET_IF", "eth1"}, + {"REFLECTOR_TV_WOL", "true"}, + {"REFLECTOR_tv_SOURCE_IF", "eth2"}, + {"REFLECTOR_tv_TARGET_IF", "eth3"}, + {"REFLECTOR_tv_WOL", "true"}, + })); + EXPECT_FALSE(config.has_value()); +} + TEST(ConfigTest, EnvRejectsEmptyConfiguration) { EXPECT_FALSE(Config::Load(std::nullopt, std::span{}).has_value()); EXPECT_FALSE(Config::Load(std::nullopt, Env({{"PATH", "/usr/bin"}})).has_value()); diff --git a/tests/default_address_monitor_test.cpp b/tests/default_address_monitor_test.cpp index b656449..2931fea 100644 --- a/tests/default_address_monitor_test.cpp +++ b/tests/default_address_monitor_test.cpp @@ -348,6 +348,19 @@ TEST_F(DefaultAddressMonitorTest, StartRejectsUnboundCallback) { EXPECT_NE(output.find("ERROR"), std::string::npos) << output; } +// If the dispatcher can't watch the already-open notification socket, Start rolls back via Close(): +// the fd is released so IsValid() reports false, rather than leaving a live-but-unwatched socket that +// still claims to be a working monitor. +TEST_F(DefaultAddressMonitorTest, StartRollsBackWhenRegistrationFails) { + auto monitor = MakeMonitor(); + dispatcher.fail_registers_remaining = 1; // Watch()'s Register is the only one here, and it fails + + EXPECT_FALSE(StartWatching(monitor)); + EXPECT_FALSE(monitor.IsValid()); // Close() released the fd on rollback + EXPECT_FALSE(dispatcher.IsWatching(monitor_fd)); + EXPECT_EQ(dispatcher.RegistrationCount(), 0); +} + #if defined(__linux__) // A sockaddr_storage holding a sockaddr_nl with the given nl_pid, as recvfrom would report a // datagram's source. static for internal linkage (GCC's -Wmissing-declarations). diff --git a/tests/default_packet_dispatcher_test.cpp b/tests/default_packet_dispatcher_test.cpp index e9b0309..0d0ee4a 100644 --- a/tests/default_packet_dispatcher_test.cpp +++ b/tests/default_packet_dispatcher_test.cpp @@ -64,6 +64,10 @@ class DefaultPacketDispatcherTest : public ::testing::Test { size_t CaptureSourceCount() const { return packet_dispatcher.capture_sources_.size(); } + + static constexpr size_t MaxPacketsPerReadEvent() noexcept { + return DefaultPacketDispatcher::MAX_PACKETS_PER_READ_EVENT; + } }; // Registers a second callback the first time it runs, then disables itself so subsequent @@ -538,6 +542,29 @@ TEST_F(DefaultPacketDispatcherTest, DispatchesNothingForUnparseableFrame) { EXPECT_EQ(counter.count, 0); } +// DrainReadableFd caps a single drain at MAX_PACKETS_PER_READ_EVENT frames even when more are +// already queued, so one bursty capture socket can't starve every other fd on the single-threaded +// event loop. Stage one more frame than the cap before a single PollOnce: only the capped count is +// delivered; the fd stays readable and the remainder is delivered on the next drain. +TEST_F(DefaultPacketDispatcherTest, DrainStopsAtMaxPacketsPerReadEventThenResumesOnNextPoll) { + TestCaptureSocket capture; + PacketCounter counter; + const auto registration = packet_dispatcher.Register( + capture.socket, PacketFilter{}, CreateDelegate<&PacketCounter::OnPacket>(&counter)); + ASSERT_TRUE(registration.IsValid()); + + const int cap = static_cast(MaxPacketsPerReadEvent()); + for (int i = 0; i < cap + 1; ++i) { + ASSERT_TRUE(capture.WriteFrame(MakeUdpFrame())); + } + + EXPECT_TRUE(dispatcher.PollOnce(std::chrono::milliseconds{1000})); + EXPECT_EQ(counter.count, cap); // the drain loop stops exactly at the cap, mid-burst + + EXPECT_TRUE(dispatcher.PollOnce(std::chrono::milliseconds{1000})); + EXPECT_EQ(counter.count, cap + 1); // fd still readable: the next drain delivers the rest +} + class DefaultPacketDispatcherRequiresRootTest : public ::testing::Test { protected: EventLoopDispatcher dispatcher; diff --git a/tests/dial_proxy_test.cpp b/tests/dial_proxy_test.cpp index 665546a..c4ce2ce 100644 --- a/tests/dial_proxy_test.cpp +++ b/tests/dial_proxy_test.cpp @@ -485,6 +485,42 @@ TEST_F(DialProxyTest, OnInterfaceChangedKeepsListenersWhenSourceAddressUnchanged EXPECT_TRUE(dispatcher.IsWatching(fd)); } +// A source address that CHANGES to a different live address (rather than disappearing outright) must also +// stale the old listeners -- is_stale's `current != old_addr` branch, not just its `current == nullopt` +// branch. Mirrors OnInterfaceChangedDropsListenersBoundToAChangedSourceAddress with a live replacement +// address instead of none, and additionally proves the next advertisement re-mints against the new address +// rather than reusing the stale listener. +TEST_F(DialProxyTest, OnInterfaceChangedDropsListenersWhenSourceAddressChangesToADifferentLiveAddress) { + // Not every platform routes all of 127/8 to lo (macOS assigns only 127.0.0.1); skip where 127.0.0.2 + // isn't bindable, as BindAddressFlowsFromTheSourceInterface does -- the docker (Linux) gate still + // exercises this. + const auto alt = IpAddress::FromString("127.0.0.2").value(); + FakeInterface alt_if; + alt_if.SetV4(alt); + if (!TcpSocket::Listen(alt_if, IpAddress::Family::V4)) { + GTEST_SKIP() << "127.0.0.2 is not bindable on this platform"; + } + + auto proxy = MakeProxy(); + ASSERT_TRUE(proxy.EnsureDiscoveryListener(Device(2)).has_value()); + ASSERT_EQ(EndpointCount(proxy), 1u); + const int old_fd = ListenerFd(proxy, Device(2)); + ASSERT_TRUE(dispatcher.IsWatching(old_fd)); + + source_if.SetV4(alt); // the source address CHANGED, not nulled + proxy.OnInterfaceChanged(); + + EXPECT_EQ(EndpointCount(proxy), 0u); // the old-address listener went too + EXPECT_FALSE(HasEndpoint(proxy, Device(2))); + EXPECT_FALSE(dispatcher.IsWatching(old_fd)); // its accept registration went with it + EXPECT_EQ(dispatcher.TimerCount(), 0u); // nothing left to sweep -> the reaper stopped + + // The next advertisement re-mints against the NEW address, not a reuse of the stale listener. + const auto authority = proxy.EnsureDiscoveryListener(Device(2)); + ASSERT_TRUE(authority.has_value()); + EXPECT_EQ(authority->addr, alt); // bound to the NEW address -- a reused stale listener would still carry 127.0.0.1 +} + TEST_F(DialProxyTest, DiscoveryAndRestCapsAreIndependent) { auto proxy = MakeProxy(); // Fill the Discovery cap entirely... @@ -1365,6 +1401,46 @@ TEST_F(DialProxyRewriteTest, SecondUrlHeaderOverflowingTheCapDropsTheWholeRespon EXPECT_FALSE(HasEndpoint(proxy, second_dev)); // the overflowing second was never minted } +// 5c. RewriteRestAuthority's closed-guard (dial_proxy.cpp:260-264): once a PRIOR header in this same message +// already Aborted the connection, a LATER header must not re-enter EnsureRestListener at all. Pre-mint the +// first device's REST listener so the response's first Application-URL hits the cheap reuse branch (no +// Register call) -- that leaves the seam's single Register failure to land on the SECOND header (a fresh +// device), which Aborts. The THIRD header names a brand-new device that WOULD mint successfully if +// RewriteRestAuthority re-entered EnsureRestListener for it (the seam is spent by then, and nothing else +// blocks it) -- so a removed guard leaves a real endpoint for it; the guard in place must not. +TEST_F(DialProxyRewriteTest, ClosedGuardStopsMintingForAHeaderAfterAPriorHeaderAborted) { + auto proxy = MakeProxy(); + OpenConnection oc = OpenOne(proxy); + ASSERT_NE(oc.id, 0u); + auto* conn = FindConnection(proxy, oc.id); + ASSERT_NE(conn, nullptr); + + const auto first_dev = DeviceRest(170, 8009); + ASSERT_TRUE(EnsureRest(proxy, first_dev).has_value()); // pre-minted: the response's first header reuses it + ASSERT_TRUE(HasEndpoint(proxy, first_dev)); + + const auto second_dev = DeviceRest(171, 8010); // fresh device: its mint is the first Register call in Feed + const auto third_dev = DeviceRest(172, 8011); // fresh device: would mint fine if the guard didn't stop Feed + ASSERT_FALSE(HasEndpoint(proxy, second_dev)); + ASSERT_FALSE(HasEndpoint(proxy, third_dev)); + + const std::string response = std::format( + "HTTP/1.1 200 OK\r\nApplication-URL: http://{}/apps\r\nLocation: http://{}/run\r\n" + "Location: http://{}/run2\r\nContent-Length: 0\r\n\r\n", + first_dev, second_dev, third_dev); + const std::span bytes{ + reinterpret_cast(response.data()), response.size()}; + ASSERT_EQ(oc.DeviceSide().Send(bytes), SendStatus::Ok); + + dispatcher.fail_registers_remaining = 1; // the SECOND header's Register call is the first one Feed makes + ASSERT_TRUE(FireReadableWhenReady(oc.conn_upstream_fd)); + + EXPECT_TRUE(ConnClosed(*conn)); // the second header's mint failed -> Abort + EXPECT_TRUE(HasEndpoint(proxy, first_dev)); // the pre-minted, reused listener is untouched + EXPECT_FALSE(HasEndpoint(proxy, second_dev)); // its failed Register rolled the endpoint back + EXPECT_FALSE(HasEndpoint(proxy, third_dev)); // the closed-guard stopped Feed before a third mint attempt +} + // 6. Back-pressure DRAIN path (Sync -> Flush): stall the raw client so the proxy's client socket buffers a // partial (non-overflowing) tail; Sync must arm write interest on it. Then drain the client and fire the // writable edge: Flush drains the tail. A deleted/incorrect Sync(to) would leave the tail unarmed and @@ -1647,6 +1723,57 @@ TEST_F(DialProxyForwardTest, EvictionReapsIdleListenersByRoleGraceButNotReferenc EXPECT_TRUE(HasEndpoint(proxy, referenced_dev)); // still referenced by the live Connection -> survives } +// ---- last_active refresh: the two production writes that keep an active endpoint out of the grace reap. +// Both are stamped from the LIVE clock (not the Evict `now` seam), so unlike the deadline-based tests above, +// these can't set last_active directly -- they bracket each real write with steady_clock::now() and separate +// mint from refresh with a real (short) sleep, wide enough that an Evict `now` lands strictly between "past +// the original mint's deadline" and "before the refreshed deadline". Deleting either refresh collapses that +// window, so the endpoint is reaped and the test fails. ---- + +// EnsureListener's reuse-branch refresh (dial_proxy.cpp:67): re-advertising an already-minted device must +// push its last_active forward, or a repeatedly-rediscovered device would still get swept on its ORIGINAL +// mint time. +TEST_F(DialProxyTest, RediscoveryRefreshesLastActivePastTheOriginalGraceDeadline) { + auto proxy = MakeProxy(); + const auto device = Device(120); + + ASSERT_TRUE(proxy.EnsureDiscoveryListener(device).has_value()); // first mint: try_emplace stamps last_active + const auto after_first = std::chrono::steady_clock::now(); + + std::this_thread::sleep_for(std::chrono::milliseconds{50}); // a real gap the refresh must land past + + ASSERT_TRUE(proxy.EnsureDiscoveryListener(device).has_value()); // re-advertise: the reuse branch's refresh + + // Past the original mint's deadline, but well before (original + the 50ms gap)'s deadline: only the + // refresh decides whether this endpoint is still alive here. + Evict(after_first + DialProxy::DISCOVERY_ENDPOINT_GRACE + std::chrono::milliseconds{10}); + + EXPECT_TRUE(HasEndpoint(proxy, device)); // survives only because re-advertising refreshed last_active +} + +// OnAccept's own refresh (dial_proxy.cpp:289), run BEFORE Accept() itself: even a spurious/EAGAIN accept edge +// counts as activity, so a device mid-connect-attempt isn't swept out from under it. Fire the listener's +// readable edge with NO client pending -- Accept() then reports EAGAIN, so no Connection is created and +// active_connections stays 0, isolating this refresh from the (separate) endpoint-reference eviction guard. +TEST_F(DialProxyTest, SpuriousAcceptEdgeRefreshesLastActivePastTheOriginalGraceDeadline) { + auto proxy = MakeProxy(); + const auto device = Device(121); + + ASSERT_TRUE(proxy.EnsureDiscoveryListener(device).has_value()); + const auto after_first = std::chrono::steady_clock::now(); + const int listener_fd = ListenerFd(proxy, device); + ASSERT_GE(listener_fd, 0); + + std::this_thread::sleep_for(std::chrono::milliseconds{50}); + + dispatcher.FireReadable(listener_fd); // no pending client: OnAccept stamps last_active, then Accept() EAGAINs + ASSERT_EQ(EndpointActiveConnections(proxy, device), 0u); // confirms nothing was actually accepted + + Evict(after_first + DialProxy::DISCOVERY_ENDPOINT_GRACE + std::chrono::milliseconds{10}); + + EXPECT_TRUE(HasEndpoint(proxy, device)); // survives only because the accept edge refreshed last_active +} + // 5. Lazy-start / self-stop: no timer before the first mint; one after; and once everything is reaped the // sweep self-stops (TimerCount back to 0). TEST_F(DialProxyForwardTest, EvictionTimerLazyStartsAndSelfStops) { diff --git a/tests/event_loop_dispatcher_test.cpp b/tests/event_loop_dispatcher_test.cpp index 0ef5e57..6eb8ad1 100644 --- a/tests/event_loop_dispatcher_test.cpp +++ b/tests/event_loop_dispatcher_test.cpp @@ -281,6 +281,27 @@ TEST_F(EventLoopDispatcherTest, RegisterRejectsAlreadyWatchedFd) { EXPECT_EQ(RegistrationCount(), 1); } +#if defined(__linux__) +// epoll_ctl(EPOLL_CTL_ADD) rejects an fd that can't be polled (a directory) with EPERM, so Register +// emplaces the callback then fails to program the kernel interest -- driving the rollback that removes +// the interest and erases the map entry. A stranded entry would permanently reject re-registering the +// fd and trip the destructor leak guard. kqueue accepts vnodes via EVFILT_READ, so this reproduces +// only on the epoll lane. +TEST_F(EventLoopDispatcherTest, RegisterRollsBackWhenKernelInterestFails) { + const int dir_fd = ::open("/", O_RDONLY | O_DIRECTORY); + ASSERT_GE(dir_fd, 0); + ReadableCounter counter; + + const auto registration = dispatcher.Register( + dir_fd, CreateDelegate<&ReadableCounter::OnReadable>(&counter)); + + EXPECT_FALSE(registration.IsValid()); + EXPECT_EQ(RegistrationCount(), 0); // the partial map entry was rolled back, not left stale + + ::close(dir_fd); +} +#endif + TEST_F(EventLoopDispatcherTest, UnregisterStopsCallback) { ReadablePipe pipe; ASSERT_GE(pipe.ReadEnd(), 0); diff --git a/tests/http_message_test.cpp b/tests/http_message_test.cpp index 6eeaa5c..a78999e 100644 --- a/tests/http_message_test.cpp +++ b/tests/http_message_test.cpp @@ -451,6 +451,18 @@ TEST(HttpFramingMiscTest, AllowsIdenticalDuplicateContentLength) { EXPECT_TRUE(d.out.ends_with("hi")); } +TEST(HttpFramingMiscTest, RefusesContentLengthThatOverflowsSizeT) { + // A value too large for size_t makes from_chars report result_out_of_range — reject rather than + // frame a truncated/garbage body length (the size_t counterpart of the chunk-size overflow guard). + UrlRewrite rewrite; + HttpFraming framing(AsRewrite(rewrite), HttpFraming::MessageType::Response); + Driver d{framing}; + d.Read( + "HTTP/1.1 200 OK\r\n" + "Content-Length: 999999999999999999999999999999\r\n\r\n"); + EXPECT_FALSE(d.ok); +} + TEST(HttpFramingMiscTest, RefusesHeadRequest) { // A HEAD response is bodyless but may still carry Content-Length, which this framer would await // as phantom body bytes and desync the keep-alive stream. Refuse HEAD at the request side. diff --git a/tests/raw_socket_test.cpp b/tests/raw_socket_test.cpp index c95d684..d22538b 100644 --- a/tests/raw_socket_test.cpp +++ b/tests/raw_socket_test.cpp @@ -528,6 +528,20 @@ TEST_F(RawSocketTest, RefusesIpv4SendWithoutIpv4Source) { }); } +// A payload that pushes the assembled frame past SendFrame's fixed-size stack buffer: +// BuildUdpFrame's CheckedFrameSize rejects it (out buffer too small) and returns 0, so SendFrame +// must log and return false instead of writing a truncated frame. Ethernet(14) + IPv4(20) + +// UDP(8) header overhead alone means a 4096-byte payload already overflows the 4096-byte buffer. +TEST_F(RawSocketTest, SendFailsWhenFrameExceedsSendBuffer) { + SetSource(IpAddress::FromV4Bytes(192, 0, 2, 1), std::nullopt); + const std::vector oversized_payload(4096, std::byte{0}); + + const std::string output = CaptureStdout([&] { + EXPECT_FALSE(socket.SendUdpBroadcastDatagram(9, 9, oversized_payload, /*ttl=*/64)); + }); + EXPECT_NE(output.find("egress"), std::string::npos) << output; +} + // A non-multicast group is rejected by the kernel (EINVAL on both platforms) and surfaced as // false — the deterministic, root-free failure case. (A successful join needs a real // multicast-capable interface; that's the RequiresRoot veth test below. Joining a *valid* group @@ -544,6 +558,17 @@ TEST_F(RawSocketTest, JoinMulticastGroupRejectsNonMulticastAddress) { EXPECT_FALSE(JoinFdValid(IpAddress::Family::V6)); } +// The bind/interface-index validation in the constructor (raw_socket.cpp's IsValid() gate) is +// platform-independent, so this runs on every platform rather than only alongside the BSD-only +// tests below. +TEST_F(RawSocketTest, RejectsInvalidInterface) { + CaptureStdout([&] { // swallow the rejection logs; IsValid() is the contract + const Interface invalid{"nonex0"}; + const RawSocket socket_on_invalid{invalid}; + EXPECT_FALSE(socket_on_invalid.IsValid()); + }); +} + #if !defined(__linux__) // DLT_NULL framing only appears on the BSDs — on Linux AF_PACKET delivers Ethernet frames for every // interface, lo included, so the parser never encounters loopback framing there. @@ -607,14 +632,6 @@ TEST_F(RawSocketTest, RejectsLoopbackWithUnsupportedFamily) { EXPECT_FALSE(ParseQuietly(f.bytes).has_value()); } -TEST_F(RawSocketTest, RejectsInvalidInterface) { - CaptureStdout([&] { // swallow the rejection logs; IsValid() is the contract - const Interface invalid{"nonex0"}; - const RawSocket socket_on_invalid{invalid}; - EXPECT_FALSE(socket_on_invalid.IsValid()); - }); -} - // Packs N bpf_hdr-prefixed frames into a single batch (one read returns all of them), // then verifies Receive() walks them one at a time. Mirrors what // RawSocketRequiresRootTest::DrainsBatchedFramesFromOneRead exercises against @@ -975,6 +992,11 @@ class RawSocketInterfacePairRequiresRootTest : public ::testing::Test { static bool JoinFdValid(const RawSocket& socket, IpAddress::Family family) { return socket.join_fds_.Get(family).IsValid(); } + // The raw fd number, so a test can confirm a second join reused the family's existing join fd + // rather than closing and reopening it (JoinFdValid alone can't distinguish those). + static int JoinFdNumber(const RawSocket& socket, IpAddress::Family family) { + return socket.join_fds_.Get(family).Get(); + } static void CloseSocket(RawSocket& socket) { socket.Close(); } }; @@ -1069,6 +1091,34 @@ TEST_F(RawSocketInterfacePairRequiresRootTest, JoinsMulticastGroupsIdempotently) EXPECT_TRUE(socket.JoinMulticastGroup(IpAddress::MdnsGroupV6()).IsValid()); } +// Second join of a *different* group in the same family, once that family's join fd is already +// open from an earlier join (JoinMulticastGroup's opened_now == false path). The existing fd must +// be reused, not reset and reopened: resetting it would close the fd holding the first group's +// kernel membership out from under it, even though the membership map still reports it held. This +// is distinct from JoinsMulticastGroupsIdempotently above, which only re-joins the *same* group +// and so never leaves the top (already-a-membership) fast path. +TEST_F(RawSocketInterfacePairRequiresRootTest, JoinReusesAlreadyOpenFamilyFd) { + Interface iface{pair.InjectInterface()}; + RawSocket socket{iface}; + ASSERT_TRUE(socket.IsValid()); + + const auto first_group = IpAddress::MdnsGroupV4(); + const auto second_group = IpAddress::SsdpGroupV4(); + auto first = socket.JoinMulticastGroup(first_group); + ASSERT_TRUE(first.IsValid()); + const int fd_after_first = JoinFdNumber(socket, IpAddress::Family::V4); + ASSERT_GE(fd_after_first, 0); + + auto second = socket.JoinMulticastGroup(second_group); // different group, same family + ASSERT_TRUE(second.IsValid()); + + EXPECT_EQ(JoinFdNumber(socket, IpAddress::Family::V4), fd_after_first) + << "second join reset the family's already-open join fd instead of reusing it"; + EXPECT_TRUE(JoinFdValid(socket, IpAddress::Family::V4)); + EXPECT_EQ(MembershipCount(socket, first_group), 1u); + EXPECT_EQ(MembershipCount(socket, second_group), 1u); +} + // The refcount + fd lifecycle: two memberships of a group keep one kernel join; the group is left // only when the last membership drops, and the family's join fd is closed once it has no group. TEST_F(RawSocketInterfacePairRequiresRootTest, RefcountsMembershipsAndFreesTheFamilyFd) { diff --git a/tests/ssdp_reflector_test.cpp b/tests/ssdp_reflector_test.cpp index 35846f9..c4356f0 100644 --- a/tests/ssdp_reflector_test.cpp +++ b/tests/ssdp_reflector_test.cpp @@ -405,6 +405,53 @@ TEST_F(SsdpReflectorTest, RequiredFamilyTornDownAndRecovered) { EXPECT_EQ(RegistrationCount(), 2); // brought back up } +// SSDP counterpart of MdnsReflectorTest.RepeatInterfaceChangeWithoutCapabilityChangeIsANoOp: an +// interface change that doesn't flip any family's reflectability is a no-op. SyncFamily's +// already-in-desired-state guard is per-family, so this also covers IPv6's second group (site-local) — +// a per-group guard, or none at all, would churn it on every call even though nothing changed. +TEST_F(SsdpReflectorTest, RepeatInterfaceChangeWithoutCapabilityChangeIsANoOp) { + SsdpReflector reflector{packet_dispatcher, source, target, MakeConfig(AddressFamily::Default)}; + ASSERT_TRUE(reflector.IsValid()); + const auto joins_before = source.joined_groups.size(); + const auto count_before = RegistrationCount(); + + reflector.OnInterfaceChanged(); + reflector.OnInterfaceChanged(); + + EXPECT_EQ(RegistrationCount(), count_before); // no duplicate registrations + EXPECT_EQ(source.joined_groups.size(), joins_before); // no duplicate joins + EXPECT_TRUE(source.left_groups.empty()); // and nothing torn down +} + +// SSDP counterpart of MdnsReflectorTest.TransientBringUpFailureLeavesFamilyDownThenRetries, timed to +// fail v6's SECOND group (site-local) rather than its first: proves BringUpFamily's per-group rollback +// (not just DynamicFamilyReflector's family-level teardown) releases the FIRST group's (link-local) +// already-taken membership/registrations when a LATER group fails during a dynamic (OnInterfaceChanged) +// bring-up — the construction-time equivalent is FailureOnALaterGroupRollsBackTheWholeFamily below. +TEST_F(SsdpReflectorTest, TransientBringUpFailureOnALaterGroupLeavesFamilyDownThenRetries) { + target.iface.SetHasSource(IpAddress::Family::V6, false); // v6 down at construction + SsdpReflector reflector{packet_dispatcher, source, target, MakeConfig(AddressFamily::Default)}; + ASSERT_TRUE(reflector.IsValid()); + EXPECT_EQ(RegistrationCount(), 2); // v4 only + + // v6 becomes reflectable; fail its SECOND group's (site-local) first registration, after the first + // group (link-local) has already taken its memberships/registrations. + target.iface.SetHasSource(IpAddress::Family::V6, true); + packet_dispatcher.fail_register_on_call = RegistrationCount() + 3; + const std::string output = CaptureStdout([&] { reflector.OnInterfaceChanged(); }); + + EXPECT_EQ(RegistrationCount(), 2); // bring-up failed -> v6 stays fully down, nothing half-set-up + EXPECT_TRUE(reflector.IsValid()); // still valid + EXPECT_NE(output.find("ERROR"), std::string::npos) << output; // the registration failure was logged + // the first group's membership, already taken before the second group's failure, was rolled back too + EXPECT_NE(std::ranges::find(source.left_groups, IpAddress::SsdpGroupV6LinkLocal()), + source.left_groups.end()); + + packet_dispatcher.fail_register_on_call = 0; // the transient condition clears + reflector.OnInterfaceChanged(); // retried on the next change + EXPECT_EQ(RegistrationCount(), 6); // v4 (2) + v6's two groups x both directions (4) +} + // Multi-group construction rollback: when a LATER group of a family fails to register, the family's // already-set-up earlier groups are rolled back (BringUpFamily resets the setup) before the // reflector reports invalid. (IPv6 has two groups; fail the second's first registration.) @@ -733,6 +780,33 @@ TEST_F(SsdpReflectorTest, CapDropsSessionsBeyondTheLimit) { EXPECT_EQ(target.sent.size(), 32u); // 33rd search not reflected } +// MAX_SESSIONS (32) caps the whole sessions_ table, not each group's slice of it: 32 distinct +// searchers spread across all three SSDP groups (v4, v6 link-local, v6 site-local) fill the table, and +// the 33rd distinct searcher's session is dropped no matter which group it targets. A per-group cap +// would leave headroom in every group (site-local holds only 10 of the 32) and wrongly accept it. +TEST_F(SsdpReflectorTest, SessionCapIsGlobalAcrossGroups) { + SsdpReflector reflector{packet_dispatcher, source, target, MakeConfig(AddressFamily::Dual)}; + ASSERT_TRUE(reflector.IsValid()); + + const std::vector groups{ + IpAddress::SsdpGroupV4(), IpAddress::SsdpGroupV6LinkLocal(), IpAddress::SsdpGroupV6SiteLocal()}; + const auto search_payload = MakeSearch(); + for (uint16_t i = 0; i < 32; ++i) { + Packet search = MakePacket(search_payload, groups[i % groups.size()]); + search.header.source.port = static_cast(20000 + i); + packet_dispatcher.Deliver(source, search); + } + ASSERT_EQ(target.sent.size(), 32u); // all 32 distinct searchers reflected; table now full + + // The 33rd distinct searcher targets v6 site-local, which so far holds only 10 of the 32 sessions. + Packet overflow = MakePacket(search_payload, IpAddress::SsdpGroupV6SiteLocal()); + overflow.header.source.port = 20032; + const std::string output = CaptureStdout([&] { packet_dispatcher.Deliver(source, overflow); }); + + EXPECT_EQ(target.sent.size(), 32u); // not reflected: MakeSession returned nullopt before the reflect + EXPECT_NE(output.find("cap reached"), std::string::npos) << output; +} + TEST_F(SsdpReflectorTest, RetransmittedMSearchReusesOneSessionAndReflectsEach) { SsdpReflector reflector{packet_dispatcher, source, target, MakeConfig(AddressFamily::IPv4)}; ASSERT_TRUE(reflector.IsValid()); @@ -1042,6 +1116,41 @@ TEST_F(SsdpReflectorTest, DialForwardsLocationUnchangedWhenListenerMintFails) { EXPECT_NE(output.find("no listener"), std::string::npos) << output; // surfaced at INFO } +// Same mint-failure fallback as DialForwardsLocationUnchangedWhenListenerMintFails, but on the unicast +// response path (OnUnicastResponse's RewriteDialLocation call) rather than the advertisement path — the +// two call sites are independent branches that could regress separately. +TEST_F(SsdpReflectorTest, DialForwardsResponseLocationUnchangedWhenListenerMintFails) { + const ScopedMinLogLevel level{LogLevel::Info}; + auto config = MakeConfig(AddressFamily::IPv4); + config.dial = true; + SsdpReflector reflector{packet_dispatcher, source, target, config}; + ASSERT_TRUE(reflector.IsValid()); + + // An M-SEARCH establishes the session so the 200 OK has a searcher to be injected to. + packet_dispatcher.Deliver(source, MakePacket(MakeSearch(), IpAddress::SsdpGroupV4())); + ASSERT_EQ(target.sent.size(), 1u); + const uint16_t reserved_port = target.sent.back().src_port; + + source.iface.SetV4(std::nullopt); // no source_if V4 address -> EnsureDiscoveryListener cannot bind a listener + + const auto response = MakeDialResponse(); + Packet reply{ + .header = PacketHeader{ + .source = {IpAddress::FromV4Bytes(10, 0, 0, 5), SSDP_PORT}, + .dest = {*target.iface.SourceAddress(IpAddress::Family::V4), reserved_port}, + .ttl = 4, + }, + .payload = response, + }; + const std::string output = CaptureStdout([&] { + packet_dispatcher.Deliver(target, reply); + }); + + ASSERT_EQ(source.sent.size(), 1u); + EXPECT_EQ(source.sent.back().payload, response); // forwarded unchanged (benign fallback) + EXPECT_NE(output.find("no listener"), std::string::npos) << output; // surfaced at INFO +} + TEST_F(SsdpReflectorTest, LogsErrorWhenReflectingAdvertisementFails) { SsdpReflector reflector{packet_dispatcher, source, target, MakeConfig(AddressFamily::IPv4)}; ASSERT_TRUE(reflector.IsValid()); diff --git a/tests/test_helpers.h b/tests/test_helpers.h index f5532d3..8483fb5 100644 --- a/tests/test_helpers.h +++ b/tests/test_helpers.h @@ -349,6 +349,12 @@ struct TestCaptureSocket { ADD_FAILURE() << "socketpair() failed: " << std::strerror(errno); return RawSocket::ForTesting(iface, -1); } + // macOS defaults an AF_UNIX datagram socket to a ~2 KB receive buffer, too small to + // stage a full drain burst (MAX_PACKETS_PER_READ_EVENT+1 frames) before reading. Raise + // both ends so a bursty test can queue the whole batch up front. + const int bufsize = 1 << 20; + ::setsockopt(fds[0], SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)); + ::setsockopt(fds[1], SOL_SOCKET, SO_SNDBUF, &bufsize, sizeof(bufsize)); if (::fcntl(fds[0], F_SETFL, O_NONBLOCK) != 0) { ADD_FAILURE() << "fcntl(O_NONBLOCK) failed: " << std::strerror(errno); ::close(fds[0]);