BIND 10 master, updated. 937635d75665f331226774ca1b7c5bff791b9d69 Merge branch 'trac2387'

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Apr 10 08:39:05 UTC 2013


The branch, master has been updated
       via  937635d75665f331226774ca1b7c5bff791b9d69 (commit)
       via  1b9e3430ec2b9a378107877e5470bf567ce17601 (commit)
       via  3c704fca950603ec97ad259462500fe6fbd549c9 (commit)
       via  f8205314f85fa56bd7a8b783764c9e4ef050ad6c (commit)
       via  9a045959025fb0961c7bf2eb09603f7604d5cbee (commit)
       via  82fc64bf97eb7d50e22edcac28cefe0b93b9a7ff (commit)
       via  e8b5540d1dbc1e80eb98d15e1dab3b9c26fe7ccb (commit)
       via  3f9a24a0ddf821e8e16b2c99d2e7335f2c40fa09 (commit)
       via  65f68f7b9235307f8af029b37ebc7c2a309ed7e9 (commit)
       via  0e61367a4241444a7465b25ee461adb7e829acc2 (commit)
       via  785ade580637cc0d78139edbda05ed13e1675dea (commit)
       via  85655172ba34a2d3c44731880f0e439d8933d277 (commit)
       via  49316890b86159c3ed72ef0913c5a58d4e8e5657 (commit)
       via  2b431168cf2961d0a04b07cfb2928b3778057d6c (commit)
       via  dfd5516cd85bfda7bab56d161082d2ce74330e2b (commit)
       via  4778557e13c231f1bd2cde5d0950b68de920d8cd (commit)
       via  7b2ee20fb35a475916a817c30668d31d15fc8907 (commit)
       via  61dcaac0cfc5b764e0d5ca144128b1aa7d5a311f (commit)
       via  281c6ab13bf40a06055a64b3e5f69c0406d33db0 (commit)
       via  8b106d20e3afa459ef9ca13401145395d8a6dcc3 (commit)
       via  8117b69a1c1c8c569032951254e5a094763d74e9 (commit)
       via  ea8936112837ab22263edabe5c44f1f40fa0ef6a (commit)
       via  e42c4c83835acbb17075759f9dd206dcff4a8d37 (commit)
       via  8e3eb1576a4f04736aeb7d44d9b13d19729a7062 (commit)
       via  edd462bc9e8f727b1ebdf14b3f7cb6c844dee35a (commit)
       via  29ec8dce0f9312be9cddb3712fbd107fb62e33d3 (commit)
       via  c70bba06b9157645d6493b16ef0ee414277ce7b1 (commit)
       via  ad6d16107655d744062af1e0570abc2023ee1ee2 (commit)
       via  b5d7336d2353cd552c2b253a358f612b54b56e54 (commit)
       via  aaace1a1f1579d5cb2a657c11115e601ef1d4b7e (commit)
       via  a852fc05305f5c062ecf9d3c9ff6a5b836370d6d (commit)
       via  53e18a35628a7a595d0de637ae4a045e41fe6b64 (commit)
       via  a1ce8481248f5859f8319ae85d9e25eaa0ace2db (commit)
       via  393534ac4698e6dfeee32ab4bdf99e25eade4a76 (commit)
       via  98b10b7662c77074dc26b27a4db1023b18821942 (commit)
       via  ed58ec0a82ec0b9df0e0f9bd53f1a32123c9fff6 (commit)
       via  0ecc1e01cde7133bfcb183ced46b011b4eb9558a (commit)
       via  09fff7c3f6d538b80e8a1760673bc4a3ac9f62d2 (commit)
       via  2045020b58739c4f1ba8c6d39ca118513f8b2e35 (commit)
       via  945103ccf34fe1140877c1dda05a98eb4ee79d98 (commit)
       via  6fc763dca0e5c617de20cf49353364219987db63 (commit)
       via  bbed8c3d28fb5a8cecd3c6d2682a8fa4b01c6566 (commit)
       via  55fe56a1ce1c5bb0c359942f37d912a18041a2ba (commit)
       via  23eca85ebf0a8a6f749984504cba553e2c4d38e8 (commit)
       via  09ed404f9b47807c74e0c6e59077e95a7b28b953 (commit)
       via  46cfb5cccadfaaea4568d8478c3ba349c9b877dd (commit)
       via  b6bd57985bbd40a4cd76e0b37c3e5ae7d3563051 (commit)
       via  8ee676bcecbdc959c24fce66ae32ddf94fe999af (commit)
       via  0318fc199d2d653198294eed8bbfd4ee69b340de (commit)
       via  d124a5b6f1fb3fb55918e3b75343c6bb70023980 (commit)
       via  72922ff97062fbb154821827932d8a8952c371fc (commit)
       via  aa95c10d7631603afd110b43cc6f12da22fda7f0 (commit)
       via  4e379f979dede89254759e40974d9c04d90ed451 (commit)
       via  fb7adf34e96efc9c2e0986ab3f567c6d38500a89 (commit)
       via  0c8f4051f57e4e1bcc29436bf50a1f7d4ff9e79d (commit)
       via  926dd635c3bca2100bd6a9975e2368183623c8ac (commit)
       via  863be53af8c369067f33c4b5688b44ebb380f789 (commit)
       via  86f31f3156e151a9446396aa04105c8dbfb6b592 (commit)
       via  f909b685377ab7c3d16f5903ccc54d0465721046 (commit)
       via  9926f9b8f9f9ed697975752c62a46e34bf45e3ca (commit)
       via  947d85d0e851a6fee3c34c734e3bd21065b57ca4 (commit)
       via  61068e227821fa32305d4edd142b2dcc6ca74eb4 (commit)
       via  2f41159cd685809c1dea5f079e99c5ecb90e1893 (commit)
       via  8a91e6ec6c257a9e630e0408ac1a5f464d769a55 (commit)
       via  240e5333012a0a60e9b42f8741c44450eae22ea2 (commit)
       via  22362bbf67afbd7851f731f6bbd337333267a939 (commit)
       via  fdfe820d25fc0ac6b8258a448fd9188d4e1f7090 (commit)
       via  167b544c41361a7cf316b042baf057afb9002d3b (commit)
       via  00d53a32412c50c20c573f40d895e8ad7f74ad4f (commit)
       via  8674224a8d108e92075436de2656e8ba912f7df6 (commit)
       via  3de4ecaf232d8a75e9da6e54384da948807a0659 (commit)
       via  f43b44eea132aa6ed092a787d0933d5b119a4025 (commit)
       via  7c41946c022b31c44c9e8b76b4c511c195c6743a (commit)
      from  0a9abd16730fa7708967c4b17dd1a36b7c75f363 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 937635d75665f331226774ca1b7c5bff791b9d69
Merge: 0a9abd1 1b9e343
Author: Mukund Sivaraman <muks at isc.org>
Date:   Wed Apr 10 13:28:57 2013 +0530

    Merge branch 'trac2387'

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/gen-rdatacode.py.in                    |    3 +
 .../dns/rdata/generic/detail/nsec3param_common.cc  |   36 ++--
 .../dns/rdata/generic/detail/nsec3param_common.h   |   25 ++-
 src/lib/dns/rdata/generic/detail/nsec_bitmap.cc    |   66 ++-----
 src/lib/dns/rdata/generic/detail/nsec_bitmap.h     |   32 +---
 src/lib/dns/rdata/generic/dnskey_48.cc             |  187 ++++++++++++++++----
 src/lib/dns/rdata/generic/dnskey_48.h              |    9 +
 src/lib/dns/rdata/generic/mx_15.cc                 |   17 +-
 src/lib/dns/rdata/generic/mx_15.h                  |    3 +
 src/lib/dns/rdata/generic/nsec3_50.cc              |  104 ++++++++---
 src/lib/dns/rdata/generic/nsec3_50.h               |    3 +
 src/lib/dns/rdata/generic/nsec3param_51.cc         |   81 +++++++--
 src/lib/dns/rdata/generic/nsec3param_51.h          |    4 +
 src/lib/dns/rdata/generic/nsec_47.cc               |    6 +-
 src/lib/dns/tests/masterload_unittest.cc           |   24 ++-
 src/lib/dns/tests/rdata_dnskey_unittest.cc         |  166 ++++++++++++-----
 src/lib/dns/tests/rdata_nsec3_unittest.cc          |  140 ++++++++++-----
 src/lib/dns/tests/rdata_nsec3param_unittest.cc     |  129 +++++++++++---
 src/lib/dns/tests/testdata/.gitignore              |    2 +
 src/lib/dns/tests/testdata/Makefile.am             |    4 +-
 .../rdata_dnskey_empty_keydata_fromWire.spec       |    7 +
 src/lib/dns/tests/testdata/rdata_dnskey_fromWire   |   24 ---
 .../dns/tests/testdata/rdata_dnskey_fromWire.spec  |    7 +
 src/lib/util/python/gen_wiredata.py.in             |   43 ++++-
 24 files changed, 787 insertions(+), 335 deletions(-)
 create mode 100644 src/lib/dns/tests/testdata/rdata_dnskey_empty_keydata_fromWire.spec
 delete mode 100644 src/lib/dns/tests/testdata/rdata_dnskey_fromWire
 create mode 100644 src/lib/dns/tests/testdata/rdata_dnskey_fromWire.spec

-----------------------------------------------------------------------
diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in
index 0d946b9..e265d56 100755
--- a/src/lib/dns/gen-rdatacode.py.in
+++ b/src/lib/dns/gen-rdatacode.py.in
@@ -37,12 +37,15 @@ new_rdata_factory_users = [('a', 'in'), ('aaaa', 'in'),
                            ('cname', 'generic'),
                            ('dlv', 'generic'),
                            ('dname', 'generic'),
+                           ('dnskey', 'generic'),
                            ('ds', 'generic'),
                            ('hinfo', 'generic'),
                            ('naptr', 'generic'),
                            ('mx', 'generic'),
                            ('ns', 'generic'),
                            ('nsec', 'generic'),
+                           ('nsec3', 'generic'),
+                           ('nsec3param', 'generic'),
                            ('ptr', 'generic'),
                            ('soa', 'generic'),
                            ('spf', 'generic'),
diff --git a/src/lib/dns/rdata/generic/detail/nsec3param_common.cc b/src/lib/dns/rdata/generic/detail/nsec3param_common.cc
index 6eea2c7..caa92ce 100644
--- a/src/lib/dns/rdata/generic/detail/nsec3param_common.cc
+++ b/src/lib/dns/rdata/generic/detail/nsec3param_common.cc
@@ -39,41 +39,33 @@ namespace detail {
 namespace nsec3 {
 
 ParseNSEC3ParamResult
-parseNSEC3ParamText(const char* const rrtype_name,
-                    const string& rdata_str, istringstream& iss,
-                    vector<uint8_t>& salt)
+parseNSEC3ParamFromLexer(const char* const rrtype_name,
+                         MasterLexer& lexer, 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);
-    }
+    const uint32_t hashalg =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (hashalg > 0xff) {
         isc_throw(InvalidRdataText, rrtype_name <<
                   " hash algorithm out of range: " << hashalg);
     }
+
+    const uint32_t flags =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     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);
-    }
+
+    const uint32_t iterations =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (iterations > 0xffff) {
         isc_throw(InvalidRdataText, rrtype_name <<
-                  " iterations out of range: " <<
-            iterations);
+                  " iterations out of range: " << iterations);
     }
 
