Browse Source

Merge branch 'master' into trac1580

Xie Jiagui 13 years ago
parent
commit
5b2b5a2c16

+ 18 - 1
ChangeLog

@@ -1,5 +1,22 @@
+380.	[bug]		jinmei
+	libdns++: miscellaneous bug fixes for the NSECPARAM RDATA
+	implementation, including incorrect handling for empty salt and
+	incorrect comparison logic.
+	(Trac #1638, git 966c129cc3c538841421f1e554167d33ef9bdf25)
+
+379.	[bug]		jelte
+	Configuration commands in bindctl now check for list indices if
+	the 'identifier' argument points to a child element of a list
+	item. Previously, it was possible to 'get' non-existent values
+	by leaving out the index, e.g. "config show Auth/listen_on/port,
+	which should be config show Auth/listen_on[<index>]/port, since
+	Auth/listen_on is a list. The command without an index will now
+	show an error. It is still possible to show/set the entire list
+	("config show Auth/listen_on").
+	(Trac #1649, git 003ca8597c8d0eb558b1819dbee203fda346ba77)
+
 378.	[func]		vorner
 378.	[func]		vorner
-	It possible to start authoritative server or resolver in multiple
+	It is possible to start authoritative server or resolver in multiple
 	instances, to use more than one core. Configuration is described in
 	instances, to use more than one core. Configuration is described in
 	the guide.
 	the guide.
 	(Trac #1596, git 17f7af0d8a42a0a67a2aade5bc269533efeb840a)
 	(Trac #1596, git 17f7af0d8a42a0a67a2aade5bc269533efeb840a)

+ 1 - 1
configure.ac

@@ -907,7 +907,7 @@ AC_PATH_PROGS(AWK, gawk awk)
 AC_SUBST(AWK)
 AC_SUBST(AWK)
 
 
 AC_ARG_ENABLE(man, [AC_HELP_STRING([--enable-man],
 AC_ARG_ENABLE(man, [AC_HELP_STRING([--enable-man],
-  [regenerate man pages [default=no]])], enable_man=yes, enable_man=no)
+  [regenerate man pages [default=no]])], enable_man=$enableval, enable_man=no)
 
 
 AM_CONDITIONAL(ENABLE_MAN, test x$enable_man != xno)
 AM_CONDITIONAL(ENABLE_MAN, test x$enable_man != xno)
 
 

+ 8 - 8
src/bin/auth/auth.spec.pre.in

@@ -216,7 +216,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 8",
         "item_title": "Received requests opcode 8",
-        "item_description": "The number of total request counts whose opcode is8 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 8 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved9",
         "item_name": "opcode.reserved9",
@@ -224,7 +224,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 9",
         "item_title": "Received requests opcode 9",
-        "item_description": "The number of total request counts whose opcode is9 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 9 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved10",
         "item_name": "opcode.reserved10",
@@ -232,7 +232,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 10",
         "item_title": "Received requests opcode 10",
-        "item_description": "The number of total request counts whose opcode is10 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 10 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved11",
         "item_name": "opcode.reserved11",
@@ -240,7 +240,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 11",
         "item_title": "Received requests opcode 11",
-        "item_description": "The number of total request counts whose opcode is11 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 11 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved12",
         "item_name": "opcode.reserved12",
@@ -248,7 +248,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 12",
         "item_title": "Received requests opcode 12",
-        "item_description": "The number of total request counts whose opcode is12 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 12 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved13",
         "item_name": "opcode.reserved13",
@@ -256,7 +256,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 13",
         "item_title": "Received requests opcode 13",
-        "item_description": "The number of total request counts whose opcode is13 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 13 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved14",
         "item_name": "opcode.reserved14",
@@ -264,7 +264,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 14",
         "item_title": "Received requests opcode 14",
-        "item_description": "The number of total request counts whose opcode is14 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 14 (reserved)"
       },
       },
       {
       {
         "item_name": "opcode.reserved15",
         "item_name": "opcode.reserved15",
@@ -272,7 +272,7 @@
         "item_optional": true,
         "item_optional": true,
         "item_default": 0,
         "item_default": 0,
         "item_title": "Received requests opcode 15",
         "item_title": "Received requests opcode 15",
-        "item_description": "The number of total request counts whose opcode is15 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 15 (reserved)"
       }
       }
     ]
     ]
   }
   }

+ 4 - 0
src/lib/dns/Makefile.am

@@ -23,6 +23,8 @@ EXTRA_DIST += rdata/generic/cname_5.cc
 EXTRA_DIST += rdata/generic/cname_5.h
 EXTRA_DIST += rdata/generic/cname_5.h
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.cc
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.cc
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.h
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.h
+EXTRA_DIST += rdata/generic/detail/nsec3param_common.cc
+EXTRA_DIST += rdata/generic/detail/nsec3param_common.h
 EXTRA_DIST += rdata/generic/detail/txt_like.h
 EXTRA_DIST += rdata/generic/detail/txt_like.h
 EXTRA_DIST += rdata/generic/detail/ds_like.h
 EXTRA_DIST += rdata/generic/detail/ds_like.h
 EXTRA_DIST += rdata/generic/dlv_32769.cc
 EXTRA_DIST += rdata/generic/dlv_32769.cc
@@ -113,6 +115,8 @@ libdns___la_SOURCES += tsigrecord.h tsigrecord.cc
 libdns___la_SOURCES += character_string.h character_string.cc
 libdns___la_SOURCES += character_string.h character_string.cc
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc
+libdns___la_SOURCES += rdata/generic/detail/nsec3param_common.cc
+libdns___la_SOURCES += rdata/generic/detail/nsec3param_common.h
 libdns___la_SOURCES += rdata/generic/detail/txt_like.h
 libdns___la_SOURCES += rdata/generic/detail/txt_like.h
 libdns___la_SOURCES += rdata/generic/detail/ds_like.h
 libdns___la_SOURCES += rdata/generic/detail/ds_like.h
 
 

+ 130 - 0
src/lib/dns/rdata/generic/detail/nsec3param_common.cc

@@ -0,0 +1,130 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+
+#include <util/encode/hex.h>
+#include <util/buffer.h>
+
+#include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
+
+#include <boost/lexical_cast.hpp>
+
+#include <sstream>
+#include <vector>
+#include <stdint.h>
+
+using namespace std;
+using namespace isc::util;
+using namespace isc::util::encode;
+
+namespace isc {
+namespace dns {
+namespace rdata {
+namespace generic {
+namespace detail {
+namespace nsec3 {
+
+ParseNSEC3ParamResult
+parseNSEC3ParamText(const char* const rrtype_name,
+                    const string& rdata_str, istringstream& iss,
+                    vector<uint8_t>& salt)
+{
+    unsigned int hashalg, flags, iterations;
+    string iterations_str, salthex;
+
+    iss >> hashalg >> flags >> iterations_str >> salthex;
+    if (iss.bad() || iss.fail()) {
+        isc_throw(InvalidRdataText, "Invalid " << rrtype_name <<
+                  " text: " << rdata_str);
+    }
+    if (hashalg > 0xff) {
+        isc_throw(InvalidRdataText, rrtype_name <<
+                  " hash algorithm out of range: " << hashalg);
+    }
+    if (flags > 0xff) {
+        isc_throw(InvalidRdataText, rrtype_name << " flags out of range: " <<
+                  flags);
+    }
+    // Convert iteration.  To reject an invalid case where there's no space
+    // between iteration and salt, we extract this field as string and convert
+    // to integer.
+    try {
+        iterations = boost::lexical_cast<unsigned int>(iterations_str);
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(InvalidRdataText, "Bad " << rrtype_name <<
+                  " iteration: " << iterations_str);
+    }
+    if (iterations > 0xffff) {
+        isc_throw(InvalidRdataText, rrtype_name <<
+                  " iterations out of range: " <<
+            iterations);
+    }
+
+    // Salt is up to 255 bytes, and space is not allowed in the HEX encoding,
+    // so the encoded string cannot be longer than the double of max length
+    // of the actual salt.
+    if (salthex.size() > 255 * 2) {
+        isc_throw(InvalidRdataText, rrtype_name << " salt is too long: "
+                  << salthex.size() << " (encoded) bytes");
+    }
+    if (salthex != "-") {       // "-" means a 0-length salt
+        decodeHex(salthex, salt);
+    }
+
+    return (ParseNSEC3ParamResult(hashalg, flags, iterations));
+}
+
+ParseNSEC3ParamResult
+parseNSEC3ParamWire(const char* const rrtype_name,
+                    InputBuffer& buffer,
+                    size_t& rdata_len, std::vector<uint8_t>& salt)
+{
+    // NSEC3/NSEC3PARAM RR must have at least 5 octets:
+    // hash algorithm(1), flags(1), iteration(2), saltlen(1)
+    if (rdata_len < 5) {
+        isc_throw(DNSMessageFORMERR, rrtype_name << " too short, length: "
+                  << rdata_len);
+    }
+
+    const uint8_t hashalg = buffer.readUint8();
+    const uint8_t flags = buffer.readUint8();
+    const uint16_t iterations = buffer.readUint16();
+
+    const uint8_t saltlen = buffer.readUint8();
+    rdata_len -= 5;
+    if (rdata_len < saltlen) {
+        isc_throw(DNSMessageFORMERR, rrtype_name <<
+                  " salt length is too large: " <<
+                  static_cast<unsigned int>(saltlen));
+    }
+
+    salt.resize(saltlen);
+    if (saltlen > 0) {
+        buffer.readData(&salt[0], saltlen);
+        rdata_len -= saltlen;
+    }
+
+    return (ParseNSEC3ParamResult(hashalg, flags, iterations));
+}
+
+} // end of nsec3
+} // end of detail
+} // end of generic
+} // end of rdata
+} // end of dns
+} // end of isc
+

