[1641] added comprehensive NSEC3::compare() tests. found bugs, and fixed them.

src/lib/dns/rdata/generic/nsec3_50.cc

@@ -281,6 +281,26 @@ NSEC3::toWire(AbstractMessageRenderer& renderer) const {
     toWireHelper(*impl_, renderer);
 }
 
+namespace {
+int
+compareVectors(const vector<uint8_t>& v1, const vector<uint8_t>& v2,
+               bool check_length_first = true)
+{
+    const size_t len1 = v1.size();
+    const size_t len2 = v2.size();
+    const size_t cmplen = min(len1, len2);
+    if (check_length_first && len1 != len2) {
+        return (len1 < len2 ? -1 : 1);
+    }
+    const int cmp = cmplen == 0 ? 0 : memcmp(&v1.at(0), &v2.at(0), cmplen);
+    if (cmp != 0) {
+        return (cmp);
+    } else {
+        return (len1 - len2);
+    }
+}
+}
+
 int
 NSEC3::compare(const Rdata& other) const {
     const NSEC3& other_nsec3 = dynamic_cast<const NSEC3&>(other);
@@ -295,44 +315,18 @@ NSEC3::compare(const Rdata& other) const {
         return (impl_->iterations_ < other_nsec3.impl_->iterations_ ? -1 : 1);
     }
 
-    size_t this_len = impl_->salt_.size();
-    size_t other_len = other_nsec3.impl_->salt_.size();
-    size_t cmplen = min(this_len, other_len);
-    int cmp = memcmp(&impl_->salt_[0], &other_nsec3.impl_->salt_[0], cmplen);
-    if (cmp != 0) {
-        return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    }
-
-    this_len = impl_->salt_.size();
-    other_len = other_nsec3.impl_->salt_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->next_[0], &other_nsec3.impl_->next_[0], cmplen);
+    int cmp = compareVectors(impl_->salt_, other_nsec3.impl_->salt_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
     }
-
-    this_len = impl_->typebits_.size();
-    other_len = other_nsec3.impl_->typebits_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->typebits_[0], &other_nsec3.impl_->typebits_[0],
-                 cmplen);
+    cmp = compareVectors(impl_->next_, other_nsec3.impl_->next_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    } else {
-        return (0);
     }
+    // Note that bitmap doesn't have a dedicated length field, so we shouldn't
+    // terminate the comparison just because the lengths are different.
+    return (compareVectors(impl_->typebits_, other_nsec3.impl_->typebits_,
+                           false));
 }
 
 uint8_t

src/lib/dns/tests/rdata_nsec3_unittest.cc

@@ -221,7 +221,7 @@ TEST_F(Rdata_NSEC3_Test, toWire) {
     toWireCheck(renderer, "rdata_nsec3_fromWire13.wire");
     toWireCheck(obuffer, "rdata_nsec3_fromWire13.wire");
 
-    // empty bitmap case is handled in the bitmpa tests
+    // empty bitmap case is handled in the bitmap tests
 }
 
 TEST_F(Rdata_NSEC3_Test, assign) {
@@ -230,4 +230,46 @@ TEST_F(Rdata_NSEC3_Test, assign) {
     EXPECT_EQ(0, rdata_nsec3.compare(other_nsec3));
 }
 
+TEST_F(Rdata_NSEC3_Test, compare) {
+    // trivial case: self equivalence
+    EXPECT_EQ(0, generic::NSEC3(nsec3_txt).compare(generic::NSEC3(nsec3_txt)));
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(generic::NSEC3(nsec3_txt).compare(*rdata_nomatch),
+                 bad_cast);
+
+    // test RDATAs, sorted in the ascendent order.
+    vector<generic::NSEC3> compare_set;
+    compare_set.push_back(generic::NSEC3("0 0 0 D399EAAB D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 0 0 D399EAAB D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 0 D399EAAB D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 - D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 D399EAAB D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EAAB D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ0000000000"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ00UUUUUUUU"));
+    compare_set.push_back(generic::NSEC3("1 1 2 FF99EA0000 D1K6GQ38"));
+
+    // Bit map: [win=0][len=1] 00000010
+    compare_set.push_back(generic::NSEC3("1 1 2 FF99EA0000 D1K6GQ38 SOA"));
+    // Bit map: [win=0][len=1] 00100000, [win=4][len=1] 10000000
+    compare_set.push_back(generic::NSEC3(
+                              "1 1 2 FF99EA0000 D1K6GQ38 NS TYPE1024"));
+    // Bit map: [win=0][len=1] 00100010
+    compare_set.push_back(generic::NSEC3("1 1 2 FF99EA0000 D1K6GQ38 NS SOA"));
+    // Bit map: [win=0][len=2] 00100000, 00000001
+    compare_set.push_back(generic::NSEC3("1 1 2 FF99EA0000 D1K6GQ38 NS MX"));
+    // Bit map: [win=4][len=1] 10000000
+    compare_set.push_back(generic::NSEC3("1 1 2 FF99EA0000 D1K6GQ38 TYPE1024"));
+    vector<generic::NSEC3>::const_iterator it;
+    const vector<generic::NSEC3>::const_iterator it_end = compare_set.end();
+    for (it = compare_set.begin(); it != it_end - 1; ++it) {
+        SCOPED_TRACE("compare " + it->toText() + " to " + (it + 1)->toText());
+        EXPECT_GT(0, (*it).compare(*(it + 1)));
+        EXPECT_LT(0, (*(it + 1)).compare(*it));
+    }
+
+}
+
 }