Add a "pretty" flavour to DNSName::canonCompare, to be used by pdnsutil only.

This flavour sorts labels made of digits numerically.

Co-Authored-By: Peter Samuelson <psamuelson@efolder.net>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>

diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index e78aea3f068bfbf410177a2f9147b056e1bf21f6..97c0c30c3c178a8a62b234910750d3d5e14fc9d3 100644 (file)
--- a/pdns/dnsname.cc
+++ b/pdns/dnsname.cc
@@ -505,7 +505,7 @@ int DNSName::slowCanonCompare_three_way(const DNSName& rhs) const
   return 0; // eq
 }
 
-int DNSName::canonCompare_three_way(const DNSName& rhs) const
+int DNSName::canonCompare_three_way(const DNSName& rhs, bool pretty) const
 {
   //      01234567890abcd
   // us:  1a3www4ds9a2nl
@@ -544,14 +544,42 @@ int DNSName::canonCompare_three_way(const DNSName& rhs) const
     rhscount--;
 
     // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
-    int res = pdns_ilexicographical_compare_three_way(
-      std::string_view(
-        d_storage.c_str() + ourpos.at(ourcount) + 1,
-        *(d_storage.c_str() + ourpos.at(ourcount))),
-      std::string_view(
-        rhs.d_storage.c_str() + rhspos.at(rhscount) + 1,
-        *(rhs.d_storage.c_str() + rhspos.at(rhscount))));
+    const uint8_t ourlen = *(d_storage.c_str() + ourpos.at(ourcount));
+    const uint8_t rhslen = *(rhs.d_storage.c_str() + rhspos.at(rhscount));
+    std::string_view ourstr(d_storage.c_str() + ourpos.at(ourcount) + 1, ourlen);
+    std::string_view rhsstr(rhs.d_storage.c_str() + rhspos.at(rhscount) + 1, rhslen);
     // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+
+    // If pretty ordering requested, sort numerical values, well,
+    // numerically (i.e. 999.example.com < 1000.example.com).
+    // Do not use for anything but human-intended output, as this breaks
+    // the DNSSEC order.
+    if (pretty) {
+      // If both names are numerical (made of digits), then the longest one
+      // always compares higher.
+      if (ourlen != rhslen) {
+        bool isNumerical{true};
+        for (const auto chr : ourstr) {
+          if (std::isdigit(static_cast<unsigned char>(chr)) == 0) {
+            isNumerical = false;
+            break;
+          }
+        }
+        if (isNumerical) {
+          for (const auto chr : rhsstr) {
+            if (std::isdigit(static_cast<unsigned char>(chr)) == 0) {
+              isNumerical = false;
+              break;
+            }
+          }
+        }
+        if (isNumerical) {
+          return ourlen < rhslen ? -1 : 1;
+        }
+      }
+    }
+
+    int res = pdns_ilexicographical_compare_three_way(ourstr, rhsstr);
     if (res != 0) {
       return res;
     }

diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
index 047dfb9a64c9f47503da3d551cfbb0145b9f00e8..9756b5faef5cd5e18101f0c02089964a4e1cd62e 100644 (file)
--- a/pdns/dnsname.hh
+++ b/pdns/dnsname.hh
@@ -192,8 +192,8 @@ public:
   }
 
   int slowCanonCompare_three_way(const DNSName& rhs) const;
-  int canonCompare_three_way(const DNSName& rhs) const;
-  inline bool canonCompare(const DNSName& rhs) const { return canonCompare_three_way(rhs) < 0; }
+  int canonCompare_three_way(const DNSName& rhs, bool pretty = false) const;
+  inline bool canonCompare(const DNSName& rhs, bool pretty = false) const { return canonCompare_three_way(rhs, pretty) < 0; }
 
   typedef boost::container::string string_t;
 

diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index f5f82695c4e31dd366792e766fada1cb7d230e0f..4deb14f35b4e5aa18b1a04779f655df80c6da2ba 100644 (file)
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -398,7 +398,7 @@ public:
     auto aType = (a.d_type == QType::SOA) ? 0 : a.d_type;
     auto bType = (b.d_type == QType::SOA) ? 0 : b.d_type;
 
-    int res = a.d_name.canonCompare_three_way(b.d_name);
+    int res = a.d_name.canonCompare_three_way(b.d_name, true);
     if (res < 0) {
       return true;
     }

diff --git a/pdns/lua-base4.cc b/pdns/lua-base4.cc
index 30c4b9a92d996433c180e64ee711d7ece4244c7e..4e7554996e9a6463ada595af8257e075daf092c3 100644 (file)
--- a/pdns/lua-base4.cc
+++ b/pdns/lua-base4.cc
@@ -102,7 +102,7 @@ void BaseLua4::prepareContext() {
   // DNSName
   d_lw->writeFunction("newDN", [](const std::string& dom){ return DNSName(dom); });
   d_lw->registerFunction("__lt", &DNSName::operator<);
-  d_lw->registerFunction("canonCompare", &DNSName::canonCompare);
+  d_lw->registerFunction<bool(DNSName::*)(const DNSName&)>("canonCompare", [](const DNSName& name, const DNSName& rhs) { return name.canonCompare(rhs); });
   d_lw->registerFunction<DNSName(DNSName::*)(const DNSName&)>("makeRelative", [](const DNSName& name, const DNSName& zone) { return name.makeRelative(zone); });
   d_lw->registerFunction<bool(DNSName::*)(const DNSName&)>("isPartOf", [](const DNSName& name, const DNSName& rhs) { return name.isPartOf(rhs); });
   d_lw->registerFunction("getRawLabels", &DNSName::getRawLabels);

diff --git a/pdns/test-dnsname_cc.cc b/pdns/test-dnsname_cc.cc
index d7ee97b726228420fe27dd2065b2f5c356fced3f..b100f3a05a80e05b359f1f7d5528499fd554cf36 100644 (file)
--- a/pdns/test-dnsname_cc.cc
+++ b/pdns/test-dnsname_cc.cc
@@ -738,6 +738,14 @@ BOOST_AUTO_TEST_CASE(test_compare_canonical) {
   BOOST_CHECK(vec==right);
 }
 
+BOOST_AUTO_TEST_CASE(test_compare_pretty) {
+  BOOST_CHECK(DNSName("99.2.1.10.in-addr.arpa").canonCompare(DNSName("101.2.1.10.in-addr.arpa"), true));
+  BOOST_CHECK(!DNSName("101.2.1.10.in-addr.arpa").canonCompare(DNSName("99.2.1.10.in-addr.arpa"), true));
+  BOOST_CHECK(DNSName("ns12.example.com").canonCompare(DNSName("ns8.example.com"), true));
+  BOOST_CHECK(!DNSName("ns8.example.com").canonCompare(DNSName("ns12.example.com"), true));
+  BOOST_CHECK(DNSName("135.org").canonCompare(DNSName("2x.org"), true));
+  BOOST_CHECK(!DNSName("2x.org").canonCompare(DNSName("135.org"), true));
+}
 
 BOOST_AUTO_TEST_CASE(test_empty_label) { // empty label