+ 134 - 0
src/lib/dns/rdata/generic/detail/nsec3param_common.h

@@ -0,0 +1,134 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __NSEC3PARAM_COMMON_H
+#define __NSEC3PARAM_COMMON_H 1
+
+#include <util/buffer.h>
+
+#include <stdint.h>
+
+#include <sstream>
+#include <string>
+#include <vector>
+
+namespace isc {
+namespace dns {
+namespace rdata {
+namespace generic {
+namespace detail {
+namespace nsec3 {
+
+/// \file
+///
+/// This helper module provides some utilities that handle NSEC3 and
+/// NSEC3PARAM RDATA.  They share the first few fields, and some operations
+/// on these fields are sufficiently complicated, so it would make sense to
+/// consolidate the processing logic into a single implementation module.
+///
+/// The functions defined here are essentially private and are only expected
+/// to be called from the \c NSEC3 and \c NSEC3PARAM class implementations.
+
+/// \brief Result values of the utilities.
+///
+/// This structure encapsulates a tuple of NSEC3/NSEC3PARAM algorithm,
+/// flags and iterations field values.  This is used as the return value
+/// of the utility functions defined in this module so the caller can
+/// use it for constructing the corresponding RDATA.
+struct ParseNSEC3ParamResult {
+    ParseNSEC3ParamResult(uint8_t param_algorithm, uint8_t param_flags,
+                          uint16_t param_iterations) :
+        algorithm(param_algorithm), flags(param_flags),
+        iterations(param_iterations)
+    {}
+    const uint8_t algorithm;
+    const uint8_t flags;
+    const uint16_t iterations;
+};
+
+/// \brief Convert textual representation of NSEC3 parameters.
+///
+/// This function takes an input string stream that consists of a complete
+/// textual representation of an NSEC3 or NSEC3PARAM RDATA and parses it
+/// extracting the hash algorithm, flags, iterations, and salt fields.
+///
+/// The first three fields are returned as the return value of this function.
+/// The salt will be stored in the given vector.  The vector is expected
+/// to be empty, but if not, the existing content will be overridden.
+///
+/// On successful return the given input stream will reach the end of the
+/// salt field.
+///
+/// \exception isc::BadValue The salt is not a valid hex string.
+/// \exception InvalidRdataText The given string is otherwise invalid for
+/// NSEC3 or NSEC3PARAM fields.
+///
+/// \param rrtype_name Either "NSEC3" or "NSEC3PARAM"; used as part of
+/// exception messages.
+/// \param rdata_str A complete textual string of the RDATA; used as part of
+/// exception messages.
+/// \param iss Input stream that consists of a complete textual string of
+/// the RDATA.
+/// \param salt A placeholder for the salt field value of the RDATA.
+/// Expected to be empty, but it's not checked (and will be overridden).
+///
+/// \return The hash algorithm, flags, iterations in the form of
+/// ParseNSEC3ParamResult.
+ParseNSEC3ParamResult parseNSEC3ParamText(const char* const rrtype_name,
+                                          const std::string& rdata_str,
+                                          std::istringstream& iss,
+                                          std::vector<uint8_t>& salt);
+
+/// \brief Extract NSEC3 parameters from wire-format data.
+///
+/// This function takes an input buffer that stores wire-format NSEC3 or
+/// NSEC3PARAM RDATA and parses it extracting the hash algorithm, flags,
+/// iterations, and salt fields.
+///
+/// The first three fields are returned as the return value of this function.
+/// The salt will be stored in the given vector.  The vector is expected
+/// to be empty, but if not, the existing content will be overridden.
+///
+/// On successful return the input buffer will point to the end of the
+/// salt field; rdata_len will be the length of the rest of RDATA
+/// (in the case of a valid NSEC3PARAM, it should be 0).
+///
+/// \exception DNSMessageFORMERR The wire data is invalid.
+///
+/// \param rrtype_name Either "NSEC3" or "NSEC3PARAM"; used as part of
+/// exception messages.
+/// \param buffer An input buffer that stores wire-format RDATA.  It must
+/// point to the beginning of the data.
+/// \param rdata_len The total length of the RDATA.
+/// \param salt A placeholder for the salt field value of the RDATA.
+/// Expected to be empty, but it's not checked (and will be overridden).
+///
+/// \return The hash algorithm, flags, iterations in the form of
+/// ParseNSEC3ParamResult.
+ParseNSEC3ParamResult parseNSEC3ParamWire(const char* const rrtype_name,
+                                          isc::util::InputBuffer& buffer,
+                                          size_t& rdata_len,
+                                          std::vector<uint8_t>& salt);
+}
+}
+}
+}
+}
+}
+
+#endif  // __NSEC3PARAM_COMMON_H
+
+// Local Variables:
+// mode: c++
+// End:

+ 5 - 0
src/lib/dns/rdata/generic/detail/nsec_bitmap.h

@@ -12,6 +12,9 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 // PERFORMANCE OF THIS SOFTWARE.
 
 
+#ifndef __NSECBITMAP_H
+#define __NSECBITMAP_H 1
+
 #include <stdint.h>
 #include <stdint.h>
 
 
 #include <sstream>
 #include <sstream>
@@ -97,6 +100,8 @@ void bitmapsToText(const std::vector<uint8_t>& typebits,
 }
 }
 }
 }
 
 
+#endif  // __NSECBITMAP_H
+
 // Local Variables:
 // Local Variables:
 // mode: c++
 // mode: c++
 // End:
 // End:

+ 23 - 58
src/lib/dns/rdata/generic/nsec3_50.cc

@@ -17,6 +17,7 @@
 #include <string>
 #include <string>
 #include <sstream>
 #include <sstream>
 #include <vector>
 #include <vector>
+#include <cassert>
 
 
 #include <boost/lexical_cast.hpp>
 #include <boost/lexical_cast.hpp>
 
 
@@ -32,12 +33,14 @@
 #include <dns/rdata.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rdataclass.h>
 #include <dns/rdata/generic/detail/nsec_bitmap.h>
 #include <dns/rdata/generic/detail/nsec_bitmap.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
 
 
 #include <stdio.h>
 #include <stdio.h>
 #include <time.h>
 #include <time.h>
 
 
 using namespace std;
 using namespace std;
 using namespace isc::dns::rdata::generic::detail::nsec;
 using namespace isc::dns::rdata::generic::detail::nsec;
+using namespace isc::dns::rdata::generic::detail::nsec3;
 using namespace isc::util::encode;
 using namespace isc::util::encode;
 using namespace isc::util;
 using namespace isc::util;
 
 
