diff --git a/src/helpers/RegionMap.cpp b/src/helpers/RegionMap.cpp index 9edc776f0..0b5a8b216 100644 --- a/src/helpers/RegionMap.cpp +++ b/src/helpers/RegionMap.cpp @@ -288,7 +288,10 @@ int RegionMap::getTransportKeysFor(const RegionEntry& src, TransportKey dest[], } RegionEntry* RegionMap::findMatch(const mesh::Packet* packet, uint8_t mask) { - if (packet->transport_codes[0] == 0) return &wildcard; + if (packet->transport_codes[0] == 0) { + // Zero means unscoped, not an exemption from the wildcard's deny policy. + return (wildcard.flags & mask) == 0 ? &wildcard : NULL; + } for (int i = 0; i < num_regions; i++) { auto region = ®ions[i]; if ((region->flags & mask) == 0) { // does region allow this? (per 'mask' param) diff --git a/src/helpers/RoutingPolicy.h b/src/helpers/RoutingPolicy.h index 5251a590a..d7aa9ae82 100644 --- a/src/helpers/RoutingPolicy.h +++ b/src/helpers/RoutingPolicy.h @@ -8,7 +8,7 @@ namespace mesh { * \brief Test a flood packet against the configured hop limits. * \param packet inbound flood packet (caller has already checked isRouteFlood()) * \param flood_max max hops for any flood packet - * \param flood_max_unscoped max hops for ROUTE_TYPE_FLOOD (ie. un-scoped) packets + * \param flood_max_unscoped max hops for ordinary or zero-code transport floods * \param flood_max_advert max hops for ADVERT packets * \returns true if the packet has exceeded a limit, and must not be forwarded */ @@ -16,7 +16,10 @@ inline bool isFloodHopLimitExceeded(const Packet* packet, uint8_t flood_max, uint8_t flood_max_unscoped, uint8_t flood_max_advert) { uint8_t hops = packet->getPathHashCount(); if (hops >= flood_max) return true; - if (packet->getRouteType() == ROUTE_TYPE_FLOOD && hops >= flood_max_unscoped) return true; + const uint8_t route_type = packet->getRouteType(); + const bool unscoped = route_type == ROUTE_TYPE_FLOOD + || (route_type == ROUTE_TYPE_TRANSPORT_FLOOD && packet->transport_codes[0] == 0); + if (unscoped && hops >= flood_max_unscoped) return true; if (packet->getPayloadType() == PAYLOAD_TYPE_ADVERT && hops >= flood_max_advert) return true; return false; } diff --git a/test/test_routing_policy/test_routing_policy.cpp b/test/test_routing_policy/test_routing_policy.cpp index bba9ec790..08903c08d 100644 --- a/test/test_routing_policy/test_routing_policy.cpp +++ b/test/test_routing_policy/test_routing_policy.cpp @@ -6,6 +6,10 @@ using namespace mesh; static Packet makeFlood(uint8_t route_type, uint8_t payload_type, uint8_t hops) { Packet p; p.header = route_type | (payload_type << PH_TYPE_SHIFT); + // A genuine scoped fixture needs a nonzero code; zero is unscoped even + // when it is carried in the transport-flood wire format. + p.transport_codes[0] = route_type == ROUTE_TYPE_TRANSPORT_FLOOD ? 0x1234 : 0; + p.transport_codes[1] = 0; p.setPathHashSizeAndCount(1, hops); p.payload_len = 1; return p; @@ -32,6 +36,27 @@ TEST(FloodHopLimit, UnscopedFloodSurvivesUpToMaxUnscopedHops) { EXPECT_TRUE(isFloodHopLimitExceeded(&too_far, 64, 3, 8)); } +TEST(FloodHopLimit, ZeroTransportCodeUsesUnscopedLimit) { + auto pkt = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_RESPONSE, 0); + pkt.transport_codes[0] = 0; + pkt.transport_codes[1] = 0x5678; // The second code cannot make it scoped. + EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 64, 0, 8)); + pkt.setPathHashCount(2); + EXPECT_FALSE(isFloodHopLimitExceeded(&pkt, 64, 3, 8)); + pkt.setPathHashCount(3); + EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 64, 3, 8)); +} + +TEST(FloodHopLimit, ZeroTransportAdvertUsesEveryApplicableLimit) { + auto pkt = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_ADVERT, 0); + pkt.transport_codes[0] = 0; + EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 64, 0, 8)); + pkt.setPathHashCount(2); + EXPECT_FALSE(isFloodHopLimitExceeded(&pkt, 64, 3, 8)); + EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 2, 3, 8)); + EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 64, 3, 2)); +} + TEST(FloodHopLimit, ScopedFloodStillHonoursFloodMaxAndAdvertMax) { auto beyond_max = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_RESPONSE, 5); EXPECT_TRUE(isFloodHopLimitExceeded(&beyond_max, 5, 64, 8)); diff --git a/test/test_upstream_routing_merge.py b/test/test_upstream_routing_merge.py index e3c1ee08f..5d71e091d 100644 --- a/test/test_upstream_routing_merge.py +++ b/test/test_upstream_routing_merge.py @@ -45,6 +45,15 @@ int main() { assert(regions.lookups == 0); packet.transport_codes[1] = 0x9999; assert(regions.findMatch(input, 1) == ®ions.wildcard); + regions.wildcard.flags = 1; + assert(regions.findMatch(input, 1) == nullptr); + assert(regions.findMatch(input, 2) == ®ions.wildcard); + assert(regions.findMatch(input, 0) == ®ions.wildcard); + assert(regions.lookups == 0); // A denied wildcard must not fall into a region. + regions.wildcard.flags = 3; + assert(regions.findMatch(input, 1) == nullptr); + assert(regions.findMatch(input, 2) == nullptr); + regions.wildcard.flags = 0; packet.transport_codes[0] = 0x1234; assert(regions.findMatch(input, 1) == ®ions.regions[0]); regions.regions[0].flags = 1; @@ -63,6 +72,56 @@ int main() { capture_output=True) subprocess.run([str(exe)], check=True, timeout=10, capture_output=True) + def test_zero_transport_code_obeys_ordinary_unscoped_hop_limits(self): + compiler = shutil.which("g++") or shutil.which("c++") + if not compiler: + self.skipTest("native C++ compiler required") + # Include the production packet representation and routing policy; + # only the constructor's platform-independent reset is supplied here. + packet_constructor = extract_braced((ROOT / "src/Packet.cpp").read_text(), + "Packet::Packet()") + source = r''' +#include +#include +#include +namespace mesh { +@CONSTRUCTOR@ +} +int main() { + for (uint8_t hash_size : {1, 2, 3}) { + for (uint8_t payload : {PAYLOAD_TYPE_RESPONSE, PAYLOAD_TYPE_ADVERT}) { + mesh::Packet packet; + packet.header = ROUTE_TYPE_TRANSPORT_FLOOD | (payload << PH_TYPE_SHIFT); + packet.transport_codes[0] = 0; + packet.transport_codes[1] = 0x5678; + packet.setPathHashSizeAndCount(hash_size, 0); + assert(mesh::isFloodHopLimitExceeded(&packet, 64, 0, 8)); + packet.setPathHashCount(2); + assert(!mesh::isFloodHopLimitExceeded(&packet, 64, 3, 8)); + packet.setPathHashCount(3); + assert(mesh::isFloodHopLimitExceeded(&packet, 64, 3, 8)); + packet.transport_codes[0] = 0x1234; + assert(!mesh::isFloodHopLimitExceeded(&packet, 64, 0, 8)); + assert(mesh::isFloodHopLimitExceeded(&packet, 3, 64, 8)); + if (payload == PAYLOAD_TYPE_ADVERT) + assert(mesh::isFloodHopLimitExceeded(&packet, 64, 64, 3)); + } + } +} +'''.replace("@CONSTRUCTOR@", packet_constructor) + with tempfile.TemporaryDirectory(prefix="mesh-upstream-hop-limits-") as directory: + cpp, exe = Path(directory) / "routing.cpp", Path(directory) / "routing" + cpp.write_text(source) + built = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + "-Wno-unused-parameter", + "-fsanitize=address,undefined", "-fno-pie", "-no-pie", + "-I", str(ROOT / "src"), str(cpp), "-o", str(exe), + ], text=True, capture_output=True, timeout=30) + self.assertEqual(built.returncode, 0, built.stderr) + ran = subprocess.run([str(exe)], text=True, capture_output=True, timeout=10) + self.assertEqual(ran.returncode, 0, ran.stderr) + def test_room_repeat_deprecation_keeps_full_rules_and_clock_sync(self): source = (ROOT / "examples/simple_room_server/MyMesh.cpp").read_text() handler = extract_braced(source, "void MyMesh::handleCommand(")