+    const string salthex =
+        lexer.getNextToken(MasterToken::STRING).getString();
+
     // 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.
diff --git a/src/lib/dns/rdata/generic/detail/nsec3param_common.h b/src/lib/dns/rdata/generic/detail/nsec3param_common.h
index 1891fae..a8b848d 100644
--- a/src/lib/dns/rdata/generic/detail/nsec3param_common.h
+++ b/src/lib/dns/rdata/generic/detail/nsec3param_common.h
@@ -15,12 +15,11 @@
 #ifndef NSEC3PARAM_COMMON_H
 #define NSEC3PARAM_COMMON_H 1
 
+#include <dns/master_lexer.h>
+
 #include <util/buffer.h>
 
 #include <stdint.h>
-
-#include <sstream>
-#include <string>
 #include <vector>
 
 namespace isc {
@@ -59,7 +58,7 @@ struct ParseNSEC3ParamResult {
 
 /// \brief Convert textual representation of NSEC3 parameters.
 ///
-/// This function takes an input string stream that consists of a complete
+/// This function takes an input MasterLexer that points at a complete
 /// textual representation of an NSEC3 or NSEC3PARAM RDATA and parses it
 /// extracting the hash algorithm, flags, iterations, and salt fields.
 ///
@@ -67,28 +66,26 @@ struct ParseNSEC3ParamResult {
 /// 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
+/// On successful return the given MasterLexer 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
+/// \exception InvalidRdataText The given RDATA is otherwise invalid for
 /// NSEC3 or NSEC3PARAM fields.
+/// \exception MasterLexer::LexerError There was a syntax error reading
+/// a field from the MasterLexer.
 ///
 /// \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 lexer The MasterLexer to read NSEC3 parameter fields from.
 /// \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);
+ParseNSEC3ParamResult parseNSEC3ParamFromLexer(const char* const rrtype_name,
+                                               isc::dns::MasterLexer& lexer,
+                                               std::vector<uint8_t>& salt);
 
 /// \brief Extract NSEC3 parameters from wire-format data.
 ///
diff --git a/src/lib/dns/rdata/generic/detail/nsec_bitmap.cc b/src/lib/dns/rdata/generic/detail/nsec_bitmap.cc
index 319056e..5deaa69 100644
--- a/src/lib/dns/rdata/generic/detail/nsec_bitmap.cc
+++ b/src/lib/dns/rdata/generic/detail/nsec_bitmap.cc
@@ -78,68 +78,28 @@ checkRRTypeBitmaps(const char* const rrtype_name,
 }
 
 void
-buildBitmapsFromText(const char* const rrtype_name,
-                     istringstream& iss, vector<uint8_t>& typebits)
-{
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    memset(bitmap, 0, sizeof(bitmap));
-
-    do {
-        string type;
-        iss >> type;
-        if (iss.bad() || iss.fail()) {
-            isc_throw(InvalidRdataText, "Unexpected input for "
-                      << rrtype_name << " bitmap");
-        }
-        try {
-            const int code = RRType(type).getCode();
-            bitmap[code / 8] |= (0x80 >> (code % 8));
-        } catch (const InvalidRRType&) {
-            isc_throw(InvalidRdataText, "Invalid RRtype in "
-                      << rrtype_name << " bitmap: " << type);
-        }
-    } while (!iss.eof());
-
-    for (int window = 0; window < 256; ++window) {
-        int octet;
-        for (octet = 31; octet >= 0; octet--) {
-            if (bitmap[window * 32 + octet] != 0) {
-                break;
-            }
-        }
-        if (octet < 0) {
-            continue;
-        }
-        typebits.push_back(window);
-        typebits.push_back(octet + 1);
-        for (int i = 0; i <= octet; ++i) {
-            typebits.push_back(bitmap[window * 32 + i]);
-        }
-    }
-}
-
-// Note: this function shares common code with buildBitmapsFromText()
-// above, but it is expected that buildBitmapsFromText() will be deleted
-// entirely once the transition to MasterLexer is done for all dependent
-// RR types. So a common method is not made from the two.
-void
 buildBitmapsFromLexer(const char* const rrtype_name,
-                      MasterLexer& lexer, vector<uint8_t>& typebits)
+                      MasterLexer& lexer, vector<uint8_t>& typebits,
+                      bool allow_empty)
 {
     uint8_t bitmap[8 * 1024];       // 64k bits
     memset(bitmap, 0, sizeof(bitmap));
 
     bool have_rrtypes = false;
+    std::string type_str;
     while (true) {
-        const MasterToken& token = lexer.getNextToken();
-        if (token.getType() != MasterToken::STRING) {
+        const MasterToken& token =
+            lexer.getNextToken(MasterToken::STRING, true);
+        if ((token.getType() == MasterToken::END_OF_FILE) ||
+            (token.getType() == MasterToken::END_OF_LINE)) {
             break;
         }
 
+        // token is now assured to be of type STRING.
+
         have_rrtypes = true;
-        std::string type_str;
+        token.getString(type_str);
         try {
-            type_str = token.getString();
             const int code = RRType(type_str).getCode();
             bitmap[code / 8] |= (0x80 >> (code % 8));
         } catch (const InvalidRRType&) {
@@ -151,8 +111,12 @@ buildBitmapsFromLexer(const char* const rrtype_name,
     lexer.ungetToken();
 
     if (!have_rrtypes) {
+         if (allow_empty) {
+              return;
+         }
          isc_throw(InvalidRdataText,
-                   rrtype_name << " record does not end with RR type mnemonic");
+                   rrtype_name <<
+                   " record does not end with RR type mnemonic");
     }
 
     for (int window = 0; window < 256; ++window) {
diff --git a/src/lib/dns/rdata/generic/detail/nsec_bitmap.h b/src/lib/dns/rdata/generic/detail/nsec_bitmap.h
index ea5cad0..5525384 100644
--- a/src/lib/dns/rdata/generic/detail/nsec_bitmap.h
+++ b/src/lib/dns/rdata/generic/detail/nsec_bitmap.h
@@ -54,33 +54,11 @@ namespace nsec {
 void checkRRTypeBitmaps(const char* const rrtype_name,
                         const std::vector<uint8_t>& typebits);
 
-/// \brief Convert textual sequence of RR types into type bitmaps.
-///
-/// This function extracts a sequence of strings, converts each sequence
-/// into an RR type, and builds NSEC/NSEC3 type bitmaps with the corresponding
-/// bits for the extracted types being on.  The resulting bitmaps (which are
-/// in the wire format according to RFC4034 and RFC5155) are stored in the
-/// given vector.  This function expects the given string stream ends with
-/// the sequence.
-///
-/// \exception InvalidRdataText The given input stream does not meet the
-/// assumption (e.g. including invalid form of RR type, not ending with
-/// an RR type string).
-///
-/// \param rrtype_name Either "NSEC" or "NSEC3"; used as part of exception
-/// messages.
-/// \param iss Input stream that consists of a complete sequence of textual
-/// RR types for which the corresponding bits are set.
-/// \param typebits A placeholder for the resulting bitmaps.  Expected to be
-/// empty, but it's not checked.
-void buildBitmapsFromText(const char* const rrtype_name,
-                          std::istringstream& iss,
-                          std::vector<uint8_t>& typebits);
-
 /// \brief Convert textual sequence of RR types read from a lexer into
 /// type bitmaps.
 ///
-/// See the other variant above for description.
+/// See the other variant above for description. If \c allow_empty is
+/// true and there are no mnemonics, \c typebits is left untouched.
 ///
 /// \exception InvalidRdataText Data read from the given lexer does not
 /// meet the assumption (e.g. including invalid form of RR type, not
@@ -93,9 +71,13 @@ void buildBitmapsFromText(const char* const rrtype_name,
 /// bits are set.
 /// \param typebits A placeholder for the resulting bitmaps.  Expected to be
 /// empty, but it's not checked.
+/// \param allow_empty If true, the function simply returns if no RR
+/// type mnemonics are found. Otherwise, it throws an exception if no RR
+/// type mnemonics are found.
 void buildBitmapsFromLexer(const char* const rrtype_name,
                            isc::dns::MasterLexer& lexer,
-                           std::vector<uint8_t>& typebits);
+                           std::vector<uint8_t>& typebits,
+                           bool allow_empty = false);
 
 /// \brief Convert type bitmaps to textual sequence of RR types.
 ///
diff --git a/src/lib/dns/rdata/generic/dnskey_48.cc b/src/lib/dns/rdata/generic/dnskey_48.cc
index 054ac18..2e9a9f3 100644
--- a/src/lib/dns/rdata/generic/dnskey_48.cc
+++ b/src/lib/dns/rdata/generic/dnskey_48.cc
@@ -27,6 +27,8 @@
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 
+#include <memory>
+
 #include <stdio.h>
 #include <time.h>
 
@@ -51,51 +53,157 @@ struct DNSKEYImpl {
     const vector<uint8_t> keydata_;
 };
 
+/// \brief Constructor from string.
+///
+/// The given string must represent a valid DNSKEY RDATA.  There can be
+/// extra space characters at the beginning or end of the text (which
+/// are simply ignored), but other extra text, including a new line,
+/// will make the construction fail with an exception.
+///
+/// The Protocol and Algorithm fields must be within their valid
+/// ranges. The Public Key field must be present and must contain a
+/// Base64 encoding of the public key. Whitespace is allowed within the
+/// Base64 text.
+///
+/// It is okay for the key data to be missing.  Note: BIND 9 also accepts
+/// DNSKEY missing key data.  While the RFC is silent in this case, and it
+/// may be debatable what an implementation should do, but since this field
+/// is algorithm dependent and this implementations doesn't reject unknown
+/// algorithms, it's lenient here.
+///
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param dnskey_str A string containing the RDATA to be created
 DNSKEY::DNSKEY(const std::string& dnskey_str) :
     impl_(NULL)
 {
-    istringstream iss(dnskey_str);
-    unsigned int flags, protocol, algorithm;
-    stringbuf keydatabuf;
+    // We use auto_ptr here because if there is an exception in this
+    // constructor, the destructor is not called and there could be a
+    // leak of the DNSKEYImpl that constructFromLexer() returns.
+    std::auto_ptr<DNSKEYImpl> impl_ptr(NULL);
+
+    try {
+        std::istringstream ss(dnskey_str);
+        MasterLexer lexer;
+        lexer.pushSource(ss);
+
+        impl_ptr.reset(constructFromLexer(lexer));
+
+        if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
+            isc_throw(InvalidRdataText,
+                      "Extra input text for DNSKEY: " << dnskey_str);
+        }
+    } catch (const MasterLexer::LexerError& ex) {
+        isc_throw(InvalidRdataText,
+                  "Failed to construct DNSKEY from '" << dnskey_str << "': "
+                  << ex.what());
+    }
 
-    iss >> flags >> protocol >> algorithm >> &keydatabuf;
-    if (iss.bad() || iss.fail()) {
-        isc_throw(InvalidRdataText, "Invalid DNSKEY text");
+    impl_ = impl_ptr.release();
+}
+
+/// \brief Constructor from InputBuffer.
+///
+/// The passed buffer must contain a valid DNSKEY RDATA.
+///
+/// The Protocol and Algorithm fields are not checked for unknown
+/// values.  It is okay for the key data to be missing (see the description
+/// of the constructor from string).
+DNSKEY::DNSKEY(InputBuffer& buffer, size_t rdata_len) :
+    impl_(NULL)
+{
+    if (rdata_len < 4) {
+        isc_throw(InvalidRdataLength, "DNSKEY too short: " << rdata_len);
     }
+
+    const uint16_t flags = buffer.readUint16();
+    const uint16_t protocol = buffer.readUint8();
+    const uint16_t algorithm = buffer.readUint8();
+
+    rdata_len -= 4;
+
+    vector<uint8_t> keydata;
+    // If key data is missing, it's OK. See the API documentation of the
+    // constructor.
+    if (rdata_len > 0) {
+        keydata.resize(rdata_len);
+        buffer.readData(&keydata[0], rdata_len);
+    }
+
+    impl_ = new DNSKEYImpl(flags, protocol, algorithm, keydata);
+}
+
+/// \brief Constructor with a context of MasterLexer.
+///
+/// The \c lexer should point to the beginning of valid textual
+/// representation of an DNSKEY RDATA.
+///
+/// See \c DNSKEY::DNSKEY(const std::string&) for description of the
+/// expected RDATA fields.
+///
+/// \throw MasterLexer::LexerError General parsing error such as
+/// missing field.
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param lexer A \c MasterLexer object parsing a master file for the
+/// RDATA to be created
+DNSKEY::DNSKEY(MasterLexer& lexer, const Name*,
+               MasterLoader::Options, MasterLoaderCallbacks&) :
+    impl_(NULL)
+{
+    impl_ = constructFromLexer(lexer);
+}
+
+DNSKEYImpl*
+DNSKEY::constructFromLexer(MasterLexer& lexer) {
+    const uint32_t flags = lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (flags > 0xffff) {
-        isc_throw(InvalidRdataText, "DNSKEY flags out of range");
+        isc_throw(InvalidRdataText,
+                  "DNSKEY flags out of range: " << flags);
     }
+
+    const uint32_t protocol =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (protocol > 0xff) {
-        isc_throw(InvalidRdataText, "DNSKEY protocol out of range");
+        isc_throw(InvalidRdataText,
+                  "DNSKEY protocol out of range: " << protocol);
     }
+
+    const uint32_t algorithm =
+        lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (algorithm > 0xff) {
-        isc_throw(InvalidRdataText, "DNSKEY algorithm out of range");
+        isc_throw(InvalidRdataText,
+                  "DNSKEY algorithm out of range: " << algorithm);
     }
 
-    vector<uint8_t> keydata;
-    decodeBase64(keydatabuf.str(), keydata);
-
-    if (algorithm == 1 && keydata.size() < 3) {
-        isc_throw(InvalidRdataLength, "DNSKEY keydata too short");
-    }
+    std::string keydata_str;
+    std::string keydata_substr;
+    while (true) {
+        const MasterToken& token =
+            lexer.getNextToken(MasterToken::STRING, true);
+        if ((token.getType() == MasterToken::END_OF_FILE) ||
+            (token.getType() == MasterToken::END_OF_LINE)) {
+            break;
+        }
 
-    impl_ = new DNSKEYImpl(flags, protocol, algorithm, keydata);
-}
+        // token is now assured to be of type STRING.
 
-DNSKEY::DNSKEY(InputBuffer& buffer, size_t rdata_len) {
-    if (rdata_len < 4) {
-        isc_throw(InvalidRdataLength, "DNSKEY too short: " << rdata_len);
+        token.getString(keydata_substr);
+        keydata_str.append(keydata_substr);
     }
 
-    uint16_t flags = buffer.readUint16();
-    uint16_t protocol = buffer.readUint8();
-    uint16_t algorithm = buffer.readUint8();
+    lexer.ungetToken();
 
-    rdata_len -= 4;
-    vector<uint8_t> keydata(rdata_len);
-    buffer.readData(&keydata[0], rdata_len);
+    vector<uint8_t> keydata;
+    // If key data is missing, it's OK. See the API documentation of the
+    // constructor.
+    if (keydata_str.size() > 0) {
+        decodeBase64(keydata_str, keydata);
+    }
 
-    impl_ = new DNSKEYImpl(flags, protocol, algorithm, keydata);
+    return (new DNSKEYImpl(flags, protocol, algorithm, keydata));
 }
 
 DNSKEY::DNSKEY(const DNSKEY& source) :
@@ -157,11 +265,11 @@ DNSKEY::compare(const Rdata& other) const {
         return (impl_->algorithm_ < other_dnskey.impl_->algorithm_ ? -1 : 1);
     }
 
-    size_t this_len = impl_->keydata_.size();
-    size_t other_len = other_dnskey.impl_->keydata_.size();
-    size_t cmplen = min(this_len, other_len);
-    int cmp = memcmp(&impl_->keydata_[0], &other_dnskey.impl_->keydata_[0],
-                     cmplen);
+    const size_t this_len = impl_->keydata_.size();
+    const size_t other_len = other_dnskey.impl_->keydata_.size();
+    const size_t cmplen = min(this_len, other_len);
+    const int cmp = memcmp(&impl_->keydata_[0],
+                           &other_dnskey.impl_->keydata_[0], cmplen);
     if (cmp != 0) {
         return (cmp);
     } else {
@@ -172,15 +280,24 @@ DNSKEY::compare(const Rdata& other) const {
 uint16_t
 DNSKEY::getTag() const {
     if (impl_->algorithm_ == 1) {
-        int len = impl_->keydata_.size();
+        // See RFC 4034 appendix B.1 for why the key data must contain
+        // at least 4 bytes with RSA/MD5: 3 trailing bytes to extract
+        // the tag from, and 1 byte of exponent length subfield before
+        // modulus.
+        const int len = impl_->keydata_.size();
+        if (len < 4) {
+            isc_throw(isc::OutOfRange,
+                      "DNSKEY keydata too short for tag extraction");
+        }
+
         return ((impl_->keydata_[len - 3] << 8) + impl_->keydata_[len - 2]);
     }
 
     uint32_t ac = impl_->flags_;
     ac += (impl_->protocol_ << 8);
     ac += impl_->algorithm_;
-    
-    size_t size = impl_->keydata_.size();
+
+    const size_t size = impl_->keydata_.size();
     for (size_t i = 0; i < size; i ++) {
         ac += (i & 1) ? impl_->keydata_[i] : (impl_->keydata_[i] << 8);
     }
diff --git a/src/lib/dns/rdata/generic/dnskey_48.h b/src/lib/dns/rdata/generic/dnskey_48.h
index 14fad9f..3ef18e6 100644
--- a/src/lib/dns/rdata/generic/dnskey_48.h
+++ b/src/lib/dns/rdata/generic/dnskey_48.h
@@ -20,6 +20,7 @@
 #include <dns/rrtype.h>
 #include <dns/rrttl.h>
 #include <dns/rdata.h>
+#include <dns/master_lexer.h>
 
 // BEGIN_HEADER_GUARD
 
@@ -42,11 +43,19 @@ public:
     ///
     /// Specialized methods
     ///
+
+    /// \brief Returns the key tag
+    ///
+    /// \throw isc::OutOfRange if the key data for RSA/MD5 is too short
+    /// to support tag extraction.
     uint16_t getTag() const;
+
     uint16_t getFlags() const;
     uint8_t getAlgorithm() const;
 
 private:
+    DNSKEYImpl* constructFromLexer(isc::dns::MasterLexer& lexer);
+
     DNSKEYImpl* impl_;
 };
 
diff --git a/src/lib/dns/rdata/generic/mx_15.cc b/src/lib/dns/rdata/generic/mx_15.cc
index 12ada97..ae9978e 100644
--- a/src/lib/dns/rdata/generic/mx_15.cc
+++ b/src/lib/dns/rdata/generic/mx_15.cc
@@ -68,15 +68,7 @@ MX::MX(const std::string& mx_str) :
         MasterLexer lexer;
         lexer.pushSource(ss);
 
-        const uint32_t num =
-            lexer.getNextToken(MasterToken::NUMBER).getNumber();
-        if (num > 65535) {
-            isc_throw(InvalidRdataText, "Invalid MX preference in: "
-                      << mx_str);
-        }
-        preference_ = static_cast<uint16_t>(num);
-
-        mxname_ = createNameFromLexer(lexer, NULL);
+        constructFromLexer(lexer, NULL);
 
         if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
             isc_throw(InvalidRdataText, "extra input text for MX: "
@@ -108,8 +100,13 @@ MX::MX(const std::string& mx_str) :
 /// is non-absolute.
 MX::MX(MasterLexer& lexer, const Name* origin,
        MasterLoader::Options, MasterLoaderCallbacks&) :
-    preference_(0), mxname_(".")
+    preference_(0), mxname_(Name::ROOT_NAME())
 {
+    constructFromLexer(lexer, origin);
+}
+
+void
+MX::constructFromLexer(MasterLexer& lexer, const Name* origin) {
     const uint32_t num = lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (num > 65535) {
         isc_throw(InvalidRdataText, "Invalid MX preference: " << num);
diff --git a/src/lib/dns/rdata/generic/mx_15.h b/src/lib/dns/rdata/generic/mx_15.h
index 1381f18..84e7112 100644
--- a/src/lib/dns/rdata/generic/mx_15.h
+++ b/src/lib/dns/rdata/generic/mx_15.h
@@ -42,6 +42,9 @@ public:
     uint16_t getMXPref() const;
 
 private:
+    void constructFromLexer(isc::dns::MasterLexer& lexer,
+                            const isc::dns::Name* origin);
+
     /// Note: this is a prototype version; we may reconsider
     /// this representation later.
     uint16_t preference_;
diff --git a/src/lib/dns/rdata/generic/nsec3_50.cc b/src/lib/dns/rdata/generic/nsec3_50.cc
index fb92246..fd8f78d 100644
--- a/src/lib/dns/rdata/generic/nsec3_50.cc
+++ b/src/lib/dns/rdata/generic/nsec3_50.cc
@@ -35,6 +35,8 @@
 #include <dns/rdata/generic/detail/nsec_bitmap.h>
 #include <dns/rdata/generic/detail/nsec3param_common.h>
 
+#include <memory>
+
 #include <stdio.h>
 #include <time.h>
 
@@ -64,24 +66,86 @@ struct NSEC3Impl {
     const vector<uint8_t> typebits_;
 };
 
+/// \brief Constructor from string.
+///
+/// The given string must represent a valid NSEC3 RDATA.  There
+/// can be extra space characters at the beginning or end of the
+/// text (which are simply ignored), but other extra text, including
+/// a new line, will make the construction fail with an exception.
+///
+/// The Hash Algorithm, Flags and Iterations fields must be within their
+/// valid ranges. The Salt field may contain "-" to indicate that the
+/// salt is of length 0. The Salt field must not contain any whitespace.
+/// The type mnemonics must be valid, and separated by whitespace. If
+/// any invalid mnemonics are found, InvalidRdataText exception is
+/// thrown.
+///
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param nsec3_str A string containing the RDATA to be created
 NSEC3::NSEC3(const std::string& nsec3_str) :
     impl_(NULL)
 {
-    istringstream iss(nsec3_str);
+    // We use auto_ptr here because if there is an exception in this
+    // constructor, the destructor is not called and there could be a
+    // leak of the NSEC3Impl that constructFromLexer() returns.
+    std::auto_ptr<NSEC3Impl> impl_ptr(NULL);
+
+    try {
+        std::istringstream ss(nsec3_str);
+        MasterLexer lexer;
+        lexer.pushSource(ss);
+
+        impl_ptr.reset(constructFromLexer(lexer));
+
+        if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
+            isc_throw(InvalidRdataText,
+                      "Extra input text for NSEC3: " << nsec3_str);
+        }
+    } catch (const MasterLexer::LexerError& ex) {
+        isc_throw(InvalidRdataText,
+                  "Failed to construct NSEC3 from '" << nsec3_str << "': "
+                  << ex.what());
+    }
+
+    impl_ = impl_ptr.release();
+}
+
+/// \brief Constructor with a context of MasterLexer.
+///
+/// The \c lexer should point to the beginning of valid textual
+/// representation of an NSEC3 RDATA.
+///
+/// See \c NSEC3::NSEC3(const std::string&) for description of the
+/// expected RDATA fields.
+///
+/// \throw MasterLexer::LexerError General parsing error such as
+/// missing field.
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param lexer A \c MasterLexer object parsing a master file for the
+/// RDATA to be created
+NSEC3::NSEC3(MasterLexer& lexer, const Name*, MasterLoader::Options,
+             MasterLoaderCallbacks&) :
+    impl_(NULL)
+{
+    impl_ = constructFromLexer(lexer);
+}
+
+NSEC3Impl*
+NSEC3::constructFromLexer(MasterLexer& lexer) {
     vector<uint8_t> salt;
     const ParseNSEC3ParamResult params =
-        parseNSEC3ParamText("NSEC3", nsec3_str, iss, salt);
+        parseNSEC3ParamFromLexer("NSEC3", lexer, salt);
 
-    // Extract Next hash.  It must be an unpadded base32hex string.
-    string nexthash;
-    iss >> nexthash;
-    if (iss.bad() || iss.fail()) {
-        isc_throw(InvalidRdataText, "Invalid NSEC3 text: " << nsec3_str);
-    }
-    assert(!nexthash.empty());
+    const string& nexthash =
+        lexer.getNextToken(MasterToken::STRING).getString();
     if (*nexthash.rbegin() == '=') {
-        isc_throw(InvalidRdataText, "NSEC3 hash has padding: " << nsec3_str);
+        isc_throw(InvalidRdataText, "NSEC3 hash has padding: " << nexthash);
     }
+
     vector<uint8_t> next;
     decodeBase32Hex(nexthash, next);
     if (next.size() > 255) {
@@ -89,22 +153,16 @@ NSEC3::NSEC3(const std::string& nsec3_str) :
                   << next.size() << " bytes");
     }
 
-    // For NSEC3 empty bitmap is possible and allowed.
-    if (iss.eof()) {
-        impl_ = new NSEC3Impl(params.algorithm, params.flags,
-                              params.iterations, salt, next,
-                              vector<uint8_t>());
-        return;
-    }
-
     vector<uint8_t> typebits;
-    buildBitmapsFromText("NSEC3", iss, typebits);
-
-    impl_ = new NSEC3Impl(params.algorithm, params.flags, params.iterations,
-                          salt, next, typebits);
+    // For NSEC3 empty bitmap is possible and allowed.
+    buildBitmapsFromLexer("NSEC3", lexer, typebits, true);
+    return (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) :
+    impl_(NULL)
+{
     vector<uint8_t> salt;
     const ParseNSEC3ParamResult params =
         parseNSEC3ParamWire("NSEC3", buffer, rdata_len, salt);
diff --git a/src/lib/dns/rdata/generic/nsec3_50.h b/src/lib/dns/rdata/generic/nsec3_50.h
index c766ade..6a1dcb5 100644
--- a/src/lib/dns/rdata/generic/nsec3_50.h
+++ b/src/lib/dns/rdata/generic/nsec3_50.h
@@ -21,6 +21,7 @@
 #include <dns/rrtype.h>
 #include <dns/rrttl.h>
 #include <dns/rdata.h>
+#include <dns/master_lexer.h>
 
 // BEGIN_HEADER_GUARD
 
@@ -47,6 +48,8 @@ public:
     const std::vector<uint8_t>& getNext() const;
 
 private:
+    NSEC3Impl* constructFromLexer(isc::dns::MasterLexer& lexer);
+
     NSEC3Impl* impl_;
 };
 
diff --git a/src/lib/dns/rdata/generic/nsec3param_51.cc b/src/lib/dns/rdata/generic/nsec3param_51.cc
index 5686353..494746d 100644
--- a/src/lib/dns/rdata/generic/nsec3param_51.cc
+++ b/src/lib/dns/rdata/generic/nsec3param_51.cc
@@ -22,6 +22,7 @@
 
 #include <boost/lexical_cast.hpp>
 
+#include <memory>
 #include <string>
 #include <sstream>
 #include <vector>
@@ -46,24 +47,84 @@ struct NSEC3PARAMImpl {
     const vector<uint8_t> salt_;
 };
 
+/// \brief Constructor from string.
+///
+/// The given string must represent a valid NSEC3PARAM RDATA.  There
+/// can be extra space characters at the beginning or end of the
+/// text (which are simply ignored), but other extra text, including
+/// a new line, will make the construction fail with an exception.
+///
+/// The Hash Algorithm, Flags and Iterations fields must be within their
+/// valid ranges. The Salt field may contain "-" to indicate that the
+/// salt is of length 0. The Salt field must not contain any whitespace.
+///
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param nsec3param_str A string containing the RDATA to be created
 NSEC3PARAM::NSEC3PARAM(const std::string& nsec3param_str) :
     impl_(NULL)
 {
-    istringstream iss(nsec3param_str);
+    // We use auto_ptr here because if there is an exception in this
+    // constructor, the destructor is not called and there could be a
+    // leak of the NSEC3PARAMImpl that constructFromLexer() returns.
+    std::auto_ptr<NSEC3PARAMImpl> impl_ptr(NULL);
+
+    try {
+        std::istringstream ss(nsec3param_str);
+        MasterLexer lexer;
+        lexer.pushSource(ss);
+
+        impl_ptr.reset(constructFromLexer(lexer));
+
+        if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
+            isc_throw(InvalidRdataText,
+                      "Extra input text for NSEC3PARAM: " << nsec3param_str);
+        }
+    } catch (const MasterLexer::LexerError& ex) {
+        isc_throw(InvalidRdataText,
+                  "Failed to construct NSEC3PARAM from '" << nsec3param_str
+                  << "': " << ex.what());
+    }
+
+    impl_ = impl_ptr.release();
+}
+
+/// \brief Constructor with a context of MasterLexer.
+///
+/// The \c lexer should point to the beginning of valid textual
+/// representation of an NSEC3PARAM RDATA.
+///
+/// See \c NSEC3PARAM::NSEC3PARAM(const std::string&) for description of
+/// the expected RDATA fields.
+///
+/// \throw MasterLexer::LexerError General parsing error such as
+/// missing field.
+/// \throw InvalidRdataText if any fields are out of their valid range,
+/// or are incorrect.
+///
+/// \param lexer A \c MasterLexer object parsing a master file for the
+/// RDATA to be created
+NSEC3PARAM::NSEC3PARAM(MasterLexer& lexer, const Name*, MasterLoader::Options,
+                       MasterLoaderCallbacks&) :
+    impl_(NULL)
+{
+    impl_ = constructFromLexer(lexer);
+}
+
+NSEC3PARAMImpl*
+NSEC3PARAM::constructFromLexer(MasterLexer& lexer) {
     vector<uint8_t> salt;
     const ParseNSEC3ParamResult params =
-        parseNSEC3ParamText("NSEC3PARAM", nsec3param_str, iss, salt);
-
-    if (!iss.eof()) {
-        isc_throw(InvalidRdataText, "Invalid NSEC3PARAM (redundant text): "
-                  << nsec3param_str);
-    }
+        parseNSEC3ParamFromLexer("NSEC3PARAM", lexer, salt);
 
-    impl_ = new NSEC3PARAMImpl(params.algorithm, params.flags,
-                               params.iterations, salt);
+    return (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) :
+    impl_(NULL)
+{
     vector<uint8_t> salt;
     const ParseNSEC3ParamResult params =
         parseNSEC3ParamWire("NSEC3PARAM", buffer, rdata_len, salt);
diff --git a/src/lib/dns/rdata/generic/nsec3param_51.h b/src/lib/dns/rdata/generic/nsec3param_51.h
index 130c759..bd2ce75 100644
--- a/src/lib/dns/rdata/generic/nsec3param_51.h
+++ b/src/lib/dns/rdata/generic/nsec3param_51.h
@@ -21,6 +21,7 @@
 #include <dns/rrtype.h>
 #include <dns/rrttl.h>
 #include <dns/rdata.h>
+#include <dns/master_lexer.h>
 
 // BEGIN_HEADER_GUARD
 
@@ -47,7 +48,10 @@ public:
     uint8_t getFlags() const;
     uint16_t getIterations() const;
     const std::vector<uint8_t>& getSalt() const;
+
 private:
+    NSEC3PARAMImpl* constructFromLexer(isc::dns::MasterLexer& lexer);
+
     NSEC3PARAMImpl* impl_;
 };
 
diff --git a/src/lib/dns/rdata/generic/nsec_47.cc b/src/lib/dns/rdata/generic/nsec_47.cc
index c474b02..ffa2c97 100644
--- a/src/lib/dns/rdata/generic/nsec_47.cc
+++ b/src/lib/dns/rdata/generic/nsec_47.cc
@@ -136,15 +136,17 @@ NSEC::NSEC(InputBuffer& buffer, size_t rdata_len) {
 ///
 /// \param lexer A \c MasterLexer object parsing a master file for the
 /// RDATA to be created
+/// \param origin The origin to use with a relative Next Domain Name
+/// field
 NSEC::NSEC(MasterLexer& lexer, const Name* origin, MasterLoader::Options,
            MasterLoaderCallbacks&)
 {
-    const Name origin_name(createNameFromLexer(lexer, origin));
+    const Name next_name(createNameFromLexer(lexer, origin));
 
     vector<uint8_t> typebits;
     buildBitmapsFromLexer("NSEC", lexer, typebits);
 
-    impl_ = new NSECImpl(origin_name, typebits);
+    impl_ = new NSECImpl(next_name, typebits);
 }
 
 NSEC::NSEC(const NSEC& source) :
diff --git a/src/lib/dns/tests/masterload_unittest.cc b/src/lib/dns/tests/masterload_unittest.cc
index 7f1961c..dfd901a 100644
--- a/src/lib/dns/tests/masterload_unittest.cc
+++ b/src/lib/dns/tests/masterload_unittest.cc
@@ -167,7 +167,11 @@ TEST_F(MasterLoadTest, loadRRsigs) {
     EXPECT_EQ(2, results.size());
 }
 
-TEST_F(MasterLoadTest, loadRRWithComment) {
+// This test was disabled by #2387, because the test data has trailing
+// comments and it (eventually) uses the string RDATA constructor which
+// doesn't support them. This test should be fixed and re-enabled by
+// #2381, or deleted.
+TEST_F(MasterLoadTest, DISABLED_loadRRWithComment) {
     // Comment at the end of line should be ignored and the RR should be
     // accepted.
     rr_stream << "example.com. 3600 IN DNSKEY	256 3 7 "
@@ -180,7 +184,11 @@ TEST_F(MasterLoadTest, loadRRWithComment) {
                                       dnskey_rdata)));
 }
 
-TEST_F(MasterLoadTest, loadRRWithCommentNoSpace) {
+// This test was disabled by #2387, because the test data has trailing
+// comments and it (eventually) uses the string RDATA constructor which
+// doesn't support them. This test should be fixed and re-enabled by
+// #2381, or deleted.
+TEST_F(MasterLoadTest, DISABLED_loadRRWithCommentNoSpace) {
     // Similar to the previous one, but there's no space before comments.
     // It should still work.
     rr_stream << "example.com. 3600 IN DNSKEY	256 3 7 "
@@ -193,7 +201,11 @@ TEST_F(MasterLoadTest, loadRRWithCommentNoSpace) {
                                       dnskey_rdata)));
 }
 
-TEST_F(MasterLoadTest, loadRRWithCommentEmptyComment) {
+// This test was disabled by #2387, because the test data has trailing
+// comments and it (eventually) uses the string RDATA constructor which
+// doesn't support them. This test should be fixed and re-enabled by
+// #2381, or deleted.
+TEST_F(MasterLoadTest, DISABLED_loadRRWithCommentEmptyComment) {
     // Similar to the previous one, but there's no data after the ;
     // It should still work.
     rr_stream << "example.com. 3600 IN DNSKEY	256 3 7 "
@@ -206,7 +218,11 @@ TEST_F(MasterLoadTest, loadRRWithCommentEmptyComment) {
                                       dnskey_rdata)));
 }
 
-TEST_F(MasterLoadTest, loadRRWithCommentEmptyCommentNoSpace) {
+// This test was disabled by #2387, because the test data has trailing
+// comments and it (eventually) uses the string RDATA constructor which
+// doesn't support them. This test should be fixed and re-enabled by
+// #2381, or deleted.
+TEST_F(MasterLoadTest, DISABLED_loadRRWithCommentEmptyCommentNoSpace) {
     // Similar to the previous one, but there's no space before or after ;
     // It should still work.
     rr_stream << "example.com. 3600 IN DNSKEY	256 3 7 "
diff --git a/src/lib/dns/tests/rdata_dnskey_unittest.cc b/src/lib/dns/tests/rdata_dnskey_unittest.cc
index 58d29bf..872dc2a 100644
--- a/src/lib/dns/tests/rdata_dnskey_unittest.cc
+++ b/src/lib/dns/tests/rdata_dnskey_unittest.cc
@@ -37,98 +37,168 @@ using namespace isc::dns::rdata;
 
 namespace {
 class Rdata_DNSKEY_Test : public RdataTest {
-    // there's nothing to specialize
+protected:
+    Rdata_DNSKEY_Test() :
+        dnskey_txt("257 3 5 BEAAAAOhHQDBrhQbtphgq2wQUpEQ5t4DtUHxoMV"
+                   "Fu2hWLDMvoOMRXjGrhhCeFvAZih7yJHf8ZGfW6hd38hXG/x"
+                   "ylYCO6Krpbdojwx8YMXLA5/kA+u50WIL8ZR1R6KTbsYVMf/"
+                   "Qx5RiNbPClw+vT+U8eXEJmO20jIS1ULgqy347cBB1zMnnz/"
+                   "4LJpA0da9CbKj3A254T515sNIMcwsB8/2+2E63/zZrQzBkj"
+                   "0BrN/9Bexjpiks3jRhZatEsXn3dTy47R09Uix5WcJt+xzqZ"
+                   "7+ysyLKOOedS39Z7SDmsn2eA0FKtQpwA6LXeG2w+jxmw3oA"
+                   "8lVUgEf/rzeC/bByBNsO70aEFTd"),
+        dnskey_txt2("257 3 5 YmluZDEwLmlzYy5vcmc="),
+        rdata_dnskey(dnskey_txt),
+        rdata_dnskey2(dnskey_txt2)
+    {}
+
+    void checkFromText_None(const string& rdata_str) {
+        checkFromText<generic::DNSKEY, isc::Exception, isc::Exception>(
+            rdata_str, rdata_dnskey2, false, false);
+    }
+
+    void checkFromText_InvalidText(const string& rdata_str) {
+        checkFromText<generic::DNSKEY, InvalidRdataText, InvalidRdataText>(
+            rdata_str, rdata_dnskey2, true, true);
+    }
+
+    void checkFromText_InvalidLength(const string& rdata_str) {
+        checkFromText<generic::DNSKEY, InvalidRdataLength, InvalidRdataLength>(
+            rdata_str, rdata_dnskey2, true, true);
+    }
+
+    void checkFromText_BadValue(const string& rdata_str) {
+        checkFromText<generic::DNSKEY, BadValue, BadValue>(
+            rdata_str, rdata_dnskey2, true, true);
+    }
+
+    void checkFromText_LexerError(const string& rdata_str) {
+        checkFromText
+            <generic::DNSKEY, InvalidRdataText, MasterLexer::LexerError>(
+                rdata_str, rdata_dnskey2, true, true);
+    }
+
+    void checkFromText_BadString(const string& rdata_str) {
+        checkFromText
+            <generic::DNSKEY, InvalidRdataText, isc::Exception>(
+                rdata_str, rdata_dnskey2, true, false);
+    }
+
+    const string dnskey_txt;
+    const string dnskey_txt2;
+    const generic::DNSKEY rdata_dnskey;
+    const generic::DNSKEY rdata_dnskey2;
 };
 
-string dnskey_txt("257 3 5 BEAAAAOhHQDBrhQbtphgq2wQUpEQ5t4DtUHxoMV"
-                  "Fu2hWLDMvoOMRXjGrhhCeFvAZih7yJHf8ZGfW6hd38hXG/x"
-                  "ylYCO6Krpbdojwx8YMXLA5/kA+u50WIL8ZR1R6KTbsYVMf/"
-                  "Qx5RiNbPClw+vT+U8eXEJmO20jIS1ULgqy347cBB1zMnnz/"
-                  "4LJpA0da9CbKj3A254T515sNIMcwsB8/2+2E63/zZrQzBkj"
-                  "0BrN/9Bexjpiks3jRhZatEsXn3dTy47R09Uix5WcJt+xzqZ"
-                  "7+ysyLKOOedS39Z7SDmsn2eA0FKtQpwA6LXeG2w+jxmw3oA"
-                  "8lVUgEf/rzeC/bByBNsO70aEFTd");
-
 TEST_F(Rdata_DNSKEY_Test, fromText) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(dnskey_txt, rdata_dnskey.toText());
-}
 
-TEST_F(Rdata_DNSKEY_Test, assign) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
-    generic::DNSKEY rdata_dnskey2 = rdata_dnskey;
-    EXPECT_EQ(0, rdata_dnskey.compare(rdata_dnskey2));
-}
+    // Space in key data is OK
+    checkFromText_None("257 3 5 YmluZDEw LmlzYy5vcmc=");
+
+    // Delimited number in key data is OK
+    checkFromText_None("257 3 5 YmluZDEwLmlzYy 5 vcmc=");
+
+    // Missing keydata is OK
+    EXPECT_NO_THROW(const generic::DNSKEY rdata_dnskey3("257 3 5"));
+
+    // Key data too short for RSA/MD5 algorithm is OK when
+    // constructing. But getTag() on this object would throw (see
+    // .getTag tests).
+    EXPECT_NO_THROW(const generic::DNSKEY rdata_dnskey4("1 1 1 YQ=="));
+
+    // Flags field out of range
+    checkFromText_InvalidText("65536 3 5 YmluZDEwLmlzYy5vcmc=");
+
+    // Protocol field out of range
+    checkFromText_InvalidText("257 256 5 YmluZDEwLmlzYy5vcmc=");
+
+    // Algorithm field out of range
+    checkFromText_InvalidText("257 3 256 YmluZDEwLmlzYy5vcmc=");
+
+    // Missing algorithm field
+    checkFromText_LexerError("257 3 YmluZDEwLmlzYy5vcmc=");
 
-TEST_F(Rdata_DNSKEY_Test, badText) {
-    EXPECT_THROW(generic::DNSKEY("257 3 5"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::DNSKEY("99999 3 5 BAAAAAAAAAAAD"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::DNSKEY("257 300 5 BAAAAAAAAAAAD"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::DNSKEY("257 3 500 BAAAAAAAAAAAD"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::DNSKEY("257 3 5 BAAAAAAAAAAAD"), BadValue);
+    // Invalid key data field (not Base64)
+    checkFromText_BadValue("257 3 5 BAAAAAAAAAAAD");
+
+    // String instead of number
+    checkFromText_LexerError("foo 3 5 YmluZDEwLmlzYy5vcmc=");
+    checkFromText_LexerError("257 foo 5 YmluZDEwLmlzYy5vcmc=");
+    checkFromText_LexerError("257 3 foo YmluZDEwLmlzYy5vcmc=");
+
+    // Trailing garbage. This should cause only the string constructor
+    // to fail, but the lexer constructor must be able to continue
+    // parsing from it.
+    checkFromText_BadString("257 3 5 YmluZDEwLmlzYy5vcmc= ; comment\n"
+                            "257 3 4 YmluZDEwLmlzYy5vcmc=");
+
+    // Unmatched parenthesis should cause a lexer error
+    checkFromText_LexerError("257 3 5 )YmluZDEwLmlzYy5vcmc=");
 }
 
-TEST_F(Rdata_DNSKEY_Test, DISABLED_badText) {
-    // Should this be allowed?  Probably not.  But the test currently fails.
-    EXPECT_THROW(generic::DNSKEY("257 3 5BEAAEFTd"),
-                 InvalidRdataText);
-    // How about this?  It's even more confusing for the parser because
-    // it could be ambiguous '51 EAAA' vs '5 1EAA..'
-    EXPECT_THROW(generic::DNSKEY("257 3 51EAAEFTd"),
-                 InvalidRdataText);
+TEST_F(Rdata_DNSKEY_Test, assign) {
+    generic::DNSKEY rdata_dnskey2("257 3 5 YQ==");
+    rdata_dnskey2 = rdata_dnskey;
+    EXPECT_EQ(0, rdata_dnskey.compare(rdata_dnskey2));
 }
 
 TEST_F(Rdata_DNSKEY_Test, createFromLexer) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(0, rdata_dnskey.compare(
         *test::createRdataUsingLexer(RRType::DNSKEY(), RRClass::IN(),
                                      dnskey_txt)));
-
-    // Exceptions cause NULL to be returned.
-    EXPECT_FALSE(test::createRdataUsingLexer(RRType::DNSKEY(), RRClass::IN(),
-                                             "257 3 5"));
 }
 
 TEST_F(Rdata_DNSKEY_Test, toWireRenderer) {
     renderer.skip(2);
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     rdata_dnskey.toWire(renderer);
 
     vector<unsigned char> data;
-    UnitTestUtil::readWireData("rdata_dnskey_fromWire", data);
+    UnitTestUtil::readWireData("rdata_dnskey_fromWire.wire", data);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
                         static_cast<const uint8_t *>(renderer.getData()) + 2,
                         renderer.getLength() - 2, &data[2], data.size() - 2);
 }
 
 TEST_F(Rdata_DNSKEY_Test, toWireBuffer) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     rdata_dnskey.toWire(obuffer);
+
+    vector<unsigned char> data;
+    UnitTestUtil::readWireData("rdata_dnskey_fromWire.wire", data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        obuffer.getData(), obuffer.getLength(),
+                        &data[2], data.size() - 2);
 }
 
 TEST_F(Rdata_DNSKEY_Test, createFromWire) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(0, rdata_dnskey.compare(
                   *rdataFactoryFromFile(RRType("DNSKEY"), RRClass("IN"),
-                                        "rdata_dnskey_fromWire")));
+                                        "rdata_dnskey_fromWire.wire")));
+
+    // Missing keydata is OK
+    const generic::DNSKEY rdata_dnskey_missing_keydata("257 3 5");
+    EXPECT_EQ(0, rdata_dnskey_missing_keydata.compare(
+        *rdataFactoryFromFile(RRType("DNSKEY"), RRClass("IN"),
+                              "rdata_dnskey_empty_keydata_fromWire.wire")));
 }
 
 TEST_F(Rdata_DNSKEY_Test, getTag) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(12892, rdata_dnskey.getTag());
+
+    // Short keydata with algorithm RSA/MD5 must throw.
+    const generic::DNSKEY rdata_dnskey_short_keydata1("1 1 1 YQ==");
+    EXPECT_THROW(rdata_dnskey_short_keydata1.getTag(), isc::OutOfRange);
+
+    // Short keydata with algorithm not RSA/MD5 must not throw.
+    const generic::DNSKEY rdata_dnskey_short_keydata2("257 3 5 YQ==");
+    EXPECT_NO_THROW(rdata_dnskey_short_keydata2.getTag());
 }
 
 TEST_F(Rdata_DNSKEY_Test, getAlgorithm) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(5, rdata_dnskey.getAlgorithm());
 }
 
 TEST_F(Rdata_DNSKEY_Test, getFlags) {
-    generic::DNSKEY rdata_dnskey(dnskey_txt);
     EXPECT_EQ(257, rdata_dnskey.getFlags());
 }
 
diff --git a/src/lib/dns/tests/rdata_nsec3_unittest.cc b/src/lib/dns/tests/rdata_nsec3_unittest.cc
index 0fec3eb..1f35713 100644
--- a/src/lib/dns/tests/rdata_nsec3_unittest.cc
+++ b/src/lib/dns/tests/rdata_nsec3_unittest.cc
@@ -17,7 +17,6 @@
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
-#include <util/encode/hex.h>
 #include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
 #include <dns/rdata.h>
@@ -35,7 +34,6 @@ using namespace std;
 using namespace isc;
 using namespace isc::dns;
 using namespace isc::util;
-using namespace isc::util::encode;
 using namespace isc::dns::rdata;
 
 namespace {
@@ -43,55 +41,114 @@ 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 {
-    // there's nothing to specialize
-public:
+protected:
     Rdata_NSEC3_Test() :
         nsec3_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                  "NS SOA RRSIG DNSKEY NSEC3PARAM"),
-        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A" )
+                  "A NS SOA"),
+        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA"),
+        nsec3_notype_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"),
+        rdata_nsec3(nsec3_txt)
     {}
+
+    void checkFromText_None(const string& rdata_str) {
+        checkFromText<generic::NSEC3, isc::Exception, isc::Exception>(
+            rdata_str, rdata_nsec3, false, false);
+    }
+
+    void checkFromText_InvalidText(const string& rdata_str) {
+        checkFromText<generic::NSEC3, InvalidRdataText, InvalidRdataText>(
+            rdata_str, rdata_nsec3, true, true);
+    }
+
+    void checkFromText_BadValue(const string& rdata_str) {
+        checkFromText<generic::NSEC3, BadValue, BadValue>(
+            rdata_str, rdata_nsec3, true, true);
+    }
+
+    void checkFromText_LexerError(const string& rdata_str) {
+        checkFromText
+            <generic::NSEC3, InvalidRdataText, MasterLexer::LexerError>(
+                rdata_str, rdata_nsec3, true, true);
+    }
+
+    void checkFromText_BadString(const string& rdata_str) {
+        checkFromText
+            <generic::NSEC3, InvalidRdataText, isc::Exception>(
+                rdata_str, rdata_nsec3, true, false);
+    }
+
     const string nsec3_txt;
     const string nsec3_nosalt_txt;
+    const string nsec3_notype_txt;
+    const generic::NSEC3 rdata_nsec3;
 };
 
 TEST_F(Rdata_NSEC3_Test, fromText) {
-    // A normal case: the test constructor should successfully parse the
-    // text and construct nsec3_txt.  It will be tested against the wire format
-    // representation in the createFromWire test.
-
-    // hash that has the possible max length (see badText about the magic
-    // numbers)
+    // Hash that has the possible max length
     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.
-    EXPECT_NO_THROW(generic::NSEC3(
-                        "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"));
-}
-
-TEST_F(Rdata_NSEC3_Test, badText) {
-    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV "
-                                "BIFF POW SPOON"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
-                                "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
-                 BadValue);     // bad base32hex
-    EXPECT_THROW(generic::NSEC3("1 1 1000000 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
-                 InvalidRdataText);
-
-    // Next hash shouldn't be padded
-    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE CPNMU=== A NS SOA"),
-                 InvalidRdataText);
-
     // Hash is too long.  Max = 255 bytes, base32-hex converts each 5 bytes
     // of the original to 8 characters, so 260 * 8 / 5 is the smallest length
     // of the encoded string that exceeds the max and doesn't require padding.
-    EXPECT_THROW(generic::NSEC3("1 1 1 D399EAAB " + string((260 * 8) / 5, '0') +
-                                " NS"),
-                 InvalidRdataText);
+    checkFromText_InvalidText("1 1 1 D399EAAB " + string((260 * 8) / 5, '0') +
+                              " A NS SOA");
+
+    // Type bitmap is empty.  it's possible and allowed for NSEC3.
+    EXPECT_NO_THROW(const generic::NSEC3 rdata_notype_nsec3(nsec3_notype_txt));
+
+    // Empty salt is also okay.
+    EXPECT_NO_THROW(const generic::NSEC3 rdata_nosalt_nsec3(nsec3_nosalt_txt));
+
+    // Bad type mnemonics
+    checkFromText_InvalidText("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"
+                              " BIFF POW SPOON");
+
+    // Bad base32hex
+    checkFromText_BadValue("1 1 1 D399EAAB "
+                           "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA");
+
+    // Hash algorithm out of range
+    checkFromText_InvalidText("256 1 1 D399EAAB "
+                              "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Flags out of range
+    checkFromText_InvalidText("1 256 1 D399EAAB "
+                              "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Iterations out of range
+    checkFromText_InvalidText("1 1 65536 D399EAAB "
+                              "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Space is not allowed in salt or the next hash. This actually
+    // causes the Base32 decoder that parses the next hash that comes
+    // afterwards, to throw.
+    checkFromText_BadValue("1 1 1 D399 EAAB H9RSFB7FPF2L8"
+                           "HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Next hash must not contain padding (trailing '=' characters)
+    checkFromText_InvalidText("1 1 1 D399EAAB "
+                              "AAECAwQFBgcICQoLDA0ODw== A NS SOA");
+
+    // String instead of number
+    checkFromText_LexerError("foo 1 1 D399EAAB "
+                             "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+    checkFromText_LexerError("1 foo 1 D399EAAB "
+                             "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+    checkFromText_LexerError("1 1 foo D399EAAB "
+                             "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Trailing garbage. This should cause only the string constructor
+    // to fail, but the lexer constructor must be able to continue
+    // parsing from it.
+    checkFromText_BadString(
+        "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA ;comment\n"
+        "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A NS SOA");
+
+    // Unmatched parenthesis should cause a lexer error
+    checkFromText_LexerError("1 1 1 D399EAAB "
+                             "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A ) NS SOA");
 }
 
 TEST_F(Rdata_NSEC3_Test, createFromWire) {
@@ -131,19 +188,18 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
 }
 
 TEST_F(Rdata_NSEC3_Test, createFromLexer) {
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
     EXPECT_EQ(0, rdata_nsec3.compare(
         *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(),
                                      nsec3_txt)));
 
-    // Exceptions cause NULL to be returned.
-    EXPECT_FALSE(test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(),
-                                             "1 1 1 ADDAFEEE CPNMU=== "
-                                             "A NS SOA"));
+    // empty salt is also okay.
+    const generic::NSEC3 rdata_nosalt_nsec3(nsec3_nosalt_txt);
+    EXPECT_EQ(0, rdata_nosalt_nsec3.compare(
+        *test::createRdataUsingLexer(RRType::NSEC3(), RRClass::IN(),
+                                     nsec3_nosalt_txt)));
 }
 
 TEST_F(Rdata_NSEC3_Test, assign) {
-    generic::NSEC3 rdata_nsec3(nsec3_txt);
     generic::NSEC3 other_nsec3 = rdata_nsec3;
     EXPECT_EQ(0, rdata_nsec3.compare(other_nsec3));
 }
diff --git a/src/lib/dns/tests/rdata_nsec3param_unittest.cc b/src/lib/dns/tests/rdata_nsec3param_unittest.cc
index 115d3d3..4fccbf3 100644
--- a/src/lib/dns/tests/rdata_nsec3param_unittest.cc
+++ b/src/lib/dns/tests/rdata_nsec3param_unittest.cc
@@ -16,8 +16,6 @@
 
 #include <exceptions/exceptions.h>
 
-#include <util/encode/base32hex.h>
-#include <util/encode/hex.h>
 #include <util/buffer.h>
 #include <dns/messagerenderer.h>
 #include <dns/rdata.h>
@@ -35,39 +33,107 @@ using namespace std;
 using namespace isc;
 using namespace isc::dns;
 using namespace isc::util;
-using namespace isc::util::encode;
 using namespace isc::dns::rdata;
 
 namespace {
 class Rdata_NSEC3PARAM_Test : public RdataTest {
-public:
-    Rdata_NSEC3PARAM_Test() : nsec3param_txt("1 1 1 D399EAAB") {}
+protected:
+    Rdata_NSEC3PARAM_Test() :
+        nsec3param_txt("1 1 1 D399EAAB"),
+        nsec3param_nosalt_txt("1 1 1 -"),
+        rdata_nsec3param(nsec3param_txt)
+    {}
+
+    void checkFromText_None(const string& rdata_str) {
+        checkFromText<generic::NSEC3PARAM, isc::Exception, isc::Exception>(
+            rdata_str, rdata_nsec3param, false, false);
+    }
+
+    void checkFromText_InvalidText(const string& rdata_str) {
+        checkFromText<generic::NSEC3PARAM, InvalidRdataText, InvalidRdataText>(
+            rdata_str, rdata_nsec3param, true, true);
+    }
+
+    void checkFromText_BadValue(const string& rdata_str) {
+        checkFromText<generic::NSEC3PARAM, BadValue, BadValue>(
+            rdata_str, rdata_nsec3param, true, true);
+    }
+
+    void checkFromText_LexerError(const string& rdata_str) {
+        checkFromText
+            <generic::NSEC3PARAM, InvalidRdataText, MasterLexer::LexerError>(
+                rdata_str, rdata_nsec3param, true, true);
+    }
+
+    void checkFromText_BadString(const string& rdata_str,
+                                 const generic::NSEC3PARAM& rdata)
+    {
+        checkFromText
+            <generic::NSEC3PARAM, InvalidRdataText, isc::Exception>(
+                rdata_str, rdata, true, false);
+    }
+
     const string nsec3param_txt;
+    const string nsec3param_nosalt_txt;
+    const generic::NSEC3PARAM rdata_nsec3param;
 };
 
 TEST_F(Rdata_NSEC3PARAM_Test, fromText) {
-    // With a salt
-    EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getHashalg());
-    EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getFlags());
-    // (salt is checked in the toText test)
+    // Empty salt is okay.
+    EXPECT_EQ(0, generic::NSEC3PARAM(nsec3param_nosalt_txt).getSalt().size());
+
+    // Salt is missing.
+    checkFromText_LexerError("1 1 1");
+
+    // Salt has whitespace within. This only fails in the string
+    // constructor, as the lexer constructor stops reading at the end of
+    // its RDATA.
+    const generic::NSEC3PARAM rdata_nsec3param2("1 1 1 D399");
+    checkFromText_BadString("1 1 1 D399 EAAB", rdata_nsec3param2);
+
+    // Hash algorithm out of range.
+    checkFromText_InvalidText("256 1 1 D399EAAB");
+
+    // Flags out of range.
+    checkFromText_InvalidText("1 256 1 D399EAAB");
+
+    // Iterations out of range.
+    checkFromText_InvalidText("1 1 65536 D399EAAB");
+
+    // Bad hex sequence
+    checkFromText_BadValue("1 1 256 D399EAABZOO");
 
-    // With an empty salt
-    EXPECT_EQ(0, generic::NSEC3PARAM("1 0 0 -").getSalt().size());
+    // String instead of number
+    checkFromText_LexerError("foo 1 256 D399EAAB");
+    checkFromText_LexerError("1 foo 256 D399EAAB");
+    checkFromText_LexerError("1 1 foo D399EAAB");
+
+    // Trailing garbage. This should cause only the string constructor
+    // to fail, but the lexer constructor must be able to continue
+    // parsing from it.
+    checkFromText_BadString("1 1 1 D399EAAB ; comment\n"
+                            "1 1 1 D399EAAB", rdata_nsec3param);
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, toText) {
-    const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
     EXPECT_EQ(nsec3param_txt, rdata_nsec3param.toText());
-}
 
-TEST_F(Rdata_NSEC3PARAM_Test, badText) {
-    // garbage space at the end
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 D399EAAB "),
-                 InvalidRdataText);
+    // Garbage space at the end should be ok. RFC5155 only forbids
+    // whitespace within the salt field, but any whitespace afterwards
+    // should be fine.
+    EXPECT_NO_THROW(generic::NSEC3PARAM("1 1 1 D399EAAB "));
+
+    // Hash algorithm in range.
+    EXPECT_NO_THROW(generic::NSEC3PARAM("255 1 1 D399EAAB"));
+
+    // Flags in range.
+    EXPECT_NO_THROW(generic::NSEC3PARAM("1 255 1 D399EAAB"));
+
+    // Iterations in range.
+    EXPECT_NO_THROW(generic::NSEC3PARAM("1 1 65535 D399EAAB"));
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
-    const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
     EXPECT_EQ(0, rdata_nsec3param.compare(
                   *rdataFactoryFromFile(RRType::NSEC3PARAM(), RRClass::IN(),
                                        "rdata_nsec3param_fromWire1")));
@@ -87,15 +153,19 @@ TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, createFromLexer) {
-    const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
     EXPECT_EQ(0, rdata_nsec3param.compare(
         *test::createRdataUsingLexer(RRType::NSEC3PARAM(), RRClass::IN(),
                                      nsec3param_txt)));
+
+    // empty salt is also okay.
+    const generic::NSEC3PARAM rdata_nosalt_nsec3param(nsec3param_nosalt_txt);
+    EXPECT_EQ(0, rdata_nosalt_nsec3param.compare(
+        *test::createRdataUsingLexer(RRType::NSEC3PARAM(), RRClass::IN(),
+                                     nsec3param_nosalt_txt)));
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) {
     renderer.skip(2);
-    const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
     rdata_nsec3param.toWire(renderer);
 
     vector<unsigned char> data;
@@ -106,13 +176,26 @@ TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) {
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, toWireBuffer) {
-    const generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
     rdata_nsec3param.toWire(obuffer);
+
+    vector<unsigned char> data;
+    UnitTestUtil::readWireData("rdata_nsec3param_fromWire1", data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        obuffer.getData(), obuffer.getLength(),
+                        &data[2], data.size() - 2);
+}
+
+TEST_F(Rdata_NSEC3PARAM_Test, getHashAlg) {
+    EXPECT_EQ(1, rdata_nsec3param.getHashalg());
+}
+
+TEST_F(Rdata_NSEC3PARAM_Test, getFlags) {
+    EXPECT_EQ(1, rdata_nsec3param.getFlags());
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, assign) {
-    generic::NSEC3PARAM rdata_nsec3param(nsec3param_txt);
-    generic::NSEC3PARAM other_nsec3param = rdata_nsec3param;
+    generic::NSEC3PARAM other_nsec3param("1 1 1 -");
+    other_nsec3param = rdata_nsec3param;
     EXPECT_EQ(0, rdata_nsec3param.compare(other_nsec3param));
 }
 
diff --git a/src/lib/dns/tests/testdata/.gitignore b/src/lib/dns/tests/testdata/.gitignore
index e8879e1..a0a14f4 100644
--- a/src/lib/dns/tests/testdata/.gitignore
+++ b/src/lib/dns/tests/testdata/.gitignore
@@ -41,6 +41,8 @@
 /rdata_minfo_toWire2.wire
 /rdata_minfo_toWireUncompressed1.wire
 /rdata_minfo_toWireUncompressed2.wire
+/rdata_dnskey_fromWire.wire
+/rdata_dnskey_empty_keydata_fromWire.wire
 /rdata_nsec3_fromWire10.wire
 /rdata_nsec3_fromWire11.wire
 /rdata_nsec3_fromWire12.wire
diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am
index 52acb7c..b6d7e35 100644
--- a/src/lib/dns/tests/testdata/Makefile.am
+++ b/src/lib/dns/tests/testdata/Makefile.am
@@ -16,6 +16,7 @@ BUILT_SOURCES += message_toText3.wire
 BUILT_SOURCES += name_toWire5.wire name_toWire6.wire
 BUILT_SOURCES += rdatafields1.wire rdatafields2.wire rdatafields3.wire
 BUILT_SOURCES += rdatafields4.wire rdatafields5.wire rdatafields6.wire
+BUILT_SOURCES += rdata_dnskey_fromWire.wire rdata_dnskey_empty_keydata_fromWire.wire
 BUILT_SOURCES += rdata_nsec_fromWire4.wire rdata_nsec_fromWire5.wire
 BUILT_SOURCES += rdata_nsec_fromWire6.wire rdata_nsec_fromWire7.wire
 BUILT_SOURCES += rdata_nsec_fromWire8.wire rdata_nsec_fromWire9.wire
@@ -101,7 +102,8 @@ EXTRA_DIST += name_toWire7 name_toWire8 name_toWire9
 EXTRA_DIST += question_fromWire question_toWire1 question_toWire2
 EXTRA_DIST += rdatafields1.spec rdatafields2.spec rdatafields3.spec
 EXTRA_DIST += rdatafields4.spec rdatafields5.spec rdatafields6.spec
-EXTRA_DIST += rdata_cname_fromWire rdata_dname_fromWire rdata_dnskey_fromWire
+EXTRA_DIST += rdata_cname_fromWire rdata_dname_fromWire
+EXTRA_DIST += rdata_dnskey_fromWire.spec rdata_dnskey_empty_keydata_fromWire.spec
 EXTRA_DIST += rdata_dhcid_fromWire rdata_dhcid_toWire
 EXTRA_DIST += rdata_ds_fromWire rdata_in_a_fromWire rdata_in_aaaa_fromWire
 EXTRA_DIST += rdata_mx_fromWire rdata_mx_toWire1 rdata_mx_toWire2
diff --git a/src/lib/dns/tests/testdata/rdata_dnskey_empty_keydata_fromWire.spec b/src/lib/dns/tests/testdata/rdata_dnskey_empty_keydata_fromWire.spec
new file mode 100644
index 0000000..b65271d
--- /dev/null
+++ b/src/lib/dns/tests/testdata/rdata_dnskey_empty_keydata_fromWire.spec
@@ -0,0 +1,7 @@
+# DNSKEY test data with empty digest
+
+[custom]
+sections: dnskey
+
+[dnskey]
+digest:
diff --git a/src/lib/dns/tests/testdata/rdata_dnskey_fromWire b/src/lib/dns/tests/testdata/rdata_dnskey_fromWire
deleted file mode 100644
index b703da1..0000000
--- a/src/lib/dns/tests/testdata/rdata_dnskey_fromWire
+++ /dev/null
@@ -1,24 +0,0 @@
-# RDLENGTH = 265 bytes
- 01 09
-# DNSKEY, flags 257
- 01 01
-# protocol 3, algorithm 5
- 03 05
-# keydata:
- 04 40 00 00 03 a1 1d 00 c1 ae 14 1b b6 98 60 ab
- 6c 10 52 91 10 e6 de 03 b5 41 f1 a0 c5 45 bb 68
- 56 2c 33 2f a0 e3 11 5e 31 ab 86 10 9e 16 f0 19
- 8a 1e f2 24 77 fc 64 67 d6 ea 17 77 f2 15 c6 ff
- 1c a5 60 23 ba 2a ba 5b 76 88 f0 c7 c6 0c 5c b0
- 39 fe 40 3e bb 9d 16 20 bf 19 47 54 7a 29 36 ec
- 61 53 1f fd 0c 79 46 23 5b 3c 29 70 fa f4 fe 53
- c7 97 10 99 8e db 48 c8 4b 55 0b 82 ac b7 e3 b7
- 01 07 5c cc 9e 7c ff e0 b2 69 03 47 5a f4 26 ca
- 8f 70 36 e7 84 f9 d7 9b 0d 20 c7 30 b0 1f 3f db
- ed 84 eb 7f f3 66 b4 33 06 48 f4 06 b3 7f f4 17
- b1 8e 98 a4 b3 78 d1 85 96 ad 12 c5 e7 dd d4 f2
- e3 b4 74 f5 48 b1 e5 67 09 b7 ec 73 a9 9e fe ca
- cc 8b 28 e3 9e 75 2d fd 67 b4 83 9a c9 f6 78 0d
- 05 2a d4 29 c0 0e 8b 5d e1 b6 c3 e8 f1 9b 0d e8
- 03 c9 55 52 01 1f fe bc de 0b f6 c1 c8 13 6c 3b
- bd 1a 10 54 dd
diff --git a/src/lib/dns/tests/testdata/rdata_dnskey_fromWire.spec b/src/lib/dns/tests/testdata/rdata_dnskey_fromWire.spec
new file mode 100644
index 0000000..87e66db
--- /dev/null
+++ b/src/lib/dns/tests/testdata/rdata_dnskey_fromWire.spec
@@ -0,0 +1,7 @@
+# DNSKEY test data
+
+[custom]
+sections: dnskey
+
+[dnskey]
+digest: BEAAAAOhHQDBrhQbtphgq2wQUpEQ5t4DtUHxoMVFu2hWLDMvoOMRXjGrhhCeFvAZih7yJHf8ZGfW6hd38hXG/xylYCO6Krpbdojwx8YMXLA5/kA+u50WIL8ZR1R6KTbsYVMf/Qx5RiNbPClw+vT+U8eXEJmO20jIS1ULgqy347cBB1zMnnz/4LJpA0da9CbKj3A254T515sNIMcwsB8/2+2E63/zZrQzBkj0BrN/9Bexjpiks3jRhZatEsXn3dTy47R09Uix5WcJt+xzqZ7+ysyLKOOedS39Z7SDmsn2eA0FKtQpwA6LXeG2w+jxmw3oA8lVUgEf/rzeC/bByBNsO70aEFTd
diff --git a/src/lib/util/python/gen_wiredata.py.in b/src/lib/util/python/gen_wiredata.py.in
index d181f73..b3858b6 100755
--- a/src/lib/util/python/gen_wiredata.py.in
+++ b/src/lib/util/python/gen_wiredata.py.in
@@ -325,7 +325,7 @@ What you are expected to do is as follows:
   examples.
 """
 
-import configparser, re, time, socket, sys
+import configparser, re, time, socket, sys, base64
 from datetime import datetime
 from optparse import OptionParser
 
@@ -413,6 +413,11 @@ def encode_string(name, len=None):
         return '%0.*x' % (len * 2, name)
     return ''.join(['%02x' % ord(ch) for ch in name])
 
+def encode_bytes(name, len=None):
+    if type(name) is int and len is not None:
+        return '%0.*x' % (len * 2, name)
+    return ''.join(['%02x' % ch for ch in name])
+
 def count_namelabels(name):
     if name == '.':             # special case
         return 0
@@ -888,6 +893,42 @@ class AFSDB(RR):
         f.write('# SUBTYPE=%d SERVER=%s\n' % (self.subtype, self.server))
         f.write('%04x %s\n' % (self.subtype, server_wire))
 
+class DNSKEY(RR):
+    '''Implements rendering DNSKEY RDATA in the test data format.
+
+    Configurable parameters are as follows (see code below for the
+    default values):
+    - flags (16-bit int): The flags field.
+    - protocol (8-bit int): The protocol field.
+    - algorithm (8-bit int): The algorithm field.
+    - digest (string): The key digest field.
+    '''
+    flags = 257
+    protocol = 3
+    algorithm = 5
+    digest = 'AAECAwQFBgcICQoLDA0ODw=='
+
+    def dump(self, f):
+        decoded_digest = base64.b64decode(bytes(self.digest, 'ascii'))
+        if self.rdlen is None:
+            self.rdlen = 4 + len(decoded_digest)
+        else:
+            self.rdlen = int(self.rdlen)
+
+        self.dump_header(f, self.rdlen)
+
+        f.write('# FLAGS=%d\n' % (self.flags))
+        f.write('%04x\n' % (self.flags))
+
+        f.write('# PROTOCOL=%d\n' % (self.protocol))
+        f.write('%02x\n' % (self.protocol))
+
+        f.write('# ALGORITHM=%d\n' % (self.algorithm))
+        f.write('%02x\n' % (self.algorithm))
+
+        f.write('# DIGEST=%s\n' % (self.digest))
+        f.write('%s\n' % (encode_bytes(decoded_digest)))
+
 class NSECBASE(RR):
     '''Implements rendering NSEC/NSEC3 type bitmaps commonly used for
     these RRs.  The NSEC and NSEC3 classes will be inherited from this



More information about the bind10-changes mailing list