@@ -65,42 +68,20 @@ NSEC3::NSEC3(const string& nsec3_str) :
     impl_(NULL)
     impl_(NULL)
 {
 {
     istringstream iss(nsec3_str);
     istringstream iss(nsec3_str);
-    unsigned int hashalg, flags, iterations;
+    vector<uint8_t> salt;
-    string iterations_str, salthex, nexthash;
+    const ParseNSEC3ParamResult params =
+        parseNSEC3ParamText("NSEC3", nsec3_str, iss, salt);
 
 
-    iss >> hashalg >> flags >> iterations_str >> salthex >> nexthash;
+    // Extract Next hash.  It must be an unpadded base32hex string.
+    string nexthash;
+    iss >> nexthash;
     if (iss.bad() || iss.fail()) {
     if (iss.bad() || iss.fail()) {
         isc_throw(InvalidRdataText, "Invalid NSEC3 text: " << nsec3_str);
         isc_throw(InvalidRdataText, "Invalid NSEC3 text: " << nsec3_str);
     }
     }
-    if (hashalg > 0xff) {
+    assert(!nexthash.empty());
-        isc_throw(InvalidRdataText,
+    if (*nexthash.rbegin() == '=') {
-                  "NSEC3 hash algorithm out of range: " << hashalg);
+        isc_throw(InvalidRdataText, "NSEC3 hash has padding: " << nsec3_str);
-    }
-    if (flags > 0xff) {
-        isc_throw(InvalidRdataText, "NSEC3 flags out of range: " << flags);
-    }
-    // Convert iteration.  To reject an invalid case where there's no space
-    // between iteration and salt, we extract this field as string and convert
-    // to integer.
-    try {
-        iterations = boost::lexical_cast<unsigned int>(iterations_str);
-    } catch (const boost::bad_lexical_cast&) {
-        isc_throw(InvalidRdataText, "Bad NSEC3 iteration: " << iterations_str);
     }
     }
-    if (iterations > 0xffff) {
-        isc_throw(InvalidRdataText, "NSEC3 iterations out of range: " <<
-            iterations);
-    }
-
-    vector<uint8_t> salt;
-    if (salthex != "-") {       // "-" means a 0-length salt
-        decodeHex(salthex, salt);
-    }
-    if (salt.size() > 255) {
-        isc_throw(InvalidRdataText, "NSEC3 salt is too long: "
-                  << salt.size() << " bytes");
-    }
-
     vector<uint8_t> next;
     vector<uint8_t> next;
     decodeBase32Hex(nexthash, next);
     decodeBase32Hex(nexthash, next);
     if (next.size() > 255) {
     if (next.size() > 255) {
@@ -110,7 +91,8 @@ NSEC3::NSEC3(const string& nsec3_str) :
 
 
     // For NSEC3 empty bitmap is possible and allowed.
     // For NSEC3 empty bitmap is possible and allowed.
     if (iss.eof()) {
     if (iss.eof()) {
-        impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next,
+        impl_ = new NSEC3Impl(params.algorithm, params.flags,
+                              params.iterations, salt, next,
                               vector<uint8_t>());
                               vector<uint8_t>());
         return;
         return;
     }
     }
@@ -118,36 +100,18 @@ NSEC3::NSEC3(const string& nsec3_str) :
     vector<uint8_t> typebits;
     vector<uint8_t> typebits;
     buildBitmapsFromText("NSEC3", iss, typebits);
     buildBitmapsFromText("NSEC3", iss, typebits);
 
 
-    impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
+    impl_ = new NSEC3Impl(params.algorithm, params.flags, params.iterations,
+                          salt, next, typebits);
 }
 }
 
 
 NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
 NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
-    // NSEC3 RR must have at least 5 octets:
+    vector<uint8_t> salt;
-    // hash algorithm(1), flags(1), iteration(2), saltlen(1)
+    const ParseNSEC3ParamResult params =
-    if (rdata_len < 5) {
+        parseNSEC3ParamWire("NSEC3", buffer, rdata_len, salt);
-        isc_throw(DNSMessageFORMERR, "NSEC3 too short, length: " << rdata_len);
-    }
-
-    const uint8_t hashalg = buffer.readUint8();
-    const uint8_t flags = buffer.readUint8();
-    const uint16_t iterations = buffer.readUint16();
-
-    const uint8_t saltlen = buffer.readUint8();
-    rdata_len -= 5;
-    if (rdata_len < saltlen) {
-        isc_throw(DNSMessageFORMERR, "NSEC3 salt length is too large: " <<
-                  static_cast<unsigned int>(saltlen));
-    }
-
-    vector<uint8_t> salt(saltlen);
-    if (saltlen > 0) {
-        buffer.readData(&salt[0], saltlen);
-        rdata_len -= saltlen;
-    }
 
 
     if (rdata_len < 1) {
     if (rdata_len < 1) {
         isc_throw(DNSMessageFORMERR, "NSEC3 too short to contain hash length, "
         isc_throw(DNSMessageFORMERR, "NSEC3 too short to contain hash length, "
-                  "length: " << rdata_len + saltlen + 5);
+                  "length: " << rdata_len + salt.size() + 5);
     }
     }
     const uint8_t nextlen = buffer.readUint8();
     const uint8_t nextlen = buffer.readUint8();
     --rdata_len;
     --rdata_len;
@@ -168,7 +132,8 @@ NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
         checkRRTypeBitmaps("NSEC3", typebits);
         checkRRTypeBitmaps("NSEC3", typebits);
     }
     }
 
 
-    impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
+    impl_ = new NSEC3Impl(params.algorithm, params.flags, params.iterations,
+                          salt, next, typebits);
 }
 }
 
 
 NSEC3::NSEC3(const NSEC3& source) :
 NSEC3::NSEC3(const NSEC3& source) :
