Apply wildcard deny and hop limits to zero-scope floods

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 = &regions[i];
     if ((region->flags & mask) == 0) {   // does region allow this? (per 'mask' param)

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;
 }

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));

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) == &regions.wildcard);
+  regions.wildcard.flags = 1;
+  assert(regions.findMatch(input, 1) == nullptr);
+  assert(regions.findMatch(input, 2) == &regions.wildcard);
+  assert(regions.findMatch(input, 0) == &regions.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) == &regions.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 <cassert>
+#include <initializer_list>
+#include <helpers/RoutingPolicy.h>
+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(")