rec: Make more specific Netmask < to less specific ones

Having the most specific ones first, then the less specific ones
then the empty one makes it easier to match the most specific first.

diff --git a/pdns/iputils.hh b/pdns/iputils.hh
index a4a4862c0a9db8cee8d78de469bc3e27627c08d6..30849cd6b302558d966ddedd101e1ca155e80ca7 100644 (file)
--- a/pdns/iputils.hh
+++ b/pdns/iputils.hh
@@ -444,7 +444,23 @@ public:
 
   bool operator<(const Netmask& rhs) const 
   {
-    return tie(d_network, d_bits) < tie(rhs.d_network, rhs.d_bits);
+    if (empty() && !rhs.empty())
+      return false;
+
+    if (!empty() && rhs.empty())
+      return true;
+
+    if (d_bits > rhs.d_bits)
+      return true;
+    if (d_bits < rhs.d_bits)
+      return false;
+
+    return d_network < rhs.d_network;
+  }
+
+  bool operator>(const Netmask& rhs) const
+  {
+    return rhs.operator<(*this);
   }
 
   bool operator==(const Netmask& rhs) const 

diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 1974bbea8ac317444d95d033e03d86012f068727..cebc9685bf945b8362d77b58e622e709428690a5 100644 (file)
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -256,6 +256,47 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) {
     BOOST_REQUIRE_EQUAL(retrieved.size(), 1);
     BOOST_CHECK_EQUAL(getRR<ARecordContent>(retrieved.at(0))->getCA().toString(), dr2Content.toString());
 
+    // Most specific netmask test
+    // wipe everything
+    MRC.doWipeCache(DNSName("."), true);
+    BOOST_CHECK_EQUAL(MRC.size(), 0);
+    records.clear();
+
+    // insert an entry for 192.0.0.1/8
+    records.clear();
+    records.push_back(dr2);
+    MRC.replace(now, power, QType(QType::A), records, signatures, true, boost::optional<Netmask>("192.0.0.1/8"));
+    BOOST_CHECK_EQUAL(MRC.size(), 1);
+
+    /* same as dr2 except for the actual IP */
+    DNSRecord dr4;
+    ComboAddress dr4Content("192.0.2.126");
+    dr4.d_name = power;
+    dr4.d_type = QType::A;
+    dr4.d_class = QClass::IN;
+    dr4.d_content = std::make_shared<ARecordContent>(dr4Content);
+    dr4.d_ttl = static_cast<uint32_t>(ttd);
+    dr4.d_place = DNSResourceRecord::AUTHORITY;
+
+    // insert an other entry but for 192.168.0.1/31
+    records.clear();
+    records.push_back(dr4);
+    MRC.replace(now, power, QType(QType::A), records, signatures, true, boost::optional<Netmask>("192.168.0.1/31"));
+    // we should not have replaced any existing entry
+    BOOST_CHECK_EQUAL(MRC.size(), 2);
+
+    // insert the same than the first one but for 192.168.0.2/32
+    records.clear();
+    records.push_back(dr2);
+    MRC.replace(now, power, QType(QType::A), records, signatures, true, boost::optional<Netmask>("192.168.0.2/32"));
+    // we should not have replaced any existing entry
+    BOOST_CHECK_EQUAL(MRC.size(), 3);
+
+    // we should get the most specific entry for 192.168.0.1, so the second one
+    BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("192.168.0.1"), nullptr), (ttd-now));
+    BOOST_REQUIRE_EQUAL(retrieved.size(), 1);
+    BOOST_CHECK_EQUAL(getRR<ARecordContent>(retrieved.at(0))->getCA().toString(), dr4Content.toString());
+    retrieved.clear();
   }
   catch(const PDNSException& e) {
     cerr<<"Had error: "<<e.reason<<endl;

diff --git a/pdns/test-iputils_hh.cc b/pdns/test-iputils_hh.cc
index 1f2a392d8f1942e1011338837b3c0e8bd863d144..9e620de33c3054a00a16793e3b58754db27bc815 100644 (file)
--- a/pdns/test-iputils_hh.cc
+++ b/pdns/test-iputils_hh.cc
@@ -191,6 +191,7 @@ BOOST_AUTO_TEST_CASE(test_Netmask) {
   Netmask all6("::/0");
   BOOST_CHECK(all6.match("::1") && all6.match("fe80::92fb:a6ff:fe4a:51da"));
 
+
   Netmask fromCombo1(ComboAddress("192.0.2.1:53"), 32);
   Netmask fromCombo2(ComboAddress("192.0.2.1:54"), 32);
   BOOST_CHECK(fromCombo1 == fromCombo2);
@@ -203,6 +204,30 @@ BOOST_AUTO_TEST_CASE(test_Netmask) {
   BOOST_CHECK(nm25.getBits() == 25);
   BOOST_CHECK(nm25.getNetwork() == ComboAddress("192.0.2.255"));
   BOOST_CHECK(nm25.getMaskedNetwork() == ComboAddress("192.0.2.128"));
+
+  /* Make sure that more specific Netmasks are lesser than less specific ones,
+     as this is very useful when matching. */
+  Netmask specific32("192.0.0.0/32");
+  Netmask specific24("192.0.0.0/24");
+  Netmask specific16("192.0.0.0/16");
+  BOOST_CHECK(specific32 < specific24);
+  BOOST_CHECK(specific24 > specific32);
+  BOOST_CHECK(specific24 < specific16);
+  BOOST_CHECK(specific16 > specific24);
+
+  Netmask sameMask1("192.0.0.0/16");
+  Netmask sameMask2("192.0.0.1/16");
+  BOOST_CHECK(sameMask1 < sameMask2);
+  BOOST_CHECK(sameMask2 > sameMask1);
+
+  /* An empty Netmask should be larger than
+     every others. */
+  Netmask empty = Netmask();
+  Netmask full("255.255.255.255/32");
+  BOOST_CHECK(empty > all);
+  BOOST_CHECK(all < empty);
+  BOOST_CHECK(empty > full);
+  BOOST_CHECK(full < empty);
 }
 
 BOOST_AUTO_TEST_CASE(test_NetmaskGroup) {