@@ -256,10 +221,10 @@ compareVectors(const vector<uint8_t>& v1, const vector<uint8_t>& v2,
 {
 {
     const size_t len1 = v1.size();
     const size_t len1 = v1.size();
     const size_t len2 = v2.size();
     const size_t len2 = v2.size();
-    const size_t cmplen = min(len1, len2);
     if (check_length_first && len1 != len2) {
     if (check_length_first && len1 != len2) {
         return (len1 - len2);
         return (len1 - len2);
     }
     }
+    const size_t cmplen = min(len1, len2);
     const int cmp = cmplen == 0 ? 0 : memcmp(&v1.at(0), &v2.at(0), cmplen);
     const int cmp = cmplen == 0 ? 0 : memcmp(&v1.at(0), &v2.at(0), cmplen);
     if (cmp != 0) {
     if (cmp != 0) {
         return (cmp);
         return (cmp);

+ 49 - 70
src/lib/dns/rdata/generic/nsec3param_51.cc

@@ -12,22 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 // PERFORMANCE OF THIS SOFTWARE.
 
 
-#include <iostream>
-#include <string>
-#include <sstream>
-#include <vector>
-
-#include <boost/lexical_cast.hpp>
-
 #include <util/buffer.h>
 #include <util/buffer.h>
 #include <util/encode/hex.h>
 #include <util/encode/hex.h>
+
 #include <dns/messagerenderer.h>
 #include <dns/messagerenderer.h>
-#include <dns/name.h>
 #include <dns/rdata.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rdataclass.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
+
+#include <boost/lexical_cast.hpp>
 
 
-#include <stdio.h>
+#include <string>
-#include <time.h>
+#include <sstream>
+#include <vector>
 
 
 using namespace std;
 using namespace std;
 using namespace isc::util;
 using namespace isc::util;
@@ -43,9 +40,9 @@ struct NSEC3PARAMImpl {
         hashalg_(hashalg), flags_(flags), iterations_(iterations), salt_(salt)
         hashalg_(hashalg), flags_(flags), iterations_(iterations), salt_(salt)
     {}
     {}
 
 
-    uint8_t hashalg_;
+    const uint8_t hashalg_;
-    uint8_t flags_;
+    const uint8_t flags_;
-    uint16_t iterations_;
+    const uint16_t iterations_;
     const vector<uint8_t> salt_;
     const vector<uint8_t> salt_;
 };
 };
 
 
@@ -53,50 +50,26 @@ NSEC3PARAM::NSEC3PARAM(const string& nsec3param_str) :
     impl_(NULL)
     impl_(NULL)
 {
 {
     istringstream iss(nsec3param_str);
     istringstream iss(nsec3param_str);
-    uint16_t hashalg, flags, iterations;
-    stringbuf saltbuf;
-
-    iss >> hashalg >> flags >> iterations >> &saltbuf;
-    if (iss.bad() || iss.fail()) {
-        isc_throw(InvalidRdataText, "Invalid NSEC3PARAM text");
-    }
-    if (hashalg > 0xf) {
-        isc_throw(InvalidRdataText, "NSEC3PARAM hash algorithm out of range");
-    }
-    if (flags > 0xff) {
-        isc_throw(InvalidRdataText, "NSEC3PARAM flags out of range");
-    }
-
-    const string salt_str = saltbuf.str();
     vector<uint8_t> salt;
     vector<uint8_t> salt;
-    if (salt_str != "-") { // "-" means an empty salt, no need to touch vector
+    const ParseNSEC3ParamResult params =
-        decodeHex(saltbuf.str(), salt);
+        parseNSEC3ParamText("NSEC3PARAM", nsec3param_str, iss, salt);
+
+    if (!iss.eof()) {
+        isc_throw(InvalidRdataText, "Invalid NSEC3PARAM (redundant text): "
+                  << nsec3param_str);
     }
     }
 
 
-    impl_ = new NSEC3PARAMImpl(hashalg, flags, iterations, salt);
+    impl_ = new NSEC3PARAMImpl(params.algorithm, params.flags,
+                               params.iterations, salt);
 }
 }
 
 
 NSEC3PARAM::NSEC3PARAM(InputBuffer& buffer, size_t rdata_len) {
 NSEC3PARAM::NSEC3PARAM(InputBuffer& buffer, size_t rdata_len) {
-    if (rdata_len < 4) {
+    vector<uint8_t> salt;
-        isc_throw(InvalidRdataLength, "NSEC3PARAM too short");
+    const ParseNSEC3ParamResult params =
-    }
+        parseNSEC3ParamWire("NSEC3PARAM", buffer, rdata_len, salt);
-
-    uint8_t hashalg = buffer.readUint8();
-    uint8_t flags = buffer.readUint8();
-    uint16_t iterations = buffer.readUint16();
-    rdata_len -= 4;
-
-    uint8_t saltlen = buffer.readUint8();
-    --rdata_len;
-
-    if (rdata_len < saltlen) {
-        isc_throw(InvalidRdataLength, "NSEC3PARAM salt too short");
-    }
-
-    vector<uint8_t> salt(saltlen);
-    buffer.readData(&salt[0], saltlen);
 
 
-    impl_ = new NSEC3PARAMImpl(hashalg, flags, iterations, salt);
+    impl_ = new NSEC3PARAMImpl(params.algorithm, params.flags,
+                               params.iterations, salt);
 }
 }
 
 
 NSEC3PARAM::NSEC3PARAM(const NSEC3PARAM& source) :
 NSEC3PARAM::NSEC3PARAM(const NSEC3PARAM& source) :
@@ -124,27 +97,31 @@ string
 NSEC3PARAM::toText() const {
 NSEC3PARAM::toText() const {
     using namespace boost;
     using namespace boost;
     return (lexical_cast<string>(static_cast<int>(impl_->hashalg_)) +
     return (lexical_cast<string>(static_cast<int>(impl_->hashalg_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
+            " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
+            " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
-        " " + encodeHex(impl_->salt_));
+            " " + (impl_->salt_.empty() ? "-" : encodeHex(impl_->salt_)));
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireHelper(const NSEC3PARAMImpl& impl, OUTPUT_TYPE& output) {
+    output.writeUint8(impl.hashalg_);
+    output.writeUint8(impl.flags_);
+    output.writeUint16(impl.iterations_);
+    output.writeUint8(impl.salt_.size());
+    if (!impl.salt_.empty()) {
+        output.writeData(&impl.salt_[0], impl.salt_.size());
+    }
 }
 }
 
 
 void
 void
 NSEC3PARAM::toWire(OutputBuffer& buffer) const {
 NSEC3PARAM::toWire(OutputBuffer& buffer) const {
-    buffer.writeUint8(impl_->hashalg_);
+    toWireHelper(*impl_, buffer);
-    buffer.writeUint8(impl_->flags_);
-    buffer.writeUint16(impl_->iterations_);
-    buffer.writeUint8(impl_->salt_.size());
-    buffer.writeData(&impl_->salt_[0], impl_->salt_.size());
 }
 }
 
 
 void
 void
 NSEC3PARAM::toWire(AbstractMessageRenderer& renderer) const {
 NSEC3PARAM::toWire(AbstractMessageRenderer& renderer) const {
-    renderer.writeUint8(impl_->hashalg_);
+    toWireHelper(*impl_, renderer);
-    renderer.writeUint8(impl_->flags_);
-    renderer.writeUint16(impl_->iterations_);
-    renderer.writeUint8(impl_->salt_.size());
-    renderer.writeData(&impl_->salt_[0], impl_->salt_.size());
 }
 }
 
 
 int
 int
@@ -161,15 +138,18 @@ NSEC3PARAM::compare(const Rdata& other) const {
         return (impl_->iterations_ < other_param.impl_->iterations_ ? -1 : 1);
         return (impl_->iterations_ < other_param.impl_->iterations_ ? -1 : 1);
     }
     }
 
 
-    size_t this_len = impl_->salt_.size();
+    const size_t this_len = impl_->salt_.size();
-    size_t other_len = other_param.impl_->salt_.size();
+    const size_t other_len = other_param.impl_->salt_.size();
-    size_t cmplen = min(this_len, other_len);
+    if (this_len != other_len) {
-    int cmp = memcmp(&impl_->salt_[0], &other_param.impl_->salt_[0],
+        return (this_len - other_len);
-                     cmplen);
+    }
+    const size_t cmplen = min(this_len, other_len);
+    const int cmp = (cmplen == 0) ? 0 :
+        memcmp(&impl_->salt_.at(0), &other_param.impl_->salt_.at(0), cmplen);
     if (cmp != 0) {
     if (cmp != 0) {
         return (cmp);
         return (cmp);
     } else {
     } else {
-        return ((this_len == other_len) ? 0 : (this_len < other_len) ? -1 : 1);
+        return (this_len - other_len);
     }
     }
 }
 }
 
 
@@ -193,6 +173,5 @@ NSEC3PARAM::getSalt() const {
     return (impl_->salt_);
     return (impl_->salt_);
 }
 }
 
 
-
 // END_RDATA_NAMESPACE
 // END_RDATA_NAMESPACE
 // END_ISC_NAMESPACE
 // END_ISC_NAMESPACE

+ 1 - 0
src/lib/dns/tests/Makefile.am

@@ -43,6 +43,7 @@ run_unittests_SOURCES += rdata_nsec_unittest.cc
 run_unittests_SOURCES += rdata_nsec3_unittest.cc
 run_unittests_SOURCES += rdata_nsec3_unittest.cc
 run_unittests_SOURCES += rdata_nsecbitmap_unittest.cc
 run_unittests_SOURCES += rdata_nsecbitmap_unittest.cc
 run_unittests_SOURCES += rdata_nsec3param_unittest.cc
 run_unittests_SOURCES += rdata_nsec3param_unittest.cc
+run_unittests_SOURCES += rdata_nsec3param_like_unittest.cc
 run_unittests_SOURCES += rdata_rrsig_unittest.cc
 run_unittests_SOURCES += rdata_rrsig_unittest.cc
 run_unittests_SOURCES += rdata_rp_unittest.cc
 run_unittests_SOURCES += rdata_rp_unittest.cc
 run_unittests_SOURCES += rdata_srv_unittest.cc
 run_unittests_SOURCES += rdata_srv_unittest.cc

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

@@ -39,19 +39,19 @@ using namespace isc::util::encode;
 using namespace isc::dns::rdata;
 using namespace isc::dns::rdata;
 
 
 namespace {
 namespace {
+
+// Note: some tests can be shared with NSEC3PARAM.  They are unified as
+// typed tests defined in nsec3param_like_unittest.
 class Rdata_NSEC3_Test : public RdataTest {
 class Rdata_NSEC3_Test : public RdataTest {
     // there's nothing to specialize
     // there's nothing to specialize
 public:
 public:
     Rdata_NSEC3_Test() :
     Rdata_NSEC3_Test() :
         nsec3_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
         nsec3_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
                   "NS SOA RRSIG DNSKEY NSEC3PARAM"),
                   "NS SOA RRSIG DNSKEY NSEC3PARAM"),
-        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A" ),
+        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A" )
-        obuffer(0), renderer(obuffer)
     {}
     {}
     const string nsec3_txt;
     const string nsec3_txt;
     const string nsec3_nosalt_txt;
     const string nsec3_nosalt_txt;
-    OutputBuffer obuffer;
-    MessageRenderer renderer;
 };
 };
 
 
 TEST_F(Rdata_NSEC3_Test, fromText) {
 TEST_F(Rdata_NSEC3_Test, fromText) {
@@ -59,20 +59,6 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
     // text and construct nsec3_txt.  It will be tested against the wire format
     // text and construct nsec3_txt.  It will be tested against the wire format
     // representation in the createFromWire test.
     // representation in the createFromWire test.
 
 
-    // Numeric parameters have possible maximum values.  Unusual, but must
-    // be accepted.
-    EXPECT_NO_THROW(generic::NSEC3("255 255 65535 D399EAAB "
-                                   "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                   "NS SOA RRSIG DNSKEY NSEC3PARAM"));
-
-    // 0-length salt
-    EXPECT_EQ(0, generic::NSEC3(nsec3_nosalt_txt).getSalt().size());
-
-    // salt that has the possible max length
-    EXPECT_EQ(255, generic::NSEC3("1 1 1 " + string(255 * 2, '0') +
-                                  " H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                  "NS").getSalt().size());
-
     // hash that has the possible max length (see badText about the magic
     // hash that has the possible max length (see badText about the magic
     // numbers)
     // numbers)
     EXPECT_EQ(255, generic::NSEC3("1 1 1 D399EAAB " +
     EXPECT_EQ(255, generic::NSEC3("1 1 1 D399EAAB " +
@@ -84,47 +70,20 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
                         "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"));
                         "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"));
 }
 }
 
 
-TEST_F(Rdata_NSEC3_Test, toText) {
-    // normal case
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    EXPECT_EQ(nsec3_txt, rdata_nsec3.toText());
-
-    // empty salt case
-    EXPECT_EQ(nsec3_nosalt_txt, generic::NSEC3(nsec3_nosalt_txt).toText());
-}
-
 TEST_F(Rdata_NSEC3_Test, badText) {
 TEST_F(Rdata_NSEC3_Test, badText) {
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                 "BIFF POW SPOON"),
                                 "BIFF POW SPOON"),
                  InvalidRdataText);
                  InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEE "
-                                "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
-                 BadValue);     // bad hex
-    EXPECT_THROW(generic::NSEC3("1 1 1 -- H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                "A"),
-                 BadValue); // this shouldn't be confused a valid empty salt
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
                                 "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
                                 "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
                  BadValue);     // bad base32hex
                  BadValue);     // bad base32hex
-    EXPECT_THROW(generic::NSEC3("1000000 1 1 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3("1 1000000 1 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
-                 InvalidRdataText);
     EXPECT_THROW(generic::NSEC3("1 1 1000000 ADDAFEEE "
     EXPECT_THROW(generic::NSEC3("1 1 1000000 ADDAFEEE "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
                  InvalidRdataText);
                  InvalidRdataText);
 
 
-    // There should be a space between "1" and "D399EAAB" (salt)
+    // Next hash shouldn't be padded
-    EXPECT_THROW(generic::NSEC3(
+    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE CPNMU=== A NS SOA"),
-                     "1 1 1D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                     "NS SOA RRSIG DNSKEY NSEC3PARAM"), InvalidRdataText);
-
-    // Salt is too long (possible max + 1 bytes)
-    EXPECT_THROW(generic::NSEC3("1 1 1 " + string(256 * 2, '0') +
-                                " H9RSFB7FPF2L8HG35CMPC765TDK23RP6 NS"),
                  InvalidRdataText);
                  InvalidRdataText);
 
 
     // Hash is too long.  Max = 255 bytes, base32-hex converts each 5 bytes
     // Hash is too long.  Max = 255 bytes, base32-hex converts each 5 bytes
@@ -136,34 +95,12 @@ TEST_F(Rdata_NSEC3_Test, badText) {
 }
 }
 
 
 TEST_F(Rdata_NSEC3_Test, createFromWire) {
 TEST_F(Rdata_NSEC3_Test, createFromWire) {
-    // Normal case
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    EXPECT_EQ(0, rdata_nsec3.compare(
-                  *rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                        "rdata_nsec3_fromWire1")));
-
     // A valid NSEC3 RR with empty type bitmap.
     // A valid NSEC3 RR with empty type bitmap.
     EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
     EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                          "rdata_nsec3_fromWire15.wire"));
                                          "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"),
-                 DNSMessageFORMERR);
-
     // Invalid bitmap cases are tested in Rdata_NSECBITMAP_Test.
     // Invalid bitmap cases are tested in Rdata_NSECBITMAP_Test.
 
 
-    // salt length is too large
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire11.wire"),
-                 DNSMessageFORMERR);
-
-    // empty salt.  unusual, but valid.
-    ConstRdataPtr rdata =
-        rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                             "rdata_nsec3_fromWire13.wire");
-    EXPECT_EQ(0, dynamic_cast<const generic::NSEC3&>(*rdata).getSalt().size());
-
     // hash length is too large
     // hash length is too large
     EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
     EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                       "rdata_nsec3_fromWire12.wire"),
                                       "rdata_nsec3_fromWire12.wire"),
