Browse Source

[trac117] handled empty type bitmap correctly. added tests for this case.

the original "from wire" code was actually buggy, too, and it's now fixed.
JINMEI Tatuya 14 years ago
parent
commit
d7a9547198

+ 18 - 11
src/lib/dns/rdata/generic/nsec3_50.cc

@@ -64,10 +64,8 @@ NSEC3::NSEC3(const string& nsec3_str) :
     istringstream iss(nsec3_str);
     unsigned int hashalg, flags, iterations;
     string iterations_str, salthex, nexthash;
-    stringbuf bitmaps;
 
-    iss >> hashalg >> flags >> iterations_str >> salthex >> nexthash
-        >> &bitmaps;
+    iss >> hashalg >> flags >> iterations_str >> salthex >> nexthash;
     if (iss.bad() || iss.fail()) {
         isc_throw(InvalidRdataText, "Invalid NSEC3 text: " << nsec3_str);
     }
@@ -107,15 +105,20 @@ NSEC3::NSEC3(const string& nsec3_str) :
                   << next.size() << " bytes");
     }
 
-    stringstream bitmap_stream(bitmaps.str());
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    vector<uint8_t> typebits;
+    // For NSEC3 empty bitmap is possible and allowed.
+    if (iss.eof()) {
+        impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next,
+                              vector<uint8_t>());
+        return;
+    }
 
+    vector<uint8_t> typebits;
+    uint8_t bitmap[8 * 1024];       // 64k bits
     memset(bitmap, 0, sizeof(bitmap));
     do { 
         string type;
         int code;
-        bitmap_stream >> type;
+        iss >> type;
         if (type.length() != 0) {
             try {
                 code = RRType(type).getCode();
@@ -124,7 +127,7 @@ NSEC3::NSEC3(const string& nsec3_str) :
                 isc_throw(InvalidRdataText, "Invalid RRtype in NSEC3");
             }
         }
-    } while (!bitmap_stream.eof());
+    } while (!iss.eof());
 
     for (int window = 0; window < 256; window++) {
         int octet;
@@ -171,7 +174,7 @@ NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
 
     const uint8_t nextlen = buffer.readUint8();
     --rdata_len;
-    if (nextlen == 0 || rdata_len <= nextlen) {
+    if (nextlen == 0 || rdata_len < nextlen) {
         isc_throw(DNSMessageFORMERR, "NSEC3 invalid hash length: " <<
                   static_cast<unsigned int>(nextlen));
     }
@@ -181,8 +184,12 @@ NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
     rdata_len -= nextlen;
 
     vector<uint8_t> typebits(rdata_len);
-    buffer.readData(&typebits[0], rdata_len);
-    checkRRTypeBitmaps("NSEC3", typebits);
+    if (rdata_len > 0) {
+        // Read and parse the bitmaps only when they exist; empty bitmap
+        // is possible for NSEC3.
+        buffer.readData(&typebits[0], rdata_len);
+        checkRRTypeBitmaps("NSEC3", typebits);
+    }
 
     impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
 }

+ 7 - 4
src/lib/dns/tests/rdata_nsec3_unittest.cc

@@ -71,6 +71,9 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
     EXPECT_EQ(255, generic::NSEC3("1 1 1 D399EAAB " +
                                   string((255 * 8) / 5, '0') +
                                   " NS").getNext().size());
+
+    // type bitmap is empty.  it's possible and allowed for NSEC3.
+    generic::NSEC3("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6");
 }
 
 TEST_F(Rdata_NSEC3_Test, toText) {
@@ -79,10 +82,6 @@ TEST_F(Rdata_NSEC3_Test, toText) {
 }
 
 TEST_F(Rdata_NSEC3_Test, badText) {
-    // Bitmap is missing
-    EXPECT_THROW(generic::NSEC3(
-                     "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"),
-                 InvalidRdataText);
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                 "BIFF POW SPOON"),
@@ -131,6 +130,10 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
                   *rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                         "rdata_nsec3_fromWire1")));
 
+    // A valid NSEC3 RR with empty type bitmap.
+    EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
+                                         "rdata_nsec3_fromWire15.wire"));
+
     // Too short RDLENGTH: it doesn't even contain the first 5 octets.
     EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                       "rdata_nsec3_fromWire2.wire"),

+ 2 - 2
src/lib/dns/tests/testdata/Makefile.am

@@ -14,7 +14,7 @@ BUILT_SOURCES += rdata_nsec3_fromWire6.wire rdata_nsec3_fromWire7.wire
 BUILT_SOURCES += rdata_nsec3_fromWire8.wire rdata_nsec3_fromWire9.wire
 BUILT_SOURCES += rdata_nsec3_fromWire10.wire rdata_nsec3_fromWire11.wire
 BUILT_SOURCES += rdata_nsec3_fromWire12.wire rdata_nsec3_fromWire13.wire
-BUILT_SOURCES += rdata_nsec3_fromWire14.wire
+BUILT_SOURCES += rdata_nsec3_fromWire14.wire rdata_nsec3_fromWire15.wire
 BUILT_SOURCES += rdata_rrsig_fromWire2.wire
 BUILT_SOURCES += rdata_soa_toWireUncompressed.wire
 BUILT_SOURCES += rdata_txt_fromWire2.wire rdata_txt_fromWire3.wire
@@ -63,7 +63,7 @@ EXTRA_DIST += rdata_nsec3_fromWire6.spec rdata_nsec3_fromWire7.spec
 EXTRA_DIST += rdata_nsec3_fromWire8.spec rdata_nsec3_fromWire9.spec
 EXTRA_DIST += rdata_nsec3_fromWire10.spec rdata_nsec3_fromWire11.spec
 EXTRA_DIST += rdata_nsec3_fromWire12.spec rdata_nsec3_fromWire13.spec
-EXTRA_DIST += rdata_nsec3_fromWire14.spec
+EXTRA_DIST += rdata_nsec3_fromWire14.spec rdata_nsec3_fromWire15.spec
 EXTRA_DIST += rdata_opt_fromWire rdata_rrsig_fromWire1
 EXTRA_DIST += rdata_rrsig_fromWire2.spec
 EXTRA_DIST += rdata_soa_fromWire rdata_soa_toWireUncompressed.spec

+ 10 - 0
src/lib/dns/tests/testdata/rdata_nsec3_fromWire15.spec

@@ -0,0 +1,10 @@
+#
+# NSEC3 RDATA with empty type bitmap.  It's okay.
+# The test data includes bytes for a bitmap field, but RDLEN indicates
+# it's not part of the RDATA and so it will be ignored.
+#
+
+[custom]
+sections: nsec3
+[nsec3]
+rdlen: 31