@@ -193,37 +130,6 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
     }
     }
 }
 }
 
 
-template <typename OUTPUT_TYPE>
-void
-toWireCheck(OUTPUT_TYPE& output, const char* const data_file) {
-    vector<uint8_t> data;
-    UnitTestUtil::readWireData(data_file, data);
-    InputBuffer buffer(&data[0], data.size());
-    const uint16_t rdlen = buffer.readUint16();
-    const generic::NSEC3 nsec3 =
-        dynamic_cast<const generic::NSEC3&>(*createRdata(
-                                                RRType::NSEC3(), RRClass::IN(),
-                                                buffer, rdlen));
-
-    output.clear();
-    output.writeUint16(rdlen);
-    nsec3.toWire(output);
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, output.getData(),
-                        output.getLength(), &data[0], data.size());
-}
-
-TEST_F(Rdata_NSEC3_Test, toWire) {
-    // normal case
-    toWireCheck(renderer, "rdata_nsec3_fromWire1");
-    toWireCheck(obuffer, "rdata_nsec3_fromWire1");
-
-    // empty salt
-    toWireCheck(renderer, "rdata_nsec3_fromWire13.wire");
-    toWireCheck(obuffer, "rdata_nsec3_fromWire13.wire");
-
-    // empty bitmap case is handled in the bitmap tests
-}
-
 TEST_F(Rdata_NSEC3_Test, assign) {
 TEST_F(Rdata_NSEC3_Test, assign) {
     generic::NSEC3 rdata_nsec3(nsec3_txt);
     generic::NSEC3 rdata_nsec3(nsec3_txt);
     generic::NSEC3 other_nsec3 = rdata_nsec3;
     generic::NSEC3 other_nsec3 = rdata_nsec3;
@@ -240,14 +146,8 @@ TEST_F(Rdata_NSEC3_Test, compare) {
 
 
     // test RDATAs, sorted in the ascendent order.  We only check comparison
     // test RDATAs, sorted in the ascendent order.  We only check comparison
     // on NSEC3-specific fields.  Bitmap comparison is tested in the bitmap
     // on NSEC3-specific fields.  Bitmap comparison is tested in the bitmap
-    // tests.
+    // tests.  Common cases for NSEC3 and NSECPARAM3 are in their shared tests.
     vector<generic::NSEC3> compare_set;
     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 D1K6GQ38"));
     compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ0000000000"));
     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 1 FF99EA0000 D1K6GQ00UUUUUUUU"));

+ 260 - 0
src/lib/dns/tests/rdata_nsec3param_like_unittest.cc

@@ -0,0 +1,260 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+
+#include <gtest/gtest.h>
+
+#include <dns/tests/unittest_util.h>
+#include <dns/tests/rdata_unittest.h>
+
+#include <string>
+#include <vector>
+
+using namespace std;
+using isc::UnitTestUtil;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace {
+
+// Template for shared tests for NSEC3 and NSEC3PARAM
+template <typename RDATA_TYPE>
+class NSEC3PARAMLikeTest : public RdataTest {
+protected:
+    NSEC3PARAMLikeTest() :
+        salt_txt("1 1 1 D399EAAB" + getCommonText()),
+        nosalt_txt("1 1 1 -" + getCommonText()),
+        obuffer(0), renderer(obuffer)
+    {}
+
+    RDATA_TYPE fromText(const string& rdata_text) {
+        return (RDATA_TYPE(rdata_text));
+    }
+
+    void compareCheck() const {
+        typename vector<RDATA_TYPE>::const_iterator it;
+        typename vector<RDATA_TYPE>::const_iterator const 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));
+        }
+    }
+
+    const string salt_txt;      // RDATA text with salt
+    const string nosalt_txt;    // RDATA text without salt
+    OutputBuffer obuffer;       // used in toWire() tests
+    MessageRenderer renderer;   // ditto
+    vector<RDATA_TYPE> compare_set; // used in compare() tests
+
+    // Convert generic Rdata to the corresponding derived Rdata class object.
+    // Defined here because it depends on the template parameter.
+    static const RDATA_TYPE& convert(const Rdata& rdata) {
+        return (dynamic_cast<const RDATA_TYPE&>(rdata));
+    }
+
+    // These depend on the specific RR type.  We use specialized methods
+    // for them.
+    static RRType getType(); // return either RRType::NSEC3() or NSEC3PARAM()
+    static string getWireFilePrefix();
+    static string getCommonText(); // commonly used part of textual form
+};
+
+// Instantiate specific typed tests
+typedef ::testing::Types<generic::NSEC3, generic::NSEC3PARAM> TestRdataTypes;
+TYPED_TEST_CASE(NSEC3PARAMLikeTest, TestRdataTypes);
+
+template <>
+RRType
+NSEC3PARAMLikeTest<generic::NSEC3>::getType() {
+    return (RRType::NSEC3());
+}
+
+template <>
+RRType
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getType() {
+    return (RRType::NSEC3PARAM());
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3>::getWireFilePrefix() {
+    return ("rdata_nsec3_");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getWireFilePrefix() {
+    return ("rdata_nsec3param_");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3>::getCommonText() {
+    // next hash + RR type bitmap
+    return (" H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
+            "NS SOA RRSIG DNSKEY NSEC3PARAM");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getCommonText() {
+    // there's no more text for NSEC3PARAM
+    return ("");
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, fromText) {
+    // Numeric parameters have possible maximum values.  Unusual, but must
+    // be accepted.
+    EXPECT_NO_THROW(this->fromText("255 255 65535 D399EAAB" +
+                                   this->getCommonText()));
+
+    // 0-length salt
+    EXPECT_EQ(0, this->fromText(this->nosalt_txt).getSalt().size());
+
+    // salt that has the possible max length
+    EXPECT_EQ(255, this->fromText("1 1 1 " + string(255 * 2, '0') +
+                                  this->getCommonText()).getSalt().size());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, badText) {
+    // Bad salt hex
+    EXPECT_THROW(this->fromText("1 1 1 SPORK0" + this->getCommonText()),
+                 isc::BadValue);
+    EXPECT_THROW(this->fromText("1 1 1 ADDAFEE" + this->getCommonText()),
+                 isc::BadValue);
+
+    // Space within salt
+    EXPECT_THROW(this->fromText("1 1 1 ADDAFE ADDAFEEE" +
+                                this->getCommonText()),
+                 InvalidRdataText);
+
+    // Similar to empty salt, but not really.  This shouldn't cause confusion.
+    EXPECT_THROW(this->fromText("1 1 1 --" + this->getCommonText()),
+                 isc::BadValue);
+
+    // Too large algorithm
+    EXPECT_THROW(this->fromText("1000000 1 1 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Too large flags
+    EXPECT_THROW(this->fromText("1 1000000 1 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Too large iterations
+    EXPECT_THROW(this->fromText("1 1 65536 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // There should be a space between "1" and "D399EAAB" (salt)
+    EXPECT_THROW(this->fromText("1 1 1D399EAAB" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Salt is too long (possible max + 1 bytes)
+    EXPECT_THROW(this->fromText("1 1 1 " + string(256 * 2, '0') +
+                                this->getCommonText()),
+                 InvalidRdataText);
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, toText) {
+    // normal case
+    EXPECT_EQ(this->salt_txt, this->fromText(this->salt_txt).toText());
+
+    // empty salt case
+    EXPECT_EQ(this->nosalt_txt, this->fromText(this->nosalt_txt).toText());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, createFromWire) {
+    // Normal case
+    EXPECT_EQ(0, this->fromText(this->salt_txt).compare(
+                  *this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                              (this->getWireFilePrefix() +
+                                               "fromWire1").c_str())));
+
+    // Too short RDLENGTH: it doesn't even contain the first 5 octets.
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire2.wire").c_str()),
+                 DNSMessageFORMERR);
+
+    // salt length is too large
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire11.wire").c_str()),
+                 DNSMessageFORMERR);
+
+    // empty salt.  not so usual, but valid.
+    ConstRdataPtr rdata =
+        this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                   (this->getWireFilePrefix() +
+                                    "fromWire13.wire").c_str());
+    EXPECT_EQ(0, this->convert(*rdata).getSalt().size());
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireCheck(RRType rrtype, OUTPUT_TYPE& output, const string& data_file) {
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData(data_file.c_str(), data);
+    InputBuffer buffer(&data[0], data.size());
+    const uint16_t rdlen = buffer.readUint16();
+
+    output.clear();
+    output.writeUint16(rdlen);
+    createRdata(rrtype, RRClass::IN(), buffer, rdlen)->toWire(output);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, output.getData(),
+                        output.getLength(), &data[0], data.size());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, toWire) {
+    // normal case
+    toWireCheck(this->getType(), this->renderer,
+                this->getWireFilePrefix() + "fromWire1");
+    toWireCheck(this->getType(), this->obuffer,
+                this->getWireFilePrefix() + "fromWire1");
+
+    // empty salt
+    toWireCheck(this->getType(), this->renderer,
+                this->getWireFilePrefix() + "fromWire13.wire");
+    toWireCheck(this->getType(), this->obuffer,
+                this->getWireFilePrefix() + "fromWire13.wire");
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, compare) {
+    // test RDATAs, sorted in the ascendent order.
+    this->compare_set.push_back(this->fromText("0 0 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 0 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 -" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 FF99EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 FF99EA0000" +
+                                               this->getCommonText()));
+
+    this->compareCheck();
+}
+
+}

+ 30 - 12
src/lib/dns/tests/rdata_nsec3param_unittest.cc

@@ -41,14 +41,14 @@ using namespace isc::dns::rdata;
 namespace {
 namespace {
 class Rdata_NSEC3PARAM_Test : public RdataTest {
 class Rdata_NSEC3PARAM_Test : public RdataTest {
 public:
 public:
-    Rdata_NSEC3PARAM_Test() : nsec3param_txt("1 0 1 D399EAAB") {}
+    Rdata_NSEC3PARAM_Test() : nsec3param_txt("1 1 1 D399EAAB") {}
     const string nsec3param_txt;
     const string nsec3param_txt;
 };
 };
 
 
 TEST_F(Rdata_NSEC3PARAM_Test, fromText) {
 TEST_F(Rdata_NSEC3PARAM_Test, fromText) {
     // With a salt
     // With a salt
     EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getHashalg());
     EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getHashalg());
-    EXPECT_EQ(0, generic::NSEC3PARAM(nsec3param_txt).getFlags());
+    EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getFlags());
     // (salt is checked in the toText test)
     // (salt is checked in the toText test)
 
 
     // With an empty salt
     // With an empty salt
@@ -61,16 +61,9 @@ TEST_F(Rdata_NSEC3PARAM_Test, toText) {
 }
 }
 
 
 TEST_F(Rdata_NSEC3PARAM_Test, badText) {
 TEST_F(Rdata_NSEC3PARAM_Test, badText) {
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 SPORK"), BadValue); // bad hex
+    // garbage space at the end
-    EXPECT_THROW(generic::NSEC3PARAM("100000 1 1 ADDAFEE"), InvalidRdataText);
+    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 D399EAAB "),
-    EXPECT_THROW(generic::NSEC3PARAM("1 100000 1 ADDAFEE"), InvalidRdataText);
+                 InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 100000 ADDAFEE"), InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3PARAM("1"), InvalidRdataText);
-}
-
-TEST_F(Rdata_NSEC3PARAM_Test, DISABLED_badText) {
-    // this currently fails
-    EXPECT_THROW(generic::NSEC3PARAM("1 0 1D399EAAB"), InvalidRdataText);
 }
 }
 
 
 TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
 TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
@@ -78,6 +71,19 @@ TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
     EXPECT_EQ(0, rdata_nsec3param.compare(
     EXPECT_EQ(0, rdata_nsec3param.compare(
                   *rdataFactoryFromFile(RRType::NSEC3PARAM(), RRClass::IN(),
                   *rdataFactoryFromFile(RRType::NSEC3PARAM(), RRClass::IN(),
                                        "rdata_nsec3param_fromWire1")));
                                        "rdata_nsec3param_fromWire1")));
+
+    // Short buffer cases.  The data is valid NSEC3PARAM RDATA, but the buffer
+    // is trimmed at the end.  All cases should result in an exception from
+    // the buffer class.
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData("rdata_nsec3param_fromWire1", data);
+    const uint16_t rdlen = (data.at(0) << 8) + data.at(1);
+    for (int i = 0; i < rdlen; ++i) {
+        // intentionally construct a short buffer
+        InputBuffer b(&data[0] + 2, i);
+        EXPECT_THROW(createRdata(RRType::NSEC3PARAM(), RRClass::IN(), b, 9),
+                     InvalidBufferPosition);
+    }
 }
 }
 
 
 TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) {
 TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) {
@@ -103,4 +109,16 @@ TEST_F(Rdata_NSEC3PARAM_Test, assign) {
     EXPECT_EQ(0, rdata_nsec3param.compare(other_nsec3param));
     EXPECT_EQ(0, rdata_nsec3param.compare(other_nsec3param));
 }
 }
 
 
+TEST_F(Rdata_NSEC3PARAM_Test, compare) {
+    // trivial case: self equivalence
+    EXPECT_EQ(0, generic::NSEC3PARAM(nsec3param_txt).
+              compare(generic::NSEC3PARAM(nsec3param_txt)));
+    EXPECT_EQ(0, generic::NSEC3PARAM("1 1 1 -").
+              compare(generic::NSEC3PARAM("1 1 1 -")));
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(generic::NSEC3PARAM(nsec3param_txt).compare(*rdata_nomatch),
+                 bad_cast);
+}
+
 }
 }

+ 6 - 0
src/lib/dns/tests/testdata/Makefile.am

@@ -30,6 +30,9 @@ BUILT_SOURCES += rdata_nsec3_fromWire10.wire rdata_nsec3_fromWire11.wire
 BUILT_SOURCES += rdata_nsec3_fromWire12.wire rdata_nsec3_fromWire13.wire
 BUILT_SOURCES += rdata_nsec3_fromWire12.wire rdata_nsec3_fromWire13.wire
 BUILT_SOURCES += rdata_nsec3_fromWire14.wire rdata_nsec3_fromWire15.wire
 BUILT_SOURCES += rdata_nsec3_fromWire14.wire rdata_nsec3_fromWire15.wire
 BUILT_SOURCES += rdata_nsec3_fromWire16.wire rdata_nsec3_fromWire17.wire
 BUILT_SOURCES += rdata_nsec3_fromWire16.wire rdata_nsec3_fromWire17.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire2.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire11.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire13.wire
 BUILT_SOURCES += rdata_rrsig_fromWire2.wire
 BUILT_SOURCES += rdata_rrsig_fromWire2.wire
 BUILT_SOURCES += rdata_minfo_fromWire1.wire rdata_minfo_fromWire2.wire
 BUILT_SOURCES += rdata_minfo_fromWire1.wire rdata_minfo_fromWire2.wire
 BUILT_SOURCES += rdata_minfo_fromWire3.wire rdata_minfo_fromWire4.wire
 BUILT_SOURCES += rdata_minfo_fromWire3.wire rdata_minfo_fromWire4.wire
@@ -104,6 +107,9 @@ EXTRA_DIST += rdata_nsec_fromWire8.spec rdata_nsec_fromWire9.spec
 EXTRA_DIST += rdata_nsec_fromWire10.spec
 EXTRA_DIST += rdata_nsec_fromWire10.spec
 EXTRA_DIST += rdata_nsec_fromWire16.spec
 EXTRA_DIST += rdata_nsec_fromWire16.spec
 EXTRA_DIST += rdata_nsec3param_fromWire1
 EXTRA_DIST += rdata_nsec3param_fromWire1
+EXTRA_DIST += rdata_nsec3param_fromWire2.spec
+EXTRA_DIST += rdata_nsec3param_fromWire11.spec
+EXTRA_DIST += rdata_nsec3param_fromWire13.spec
 EXTRA_DIST += rdata_nsec3_fromWire1
 EXTRA_DIST += rdata_nsec3_fromWire1
 EXTRA_DIST += rdata_nsec3_fromWire2.spec rdata_nsec3_fromWire3
 EXTRA_DIST += rdata_nsec3_fromWire2.spec rdata_nsec3_fromWire3
 EXTRA_DIST += rdata_nsec3_fromWire4.spec rdata_nsec3_fromWire5.spec
 EXTRA_DIST += rdata_nsec3_fromWire4.spec rdata_nsec3_fromWire5.spec

+ 2 - 2
src/lib/dns/tests/testdata/rdata_nsec3param_fromWire1

@@ -1,5 +1,5 @@
 # RDLENGTH, 9 bytes
 # RDLENGTH, 9 bytes
 00 09
 00 09
 # NSEC3PARAM record
 # NSEC3PARAM record
-# 1 0 1 D399EAAB
+# 1 1 1 D399EAAB
-01 00 00 01 04 d3 99 ea ab
+01 01 00 01 04 d3 99 ea ab

+ 8 - 0
src/lib/dns/tests/testdata/rdata_nsec3param_fromWire11.spec

@@ -0,0 +1,8 @@
+#
+# An invalid NSEC3PARAM RDATA: Saltlen is too large
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+rdlen: 7

+ 9 - 0
src/lib/dns/tests/testdata/rdata_nsec3param_fromWire13.spec

@@ -0,0 +1,9 @@
+#
+# A valid (but unusual) NSEC3PARAM RDATA: salt is empty.
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+saltlen: 0
+salt: ''

+ 9 - 0
src/lib/dns/tests/testdata/rdata_nsec3param_fromWire2.spec

@@ -0,0 +1,9 @@
+#
+# A malformed NSEC3PARAM RDATA: RDLEN indicates it doesn't even contain the
+# fixed 5 octects
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+rdlen: 4

+ 8 - 0
src/lib/python/isc/cc/data.py

@@ -21,6 +21,7 @@
 #
 #
 
 
 import json
 import json
+import re
 
 
 class DataNotFoundError(Exception):
 class DataNotFoundError(Exception):
     """Raised if an identifier does not exist according to a spec file,
     """Raised if an identifier does not exist according to a spec file,
@@ -86,6 +87,13 @@ def split_identifier(identifier):
     id_parts[:] = (value for value in id_parts if value != "")
     id_parts[:] = (value for value in id_parts if value != "")
     return id_parts
     return id_parts
 
 
+def identifier_has_list_index(identifier):
+    """Returns True if the given identifier string has at least one
+       list index (with [I], where I is a number"""
+    return (type(identifier) == str and
+            re.search("\[\d+\]", identifier) is not None)
+
+
 def split_identifier_list_indices(identifier):
 def split_identifier_list_indices(identifier):
     """Finds list indexes in the given identifier, which are of the
     """Finds list indexes in the given identifier, which are of the
        format [integer].
        format [integer].

+ 52 - 11
src/lib/python/isc/config/config_data.py

@@ -28,6 +28,22 @@ class ConfigDataError(Exception): pass
 
 
 BIND10_CONFIG_DATA_VERSION = 2
 BIND10_CONFIG_DATA_VERSION = 2
 
 
+# Helper functions
+def spec_part_is_list(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       list specification, and False otherwise."""
+    return (type(spec_part) == dict and 'list_item_spec' in spec_part)
+
+def spec_part_is_map(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       map specification, and False otherwise."""
+    return (type(spec_part) == dict and 'map_item_spec' in spec_part)
+
+def spec_part_is_named_set(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       named_set specification, and False otherwise."""
+    return (type(spec_part) == dict and 'named_map_item_spec' in spec_part)
+
 def check_type(spec_part, value):
 def check_type(spec_part, value):
     """Does nothing if the value is of the correct type given the
     """Does nothing if the value is of the correct type given the
        specification part relevant for the value. Raises an
        specification part relevant for the value. Raises an
@@ -112,9 +128,9 @@ def _get_map_or_list(spec_part):
     """Returns the list or map specification if this is a list or a
     """Returns the list or map specification if this is a list or a
        map specification part. If not, returns the given spec_part
        map specification part. If not, returns the given spec_part
        itself"""
        itself"""
-    if "map_item_spec" in spec_part:
+    if spec_part_is_map(spec_part):
         return spec_part["map_item_spec"]
         return spec_part["map_item_spec"]
-    elif "list_item_spec" in spec_part:
+    elif spec_part_is_list(spec_part):
         return spec_part["list_item_spec"]
         return spec_part["list_item_spec"]
     else:
     else:
         return spec_part
         return spec_part
@@ -134,13 +150,13 @@ def _find_spec_part_single(cur_spec, id_part):
     # list or a map, which is internally represented by a dict with
     # list or a map, which is internally represented by a dict with
     # an element 'map_item_spec', a dict with an element 'list_item_spec',
     # an element 'map_item_spec', a dict with an element 'list_item_spec',
     # or a list (when it is the 'main' config_data element of a module).
     # or a list (when it is the 'main' config_data element of a module).
-    if type(cur_spec) == dict and 'map_item_spec' in cur_spec.keys():
+    if spec_part_is_map(cur_spec):
         for cur_spec_item in cur_spec['map_item_spec']:
         for cur_spec_item in cur_spec['map_item_spec']:
             if cur_spec_item['item_name'] == id:
             if cur_spec_item['item_name'] == id:
                 return cur_spec_item
                 return cur_spec_item
         # not found
         # not found
         raise isc.cc.data.DataNotFoundError(id + " not found")
         raise isc.cc.data.DataNotFoundError(id + " not found")
-    elif type(cur_spec) == dict and 'list_item_spec' in cur_spec.keys():
+    elif spec_part_is_list(cur_spec):
         if cur_spec['item_name'] == id:
         if cur_spec['item_name'] == id:
             return cur_spec['list_item_spec']
             return cur_spec['list_item_spec']
         # not found
         # not found
@@ -156,9 +172,22 @@ def _find_spec_part_single(cur_spec, id_part):
     else:
     else:
         raise isc.cc.data.DataNotFoundError("Not a correct config specification")
         raise isc.cc.data.DataNotFoundError("Not a correct config specification")
 
 
-def find_spec_part(element, identifier):
+def find_spec_part(element, identifier, strict_identifier = True):
     """find the data definition for the given identifier
     """find the data definition for the given identifier
-       returns either a map with 'item_name' etc, or a list of those"""
+       returns either a map with 'item_name' etc, or a list of those
+       Parameters:
+       element: The specification element to start the search in
+       identifier: The element to find (relative to element above)
+       strict_identifier: If True (the default), additional checking occurs.
+                          Currently the only check is whether a list index is
+                          specified (except for the last part of the
+                          identifier)
+       Raises a DataNotFoundError if the data is not found, or if
+       strict_identifier is True and any non-final identifier parts
+       (i.e. before the last /) identify a list element and do not contain
+       an index.
+       Returns the spec element identified by the given identifier.
+    """
     if identifier == "":
     if identifier == "":
         return element
         return element
     id_parts = identifier.split("/")
     id_parts = identifier.split("/")
@@ -171,6 +200,10 @@ def find_spec_part(element, identifier):
     # always want the 'full' spec of the item
     # always want the 'full' spec of the item
     for id_part in id_parts[:-1]:
     for id_part in id_parts[:-1]:
         cur_el = _find_spec_part_single(cur_el, id_part)
         cur_el = _find_spec_part_single(cur_el, id_part)
+        if strict_identifier and spec_part_is_list(cur_el) and\
+           not isc.cc.data.identifier_has_list_index(id_part):
+            raise isc.cc.data.DataNotFoundError(id_part +
+                                                " is a list and needs an index")
         cur_el = _get_map_or_list(cur_el)
         cur_el = _get_map_or_list(cur_el)
 
 
     cur_el = _find_spec_part_single(cur_el, id_parts[-1])
     cur_el = _find_spec_part_single(cur_el, id_parts[-1])
@@ -184,12 +217,12 @@ def spec_name_list(spec, prefix="", recurse=False):
     if prefix != "" and not prefix.endswith("/"):
     if prefix != "" and not prefix.endswith("/"):
         prefix += "/"
         prefix += "/"
     if type(spec) == dict:
     if type(spec) == dict:
-        if 'map_item_spec' in spec:
+        if spec_part_is_map(spec):
             for map_el in spec['map_item_spec']:
             for map_el in spec['map_item_spec']:
                 name = map_el['item_name']
                 name = map_el['item_name']
                 if map_el['item_type'] == 'map':
                 if map_el['item_type'] == 'map':
                     name += "/"
                     name += "/"
-                if recurse and 'map_item_spec' in map_el:
+                if recurse and spec_part_is_map(map_el):
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                 else:
                 else:
                     result.append(prefix + name)
                     result.append(prefix + name)
@@ -244,7 +277,12 @@ class ConfigData:
     def get_default_value(self, identifier):
     def get_default_value(self, identifier):
         """Returns the default from the specification, or None if there
         """Returns the default from the specification, or None if there
            is no default"""
            is no default"""
-        spec = find_spec_part(self.specification.get_config_spec(), identifier)
+        # We are searching for the default value, so we can set
+        # strict_identifier to false (in fact, we need to; we may not know
+        # some list indices, or they may not exist, we are looking for
+        # a default value for a reason here).
+        spec = find_spec_part(self.specification.get_config_spec(),
+                              identifier, False)
         if spec and 'item_default' in spec:
         if spec and 'item_default' in spec:
             return spec['item_default']
             return spec['item_default']
         else:
         else:
@@ -607,7 +645,7 @@ class MultiConfigData:
            Throws DataNotFoundError if the identifier is bad
            Throws DataNotFoundError if the identifier is bad
         """
         """
         result = []
         result = []
-        if not identifier:
+        if not identifier or identifier == "/":
             # No identifier, so we need the list of current modules
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
             for module in self._specifications.keys():
                 if all:
                 if all:
@@ -619,8 +657,11 @@ class MultiConfigData:
                     entry = _create_value_map_entry(module, 'module', None)
                     entry = _create_value_map_entry(module, 'module', None)
                     result.append(entry)
                     result.append(entry)
         else:
         else:
-            if identifier[0] == '/':
+            # Strip off start and end slashes, if they are there
+            if len(identifier) > 0 and identifier[0] == '/':
                 identifier = identifier[1:]
                 identifier = identifier[1:]
+            if len(identifier) > 0 and identifier[-1] == '/':
+                identifier = identifier[:-1]
             module, sep, id = identifier.partition('/')
             module, sep, id = identifier.partition('/')
             spec = self.get_module_spec(module)
             spec = self.get_module_spec(module)
             if spec:
             if spec:

+ 53 - 2
src/lib/python/isc/config/tests/config_data_test.py

@@ -185,6 +185,43 @@ class TestConfigData(unittest.TestCase):
         spec_part = find_spec_part(config_spec, "item6/value1")
         spec_part = find_spec_part(config_spec, "item6/value1")
         self.assertEqual({'item_name': 'value1', 'item_type': 'string', 'item_optional': True, 'item_default': 'default'}, spec_part)
         self.assertEqual({'item_name': 'value1', 'item_type': 'string', 'item_optional': True, 'item_default': 'default'}, spec_part)
 
 
+    def test_find_spec_part_lists(self):
+        # A few specific tests for list data
+        module_spec = isc.config.module_spec_from_file(self.data_path +
+                                                       os.sep +
+                                                       "spec31.spec")
+        config_spec = module_spec.get_config_spec()
+
+        expected_spec_part = {'item_name': 'number',
+                              'item_type': 'integer',
+                              'item_default': 1,
+                              'item_optional': False}
+
+        # First a check for a correct fetch
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2[1]")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # Leaving out an index should fail by default
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          find_spec_part, config_spec,
+                          "/first_list_items[0]/second_list_items/"
+                          "map_element/list1[1]/list2[1]")
+
+        # But not for the last element
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # And also not if strict_identifier is false (third argument)
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items/"
+                                   "map_element/list1[1]/list2[1]", False)
+        self.assertEqual(expected_spec_part, spec_part)
+
+
     def test_spec_name_list(self):
     def test_spec_name_list(self):
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
         self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
         self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
@@ -483,15 +520,25 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(MultiConfigData.DEFAULT, status)
         self.assertEqual(MultiConfigData.DEFAULT, status)
 
 
 
 
-
     def test_get_value_maps(self):
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
         self.assertEqual([], maps)
         
         
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_specification(module_spec)
+
+        expected = [{'default': False,
+                     'type': 'module',
+                     'name': 'Spec1',
+                     'value': None,
+                     'modified': False}]
+
         maps = self.mcd.get_value_maps()
         maps = self.mcd.get_value_maps()
-        self.assertEqual([{'default': False, 'type': 'module', 'name': 'Spec1', 'value': None, 'modified': False}], maps)
+        self.assertEqual(expected, maps)
+
+        maps = self.mcd.get_value_maps("/")
+        self.assertEqual(expected, maps)
+
         maps = self.mcd.get_value_maps('Spec2')
         maps = self.mcd.get_value_maps('Spec2')
         self.assertEqual([], maps)
         self.assertEqual([], maps)
         maps = self.mcd.get_value_maps('Spec1')
         maps = self.mcd.get_value_maps('Spec1')
@@ -566,6 +613,10 @@ class TestMultiConfigData(unittest.TestCase):
         maps = self.mcd.get_value_maps("/Spec22/value9")
         maps = self.mcd.get_value_maps("/Spec22/value9")
         self.assertEqual(expected, maps)
         self.assertEqual(expected, maps)
 
 
+        # A slash at the end should not produce different output
+        maps = self.mcd.get_value_maps("/Spec22/value9/")
+        self.assertEqual(expected, maps)
+
     def test_get_value_maps_named_set(self):
     def test_get_value_maps_named_set(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_specification(module_spec)

+ 35 - 13
src/lib/util/python/gen_wiredata.py.in

@@ -949,12 +949,11 @@ class NSEC(NSECBASE):
                                                  int(len(name_wire) / 2)))
                                                  int(len(name_wire) / 2)))
         f.write('%s\n' % name_wire)
         f.write('%s\n' % name_wire)
 
 
-class NSEC3(NSECBASE):
+class NSEC3PARAM(RR):
-    '''Implements rendering NSEC3 RDATA in the test data format.
+    '''Implements rendering NSEC3PARAM RDATA in the test data format.
 
 
     Configurable parameters are as follows (see the description of the
     Configurable parameters are as follows (see the description of the
     same name of attribute for the default value):
     same name of attribute for the default value):
-    - Type bitmap related parameters: see class NSECBASE
     - hashalg (8-bit int): The Hash Algorithm field.  Note that
     - hashalg (8-bit int): The Hash Algorithm field.  Note that
       currently the only defined algorithm is SHA-1, for which a value
       currently the only defined algorithm is SHA-1, for which a value
       of 1 will be used, and it's the default.  So this implementation
       of 1 will be used, and it's the default.  So this implementation
@@ -967,9 +966,6 @@ class NSEC3(NSECBASE):
     - saltlen (int): The Salt Length field.
     - saltlen (int): The Salt Length field.
     - salt (string): The Salt field.  It is converted to a sequence of
     - salt (string): The Salt field.  It is converted to a sequence of
       ascii codes and its hexadecimal representation will be used.
       ascii codes and its hexadecimal representation will be used.
-    - hashlen (int): The Hash Length field.
-    - hash (string): The Next Hashed Owner Name field.  This parameter
-      is interpreted as "salt".
     '''
     '''
 
 
     hashalg = 1                 # SHA-1
     hashalg = 1                 # SHA-1
@@ -978,15 +974,18 @@ class NSEC3(NSECBASE):
     iterations = 1
     iterations = 1
     saltlen = 5
     saltlen = 5
     salt = 's' * saltlen
     salt = 's' * saltlen
-    hashlen = 20
+
-    hash = 'h' * hashlen
+    def dump(self, f):
-    def dump_fixedpart(self, f, bitmap_totallen):
         if self.rdlen is None:
         if self.rdlen is None:
-            # if rdlen needs to be calculated, it must be based on the bitmap
+            self.rdlen = 4 + 1 + len(self.salt)
-            # length, because the configured maplen can be fake.
-            self.rdlen = 4 + 1 + len(self.salt) + 1 + len(self.hash) \
-                + bitmap_totallen
         self.dump_header(f, self.rdlen)
         self.dump_header(f, self.rdlen)
+        self._dump_params(f)
+
+    def _dump_params(self, f):
+        '''This method is intended to be shared with NSEC3 class.
+
+        '''
+
         optout_val = 1 if self.optout else 0
         optout_val = 1 if self.optout else 0
         f.write('# Hash Alg=%s, Opt-Out=%d, Other Flags=%0x, Iterations=%d\n' %
         f.write('# Hash Alg=%s, Opt-Out=%d, Other Flags=%0x, Iterations=%d\n' %
                 (code_totext(self.hashalg, rdict_nsec3_algorithm),
                 (code_totext(self.hashalg, rdict_nsec3_algorithm),
@@ -997,6 +996,29 @@ class NSEC3(NSECBASE):
         f.write('%02x%s%s\n' % (self.saltlen,
         f.write('%02x%s%s\n' % (self.saltlen,
                                 ' ' if len(self.salt) > 0 else '',
                                 ' ' if len(self.salt) > 0 else '',
                                 encode_string(self.salt)))
                                 encode_string(self.salt)))
+
+class NSEC3(NSECBASE, NSEC3PARAM):
+    '''Implements rendering NSEC3 RDATA in the test data format.
+
+    Configurable parameters are as follows (see the description of the
+    same name of attribute for the default value):
+    - Type bitmap related parameters: see class NSECBASE
+    - Hash parameter related parameters: see class NSEC3PARAM
+    - hashlen (int): The Hash Length field.
+    - hash (string): The Next Hashed Owner Name field.  This parameter
+      is interpreted as "salt".
+    '''
+
+    hashlen = 20
+    hash = 'h' * hashlen
+    def dump_fixedpart(self, f, bitmap_totallen):
+        if self.rdlen is None:
+            # if rdlen needs to be calculated, it must be based on the bitmap
+            # length, because the configured maplen can be fake.
+            self.rdlen = 4 + 1 + len(self.salt) + 1 + len(self.hash) \
+                + bitmap_totallen
+        self.dump_header(f, self.rdlen)
+        self._dump_params(f)
         f.write("# Hash Len=%d, Hash='%s'\n" % (self.hashlen, self.hash))
         f.write("# Hash Len=%d, Hash='%s'\n" % (self.hashlen, self.hash))
         f.write('%02x%s%s\n' % (self.hashlen,
         f.write('%02x%s%s\n' % (self.hashlen,
                                 ' ' if len(self.hash) > 0 else '',
                                 ' ' if len(self.hash) > 0 else '',