Browse Source

[master] Merge branch 'trac2389'

JINMEI Tatuya 12 years ago
parent
commit
66535ab575

+ 9 - 1
doc/differences.txt

@@ -1,7 +1,7 @@
 Differences of Bind 10 to other software
 Differences of Bind 10 to other software
 ========================================
 ========================================
 
 
-Bind 9
+BIND 9
 ------
 ------
 
 
 TODO: There are definitely more differences than just this.
 TODO: There are definitely more differences than just this.
@@ -10,3 +10,11 @@ TODO: There are definitely more differences than just this.
   received zone doesn't contain a NS record, bind 9 stops serving the
   received zone doesn't contain a NS record, bind 9 stops serving the
   zone and returns SERVFAIL to queries for that zone. Bind 10 still
   zone and returns SERVFAIL to queries for that zone. Bind 10 still
   uses the previous version of zone.
   uses the previous version of zone.
+
+RDATA implementations:
+* IN/A: BIND 10 does not accept abbreviated forms of textual IPv4
+  addresses for class-IN, type-A RDATA.  BIND 9 warns about it but
+  still accepts it as the standard inet_aton() function.  Such forms
+  should actually be NOT accepted per RFC 1035, but BIND 9 accepts them
+  probably because of compatibility reasons.  Until our strict
+  (and more correct) behavior causes operations issues, we'll keep it.

+ 1 - 1
src/lib/dns/gen-rdatacode.py.in

@@ -32,7 +32,7 @@ import sys
 #
 #
 # Example:
 # Example:
 #     new_rdata_factory_users = [('a', 'in'), ('a', 'ch'), ('soa', 'generic')]
 #     new_rdata_factory_users = [('a', 'in'), ('a', 'ch'), ('soa', 'generic')]
-new_rdata_factory_users = [('aaaa', 'in'),
+new_rdata_factory_users = [('a', 'in'), ('aaaa', 'in'),
                            ('cname', 'generic'),
                            ('cname', 'generic'),
                            ('dname', 'generic'),
                            ('dname', 'generic'),
                            ('hinfo', 'generic'),
                            ('hinfo', 'generic'),

+ 93 - 8
src/lib/dns/rdata/in_1/a_1.cc

@@ -15,6 +15,8 @@
 #include <stdint.h>
 #include <stdint.h>
 #include <string.h>
 #include <string.h>
 
 
+#include <cerrno>
+#include <cstring>
 #include <string>
 #include <string>
 
 
 #include <arpa/inet.h> // XXX: for inet_pton/ntop(), not exist in C++ standards
 #include <arpa/inet.h> // XXX: for inet_pton/ntop(), not exist in C++ standards
@@ -23,8 +25,11 @@
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
 #include <util/buffer.h>
 #include <util/buffer.h>
+
 #include <dns/exceptions.h>
 #include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
 #include <dns/messagerenderer.h>
+#include <dns/master_lexer.h>
+#include <dns/master_loader_callbacks.h>
 #include <dns/rdata.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rdataclass.h>
 
 
@@ -34,16 +39,91 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
 
-A::A(const std::string& addrstr) {
-    // RFC1035 states textual representation of IN/A RDATA is
-    // "four decimal numbers separated by dots without any embedded spaces".
-    // This is exactly what inet_pton() accepts for AF_INET.  In particular,
-    // it rejects an abbreviated form such as "10.1" meaning "10.0.0.1".
-    if (inet_pton(AF_INET, addrstr.c_str(), &addr_) != 1) {
+namespace {
+void
+convertToIPv4Addr(const char* src, size_t src_len, uint32_t* dst) {
+    // This check specifically rejects invalid input that begins with valid
+    // address text followed by a nul character (and possibly followed by
+    // further garbage).  It cannot be detected by inet_pton().
+    //
+    // Note that this is private subroutine of the in::A constructors, which
+    // pass std::string.size() or StringRegion::len as src_len, so it should
+    // be equal to strlen() unless there's an intermediate nul character.
+    if (src_len != strlen(src)) {
         isc_throw(InvalidRdataText,
         isc_throw(InvalidRdataText,
-                  "IN/A RDATA construction from text failed: Address cannot be "
-                  "converted: " << addrstr);
+                  "Bad IN/A RDATA text: unexpected nul in string: '"
+                  << src << "'");
     }
     }
+    const int result = inet_pton(AF_INET, src, dst);
+    if (result == 0) {
+        isc_throw(InvalidRdataText, "Bad IN/A RDATA text: '" << src << "'");
+    } else if (result < 0) {
+        isc_throw(isc::Unexpected,
+                  "Unexpected failure in parsing IN/A RDATA text: '"
+                  << src << "': " << std::strerror(errno));
+    }
+}
+}
+
+/// \brief Constructor from string.
+///
+/// The given string must be a valid textual representation of an IPv4
+/// address as specified in RFC1035, that is, four decimal numbers separated
+/// by dots without any embedded spaces.  Note that it excludes abbreviated
+/// forms such as "10.1" to mean "10.0.0.1".
+///
+/// Internally, this implementation uses the standard inet_pton() library
+/// function for the AF_INET family to parse and convert the textual
+/// representation.  While standard compliant implementations of this function
+/// should accept exactly what this constructor expects, specific
+/// implementation may behave differently, in which case this constructor
+/// will simply accept the result of inet_pton().  In any case, the user of
+/// the class shouldn't assume such specific implementation behavior of
+/// inet_pton().
+///
+/// No extra character should be contained in \c addrstr other than the
+/// textual address.  These include spaces and the nul character.
+///
+/// \throw InvalidRdata The text extracted by the lexer isn't recognized as
+/// a valid IPv4 address.
+/// \throw Unexpected Unexpected system error in conversion (this should be
+/// very rare).
+///
+/// \param addrstr Textual representation of IPv4 address to be used as the
+/// RDATA.
+A::A(const std::string& addrstr) {
+    convertToIPv4Addr(addrstr.c_str(), addrstr.size(), &addr_);
+}
+
+/// \brief Constructor with a context of MasterLexer.
+///
+/// The \c lexer should point to the beginning of valid textual representation
+/// of a class IN A RDATA.
+///
+/// The acceptable form of the textual address is generally the same as the
+/// string version of the constructor, but this version accepts beginning
+/// spaces and trailing spaces or other characters.  Trailing non space
+/// characters would be considered an invalid form in an RR representation,
+/// but handling such errors is not the responsibility of this constructor.
+/// It also accepts other unusual syntax that would be considered valid
+/// in the context of DNS master file; for example, it accepts an IPv4
+/// address surrounded by parentheses, such as "(192.0.2.1)", although it's
+/// very unlikely to be used for this type of RDATA.
+///
+/// \throw MasterLexer::LexerError General parsing error such as missing field.
+/// \throw InvalidRdata The text extracted by the lexer isn't recognized as
+/// a valid IPv4 address.
+/// \throw Unexpected Unexpected system error in conversion (this should be
+/// very rare).
+///
+/// \param lexer A \c MasterLexer object parsing a master file for the
+/// RDATA to be created
+A::A(MasterLexer& lexer, const Name*,
+     MasterLoader::Options, MasterLoaderCallbacks&)
+{
+    const MasterToken& token = lexer.getNextToken(MasterToken::STRING);
+    convertToIPv4Addr(token.getStringRegion().beg, token.getStringRegion().len,
+                      &addr_);
 }
 }
 
 
 A::A(InputBuffer& buffer, size_t rdata_len) {
 A::A(InputBuffer& buffer, size_t rdata_len) {
@@ -61,6 +141,7 @@ A::A(InputBuffer& buffer, size_t rdata_len) {
     buffer.readData(&addr_, sizeof(addr_));
     buffer.readData(&addr_, sizeof(addr_));
 }
 }
 
 
+/// \brief Copy constructor.
 A::A(const A& other) : Rdata(), addr_(other.addr_)
 A::A(const A& other) : Rdata(), addr_(other.addr_)
 {}
 {}
 
 
@@ -74,6 +155,7 @@ A::toWire(AbstractMessageRenderer& renderer) const {
     renderer.writeData(&addr_, sizeof(addr_));
     renderer.writeData(&addr_, sizeof(addr_));
 }
 }
 
 
+/// \brief Return a textual form of the underlying IPv4 address of the RDATA.
 string
 string
 A::toText() const {
 A::toText() const {
     char addr_string[sizeof("255.255.255.255")];
     char addr_string[sizeof("255.255.255.255")];
@@ -86,6 +168,9 @@ A::toText() const {
     return (addr_string);
     return (addr_string);
 }
 }
 
 
+/// \brief Compare two in::A RDATAs.
+///
+/// In effect, it compares the two RDATA as an unsigned 32-bit integer.
 int
 int
 A::compare(const Rdata& other) const {
 A::compare(const Rdata& other) const {
     const A& other_a = dynamic_cast<const A&>(other);
     const A& other_a = dynamic_cast<const A&>(other);

+ 64 - 9
src/lib/dns/rdata/in_1/aaaa_28.cc

@@ -24,6 +24,8 @@
 #include <stdint.h>
 #include <stdint.h>
 #include <string.h>
 #include <string.h>
 
 
+#include <cerrno>
+#include <cstring>
 #include <string>
 #include <string>
 
 
 #include <arpa/inet.h> // XXX: for inet_pton/ntop(), not exist in C++ standards
 #include <arpa/inet.h> // XXX: for inet_pton/ntop(), not exist in C++ standards
@@ -35,24 +37,72 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
 
-AAAA::AAAA(const std::string& addrstr) {
-    if (inet_pton(AF_INET6, addrstr.c_str(), &addr_) != 1) {
+namespace {
+void
+convertToIPv6Addr(const char* src, size_t src_len, void* dst) {
+    // See a_1.cc for this check.
+    if (src_len != strlen(src)) {
         isc_throw(InvalidRdataText,
         isc_throw(InvalidRdataText,
-                  "IN/AAAA RDATA construction from text failed: "
-                  "Address cannot be converted: " << addrstr);
+                  "Bad IN/AAAA RDATA text: unexpected nul in string: '"
+                  << src << "'");
     }
     }
+    const int result = inet_pton(AF_INET6, src, dst);
+    if (result == 0) {
+        isc_throw(InvalidRdataText, "Bad IN/AAAA RDATA text: '" << src << "'");
+    } else if (result < 0) {
+        isc_throw(isc::Unexpected,
+                  "Unexpected failure in parsing IN/AAAA RDATA text: '"
+                  << src << "': " << std::strerror(errno));
+    }
+}
 }
 }
 
 
+/// \brief Constructor from string.
+///
+/// The given string must be a valid textual representation of an IPv6
+/// address as specified in RFC1886.
+///
+/// No extra character should be contained in \c addrstr other than the
+/// textual address.  These include spaces and the nul character.
+///
+/// \throw InvalidRdata The text extracted by the lexer isn't recognized as
+/// a valid IPv6 address.
+/// \throw Unexpected Unexpected system error in conversion (this should be
+/// very rare).
+///
+/// \param addrstr Textual representation of IPv6 address to be used as the
+/// RDATA.
+AAAA::AAAA(const std::string& addrstr) {
+    convertToIPv6Addr(addrstr.c_str(), addrstr.size(), addr_);
+}
+
+/// \brief Constructor with a context of MasterLexer.
+///
+/// The \c lexer should point to the beginning of valid textual representation
+/// of a class IN AAAA RDATA.
+///
+/// The acceptable form of the textual address is generally the same as the
+/// string version of the constructor, but this version is slightly more
+/// flexible.  See the similar constructor of \c in::A class; the same
+/// notes apply here.
+///
+/// \throw MasterLexer::LexerError General parsing error such as missing field.
+/// \throw InvalidRdata The text extracted by the lexer isn't recognized as
+/// a valid IPv6 address.
+/// \throw Unexpected Unexpected system error in conversion (this should be
+/// very rare).
+///
+/// \param lexer A \c MasterLexer object parsing a master file for the
+/// RDATA to be created
 AAAA::AAAA(MasterLexer& lexer, const Name*,
 AAAA::AAAA(MasterLexer& lexer, const Name*,
            MasterLoader::Options, MasterLoaderCallbacks&)
            MasterLoader::Options, MasterLoaderCallbacks&)
 {
 {
     const MasterToken& token = lexer.getNextToken(MasterToken::STRING);
     const MasterToken& token = lexer.getNextToken(MasterToken::STRING);
-    if (inet_pton(AF_INET6, token.getStringRegion().beg, &addr_) != 1) {
-        isc_throw(InvalidRdataText, "Failed to convert '"
-                  << token.getString() << "' to IN/AAAA RDATA");
-    }
+    convertToIPv6Addr(token.getStringRegion().beg, token.getStringRegion().len,
+                      addr_);
 }
 }
 
 
+/// \brief Copy constructor.
 AAAA::AAAA(InputBuffer& buffer, size_t rdata_len) {
 AAAA::AAAA(InputBuffer& buffer, size_t rdata_len) {
     if (rdata_len != sizeof(addr_)) {
     if (rdata_len != sizeof(addr_)) {
         isc_throw(DNSMessageFORMERR,
         isc_throw(DNSMessageFORMERR,
@@ -72,6 +122,7 @@ AAAA::AAAA(const AAAA& other) : Rdata() {
     memcpy(addr_, other.addr_, sizeof(addr_));
     memcpy(addr_, other.addr_, sizeof(addr_));
 }
 }
 
 
+/// \brief Return a textual form of the underlying IPv6 address of the RDATA.
 void
 void
 AAAA::toWire(OutputBuffer& buffer) const {
 AAAA::toWire(OutputBuffer& buffer) const {
     buffer.writeData(&addr_, sizeof(addr_));
     buffer.writeData(&addr_, sizeof(addr_));
@@ -86,7 +137,8 @@ string
 AAAA::toText() const {
 AAAA::toText() const {
     char addr_string[sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")];
     char addr_string[sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")];
 
 
-    if (inet_ntop(AF_INET6, &addr_, addr_string, sizeof(addr_string)) == NULL) {
+    if (inet_ntop(AF_INET6, &addr_, addr_string, sizeof(addr_string))
+        == NULL) {
         isc_throw(Unexpected,
         isc_throw(Unexpected,
                   "Failed to convert IN/AAAA RDATA to textual IPv6 address");
                   "Failed to convert IN/AAAA RDATA to textual IPv6 address");
     }
     }
@@ -94,6 +146,9 @@ AAAA::toText() const {
     return (string(addr_string));
     return (string(addr_string));
 }
 }
 
 
+/// \brief Compare two in::AAAA RDATAs.
+///
+/// In effect, it compares the two RDATA as an unsigned 128-bit integer.
 int
 int
 AAAA::compare(const Rdata& other) const {
 AAAA::compare(const Rdata& other) const {
     const AAAA& other_a = dynamic_cast<const AAAA&>(other);
     const AAAA& other_a = dynamic_cast<const AAAA&>(other);

+ 9 - 9
src/lib/dns/tests/master_loader_unittest.cc

@@ -419,16 +419,16 @@ struct ErrorCase {
 
 
     // Parameter ordering errors
     // Parameter ordering errors
     { "www      IN      A   3600 192.168.2.7",
     { "www      IN      A   3600 192.168.2.7",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: Bad IN/A RDATA text: '3600'",
       "Incorrect order of class, TTL and type" },
       "Incorrect order of class, TTL and type" },
     { "www      A       IN  3600 192.168.2.8",
     { "www      A       IN  3600 192.168.2.8",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: Bad IN/A RDATA text: 'IN'",
       "Incorrect order of class, TTL and type" },
       "Incorrect order of class, TTL and type" },
     { "www      3600    A   IN   192.168.2.7",
     { "www      3600    A   IN   192.168.2.7",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: Bad IN/A RDATA text: 'IN'",
       "Incorrect order of class, TTL and type" },
       "Incorrect order of class, TTL and type" },
     { "www      A       3600 IN  192.168.2.8",
     { "www      A       3600 IN  192.168.2.8",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: Bad IN/A RDATA text: '3600'",
       "Incorrect order of class, TTL and type" },
       "Incorrect order of class, TTL and type" },
 
 
     // Missing type and Rdata
     // Missing type and Rdata
@@ -440,19 +440,19 @@ struct ErrorCase {
 
 
     // Missing Rdata
     // Missing Rdata
     { "www A",
     { "www A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
       "Missing Rdata" },
     { "www 3600 A",
     { "www 3600 A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
       "Missing Rdata" },
     { "www IN A",
     { "www IN A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
       "Missing Rdata" },
     { "www 3600 IN A",
     { "www 3600 IN A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
       "Missing Rdata" },
     { "www IN 3600 A",
     { "www IN 3600 A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
       "Missing Rdata" },
 
 
     { "www      3600    IN", NULL, "Unexpected EOLN" },
     { "www      3600    IN", NULL, "Unexpected EOLN" },

+ 54 - 17
src/lib/dns/tests/rdata_in_a_unittest.cc

@@ -12,11 +12,14 @@
 // 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 <dns/rdataclass.h>
+
 #include <util/buffer.h>
 #include <util/buffer.h>
 #include <dns/exceptions.h>
 #include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
 #include <dns/messagerenderer.h>
+#include <dns/master_lexer.h>
+#include <dns/master_loader.h>
 #include <dns/rdata.h>
 #include <dns/rdata.h>
-#include <dns/rdataclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 #include <dns/rrtype.h>
 
 
@@ -25,6 +28,8 @@
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/rdata_unittest.h>
 #include <dns/tests/rdata_unittest.h>
 
 
+#include <sstream>
+
 using isc::UnitTestUtil;
 using isc::UnitTestUtil;
 using namespace std;
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns;
@@ -33,20 +38,55 @@ using namespace isc::dns::rdata;
 
 
 namespace {
 namespace {
 class Rdata_IN_A_Test : public RdataTest {
 class Rdata_IN_A_Test : public RdataTest {
-    // there's nothing to specialize
+protected:
+    Rdata_IN_A_Test() : rdata_in_a("192.0.2.1") {}
+
+    void checkFromTextIN_A(const std::string& rdata_txt,
+                       bool throw_str_version = true,
+                       bool throw_lexer_version = true) {
+        checkFromText<in::A, InvalidRdataText, InvalidRdataText>(
+            rdata_txt, rdata_in_a, throw_str_version, throw_lexer_version);
+    }
+
+    const in::A rdata_in_a;
 };
 };
 
 
-const in::A rdata_in_a("192.0.2.1");
 const uint8_t wiredata_in_a[] = { 192, 0, 2, 1 };
 const uint8_t wiredata_in_a[] = { 192, 0, 2, 1 };
 
 
 TEST_F(Rdata_IN_A_Test, createFromText) {
 TEST_F(Rdata_IN_A_Test, createFromText) {
-    EXPECT_EQ(0, rdata_in_a.compare(in::A("192.0.2.1")));
+    // Normal case: no exception for either case, so the exception type
+    // doesn't matter.
+    checkFromText<in::A, isc::Exception, isc::Exception>("192.0.2.1",
+                                                         rdata_in_a, false,
+                                                         false);
+
     // should reject an abbreviated form of IPv4 address
     // should reject an abbreviated form of IPv4 address
-    EXPECT_THROW(in::A("10.1"), InvalidRdataText);
+    checkFromTextIN_A("10.1");
     // or an IPv6 address
     // or an IPv6 address
-    EXPECT_THROW(in::A("2001:db8::1234"), InvalidRdataText);
+    checkFromTextIN_A("2001:db8::1234");
     // or any meaningless text as an IP address
     // or any meaningless text as an IP address
-    EXPECT_THROW(in::A("xxx"), InvalidRdataText);
+    checkFromTextIN_A("xxx");
+
+    // trailing white space: only string version throws
+    checkFromTextIN_A("192.0.2.1  ", true, false);
+    // same for beginning white space.
+    checkFromTextIN_A("  192.0.2.1", true, false);
+    // same for trailing non-space garbage (note that lexer version still
+    // ignore it; it's expected to be detected at a higher layer).
+    checkFromTextIN_A("192.0.2.1 xxx", true, false);
+
+    // nul character after a valid textual representation.
+    string nul_after_addr = "192.0.2.1";
+    nul_after_addr.push_back(0);
+    checkFromTextIN_A(nul_after_addr, true, true);
+
+    // a valid address surrounded by parentheses; only okay with lexer
+    checkFromTextIN_A("(192.0.2.1)", true, false);
+
+    // input that would cause lexer-specific error; it's bad text as an
+    // address so should result in the string version, too.
+    checkFromText<in::A, InvalidRdataText, MasterLexer::LexerError>(
+        ")192.0.2.1", rdata_in_a);
 }
 }
 
 
 TEST_F(Rdata_IN_A_Test, createFromWire) {
 TEST_F(Rdata_IN_A_Test, createFromWire) {
@@ -68,11 +108,6 @@ TEST_F(Rdata_IN_A_Test, createFromWire) {
                  DNSMessageFORMERR);
                  DNSMessageFORMERR);
 }
 }
 
 
-TEST_F(Rdata_IN_A_Test, createFromLexer) {
-    EXPECT_EQ(0, rdata_in_a.compare(
-        *test::createRdataUsingLexer(RRType::A(), RRClass::IN(), "192.0.2.1")));
-}
-
 TEST_F(Rdata_IN_A_Test, toWireBuffer) {
 TEST_F(Rdata_IN_A_Test, toWireBuffer) {
     rdata_in_a.toWire(obuffer);
     rdata_in_a.toWire(obuffer);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
@@ -89,15 +124,17 @@ TEST_F(Rdata_IN_A_Test, toWireRenderer) {
 
 
 TEST_F(Rdata_IN_A_Test, toText) {
 TEST_F(Rdata_IN_A_Test, toText) {
     EXPECT_EQ("192.0.2.1", rdata_in_a.toText());
     EXPECT_EQ("192.0.2.1", rdata_in_a.toText());
-    string longaddr("255.255.255.255"); // this shouldn't make the code crash
+
+    // this shouldn't make the code crash
+    const string longaddr("255.255.255.255");
     EXPECT_EQ(longaddr, in::A(longaddr).toText());
     EXPECT_EQ(longaddr, in::A(longaddr).toText());
 }
 }
 
 
 TEST_F(Rdata_IN_A_Test, compare) {
 TEST_F(Rdata_IN_A_Test, compare) {
-    in::A small1("1.1.1.1");
-    in::A small2("1.2.3.4");
-    in::A large1("255.255.255.255");
-    in::A large2("4.3.2.1");
+    const in::A small1("1.1.1.1");
+    const in::A small2("1.2.3.4");
+    const in::A large1("255.255.255.255");
+    const in::A large2("4.3.2.1");
 
 
     // trivial case: self equivalence
     // trivial case: self equivalence
     // cppcheck-suppress uselessCallsCompare
     // cppcheck-suppress uselessCallsCompare

+ 48 - 5
src/lib/dns/tests/rdata_in_aaaa_unittest.cc

@@ -33,18 +33,61 @@ using namespace isc::dns::rdata;
 
 
 namespace {
 namespace {
 class Rdata_IN_AAAA_Test : public RdataTest {
 class Rdata_IN_AAAA_Test : public RdataTest {
-    // there's nothing to specialize
+protected:
+    Rdata_IN_AAAA_Test() : rdata_in_aaaa("2001:db8::1234") {}
+
+    // Common check to see the result of in::A Rdata construction either from
+    // std::string or with MasterLexer object.  If it's expected to succeed
+    // the result should be identical to the commonly used test data
+    // (rdata_in_a); otherwise it should result in the exception specified as
+    // the template parameter.
+    void checkFromTextIN_AAAA(const string& in_aaaa_txt,
+                              bool throw_str_version = true,
+                              bool throw_lexer_version = true)
+    {
+        checkFromText<in::AAAA, InvalidRdataText, InvalidRdataText>(
+            in_aaaa_txt, rdata_in_aaaa, throw_str_version,
+            throw_lexer_version);
+    }
+
+    const in::AAAA rdata_in_aaaa;
 };
 };
 
 
-const in::AAAA rdata_in_aaaa("2001:db8::1234");
 const uint8_t wiredata_in_aaaa[] = {
 const uint8_t wiredata_in_aaaa[] = {
     0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x12, 0x34 };
     0x00, 0x00, 0x12, 0x34 };
 
 
 TEST_F(Rdata_IN_AAAA_Test, createFromText) {
 TEST_F(Rdata_IN_AAAA_Test, createFromText) {
-    rdata_in_aaaa.compare(in::AAAA(string("2001:db8::1234")));
-    EXPECT_THROW(in::AAAA("192.0.2.1"), InvalidRdataText);
-    EXPECT_THROW(in::AAAA("xxx"), InvalidRdataText);
+    // Normal case: no exception for either case, so the exception type
+    // doesn't matter.
+    checkFromText<in::AAAA, isc::Exception, isc::Exception>(
+        "2001:db8::1234", rdata_in_aaaa, false, false);
+
+    // should reject an IP4 address.
+    checkFromTextIN_AAAA("192.0.2.1");
+    // or any meaningless text as an IPv6 address
+    checkFromTextIN_AAAA("xxx");
+
+    // trailing white space: only string version throws
+    checkFromTextIN_AAAA("2001:db8::1234  ", true, false);
+    // same for beginning white space.
+    checkFromTextIN_AAAA("  2001:db8::1234", true, false);
+    // same for trailing non-space garbage (note that lexer version still
+    // ignore it; it's expected to be detected at a higher layer).
+    checkFromTextIN_AAAA("2001:db8::1234 xxx", true, false);
+
+    // nul character after a valid textual representation.
+    string nul_after_addr = "2001:db8::1234";
+    nul_after_addr.push_back(0);
+    checkFromTextIN_AAAA(nul_after_addr, true, true);
+
+    // a valid address surrounded by parentheses; only okay with lexer
+    checkFromTextIN_AAAA("(2001:db8::1234)", true, false);
+
+    // input that would cause lexer-specific error; it's bad text as an
+    // address so should result in the string version, too.
+    checkFromText<in::AAAA, InvalidRdataText, MasterLexer::LexerError>(
+        ")2001:db8::1234", rdata_in_aaaa);
 }
 }
 
 
 TEST_F(Rdata_IN_AAAA_Test, createFromWire) {
 TEST_F(Rdata_IN_AAAA_Test, createFromWire) {

+ 63 - 61
src/lib/dns/tests/rdata_soa_unittest.cc

@@ -39,123 +39,125 @@ protected:
                   2010012601, 3600, 300, 3600000, 1200)
                   2010012601, 3600, 300, 3600000, 1200)
     {}
     {}
 
 
-    // Common check to see if the given text can be used to construct SOA
-    // Rdata that is identical rdata_soa.
-    void checkFromText(const char* soa_txt, const Name* origin = NULL) {
-        std::stringstream ss(soa_txt);
-        MasterLexer lexer;
-        lexer.pushSource(ss);
-
-        if (origin == NULL) {
-            // from-string constructor works correctly only when origin
-            // is NULL (by its nature).
-            EXPECT_EQ(0, generic::SOA(soa_txt).compare(rdata_soa));
-        }
-        EXPECT_EQ(0, generic::SOA(lexer, origin, MasterLoader::DEFAULT,
-                                  loader_cb).compare(rdata_soa));
-    }
-
-    // Common check if given text (which is invalid as SOA RDATA) is rejected
-    // with the specified type of exception: ExForString is the expected
-    // exception for the "from string" constructor; ExForLexer is for the
-    // constructor with master lexer.
     template <typename ExForString, typename ExForLexer>
     template <typename ExForString, typename ExForLexer>
-    void checkFromBadTexxt(const char* soa_txt, const Name* origin = NULL) {
-        EXPECT_THROW(generic::SOA soa(soa_txt), ExForString);
-
-        std::stringstream ss(soa_txt);
-        MasterLexer lexer;
-        lexer.pushSource(ss);
-        EXPECT_THROW(generic::SOA soa(lexer, origin, MasterLoader::DEFAULT,
-                                      loader_cb), ExForLexer);
+    void checkFromTextSOA(const string& soa_txt, const Name* origin = NULL,
+                          bool throw_str_version = true,
+                          bool throw_lexer_version = true)
+    {
+        checkFromText<generic::SOA, ExForString, ExForLexer>(
+            soa_txt, rdata_soa, throw_str_version, throw_lexer_version,
+            origin);
     }
     }
 
 
     const generic::SOA rdata_soa;
     const generic::SOA rdata_soa;
 };
 };
 
 
 TEST_F(Rdata_SOA_Test, createFromText) {
 TEST_F(Rdata_SOA_Test, createFromText) {
+    // Below we specify isc::Exception as a dummy value for the exception type
+    // in case it's not expected to throw an exception; the type isn't used
+    // in the check code.
+
     // A simple case.
     // A simple case.
-    checkFromText("ns.example.com. root.example.com. "
-                  "2010012601 3600 300 3600000 1200");
+    checkFromTextSOA<isc::Exception, isc::Exception>(
+        "ns.example.com. root.example.com. 2010012601 3600 300 3600000 1200",
+        NULL, false, false);
 
 
     // Beginning and trailing space are ignored.
     // Beginning and trailing space are ignored.
-    checkFromText("  ns.example.com. root.example.com. "
-                  "2010012601 3600 300 3600000 1200  ");
+    checkFromTextSOA<isc::Exception, isc::Exception>(
+        "  ns.example.com. root.example.com. "
+        "2010012601 3600 300 3600000 1200  ", NULL, false, false);
 
 
     // using extended TTL-like form for some parameters.
     // using extended TTL-like form for some parameters.
-    checkFromText("ns.example.com. root.example.com. "
-                  "2010012601 1H 5M 1000H 20M");
+    checkFromTextSOA<isc::Exception, isc::Exception>(
+        "ns.example.com. root.example.com. 2010012601 1H 5M 1000H 20M",
+        NULL, false, false);
 
 
     // multi-line.
     // multi-line.
-    checkFromText("ns.example.com. (root.example.com.\n"
-                  "2010012601 1H 5M 1000H) 20M");
+    checkFromTextSOA<isc::Exception, isc::Exception>(
+        "ns.example.com. (root.example.com.\n"
+        "2010012601 1H 5M 1000H) 20M", NULL, false, false);
 
 
     // relative names for MNAME and RNAME with a separate origin (lexer
     // relative names for MNAME and RNAME with a separate origin (lexer
     // version only)
     // version only)
     const Name origin("example.com");
     const Name origin("example.com");
-    checkFromText("ns root 2010012601 1H 5M 1000H 20M", &origin);
+    checkFromTextSOA<MissingNameOrigin, isc::Exception>(
+        "ns root 2010012601 1H 5M 1000H 20M", &origin, true, false);
 
 
-    // with the '@' notation with a separate origin (lexer version only)
+    // with the '@' notation with a separate origin (lexer version only;
+    // string version would throw)
     const Name full_mname("ns.example.com");
     const Name full_mname("ns.example.com");
-    checkFromText("@ root.example.com. 2010012601 1H 5M 1000H 20M",
-                  &full_mname);
+    checkFromTextSOA<MissingNameOrigin, isc::Exception>(
+        "@ root.example.com. 2010012601 1H 5M 1000H 20M", &full_mname, true,
+        false);
 
 
     // bad MNAME/RNAMEs
     // bad MNAME/RNAMEs
-    checkFromBadTexxt<EmptyLabel, EmptyLabel>(
+    checkFromTextSOA<EmptyLabel, EmptyLabel>(
         "bad..example. . 2010012601 1H 5M 1000H 20M");
         "bad..example. . 2010012601 1H 5M 1000H 20M");
-    checkFromBadTexxt<EmptyLabel, EmptyLabel>(
+    checkFromTextSOA<EmptyLabel, EmptyLabel>(
         ". bad..example. 2010012601 1H 5M 1000H 20M");
         ". bad..example. 2010012601 1H 5M 1000H 20M");
 
 
     // Names shouldn't be quoted. (Note: on completion of #2534, the resulting
     // Names shouldn't be quoted. (Note: on completion of #2534, the resulting
     // exception will be different).
     // exception will be different).
-    checkFromBadTexxt<MissingNameOrigin, MissingNameOrigin>(
+    checkFromTextSOA<MissingNameOrigin, MissingNameOrigin>(
         "\".\" . 0 0 0 0 0");
         "\".\" . 0 0 0 0 0");
-    checkFromBadTexxt<MissingNameOrigin, MissingNameOrigin>(
+    checkFromTextSOA<MissingNameOrigin, MissingNameOrigin>(
         ". \".\" 0 0 0 0 0");
         ". \".\" 0 0 0 0 0");
 
 
     // Missing MAME or RNAME: for the string version, the serial would be
     // Missing MAME or RNAME: for the string version, the serial would be
     // tried as RNAME and result in "not absolute".  For the lexer version,
     // tried as RNAME and result in "not absolute".  For the lexer version,
     // it reaches the end-of-line, missing min TTL.
     // it reaches the end-of-line, missing min TTL.
-    checkFromBadTexxt<MissingNameOrigin, MasterLexer::LexerError>(
+    checkFromTextSOA<MissingNameOrigin, MasterLexer::LexerError>(
         ". 2010012601 0 0 0 0", &Name::ROOT_NAME());
         ". 2010012601 0 0 0 0", &Name::ROOT_NAME());
 
 
     // bad serial.  the string version converts lexer error to
     // bad serial.  the string version converts lexer error to
     // InvalidRdataText.
     // InvalidRdataText.
-    checkFromBadTexxt<InvalidRdataText, MasterLexer::LexerError>(
+    checkFromTextSOA<InvalidRdataText, MasterLexer::LexerError>(
         ". . bad 0 0 0 0");
         ". . bad 0 0 0 0");
 
 
     // bad serial; exceeding the uint32_t range (4294967296 = 2^32)
     // bad serial; exceeding the uint32_t range (4294967296 = 2^32)
-    checkFromBadTexxt<InvalidRdataText, MasterLexer::LexerError>(
+    checkFromTextSOA<InvalidRdataText, MasterLexer::LexerError>(
         ". . 4294967296 0 0 0 0");
         ". . 4294967296 0 0 0 0");
 
 
     // Bad format for other numeric parameters.  These will be tried as a TTL,
     // Bad format for other numeric parameters.  These will be tried as a TTL,
     // and result in an exception there.
     // and result in an exception there.
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(". . 2010012601 bad 0 0 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
+        ". . 2010012601 bad 0 0 0");
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
         ". . 2010012601 4294967296 0 0 0");
         ". . 2010012601 4294967296 0 0 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(". . 2010012601 0 bad 0 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
+        ". . 2010012601 0 bad 0 0");
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
         ". . 2010012601 0 4294967296 0 0");
         ". . 2010012601 0 4294967296 0 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(". . 2010012601 0 0 bad 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
+        ". . 2010012601 0 0 bad 0");
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
         ". . 2010012601 0 0 4294967296 0");
         ". . 2010012601 0 0 4294967296 0");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(". . 2010012601 0 0 0 bad");
-    checkFromBadTexxt<InvalidRRTTL, InvalidRRTTL>(
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
+        ". . 2010012601 0 0 0 bad");
+    checkFromTextSOA<InvalidRRTTL, InvalidRRTTL>(
         ". . 2010012601 0 0 0 4294967296");
         ". . 2010012601 0 0 0 4294967296");
 
 
     // No space between RNAME and serial.  This case is the same as missing
     // No space between RNAME and serial.  This case is the same as missing
     // M/RNAME.
     // M/RNAME.
-    checkFromBadTexxt<MissingNameOrigin, MasterLexer::LexerError>(
+    checkFromTextSOA<MissingNameOrigin, MasterLexer::LexerError>(
         ". example.0 0 0 0 0", &Name::ROOT_NAME());
         ". example.0 0 0 0 0", &Name::ROOT_NAME());
 
 
     // Extra parameter.  string version immediately detects the error.
     // Extra parameter.  string version immediately detects the error.
-    EXPECT_THROW(generic::SOA soa(". . 0 0 0 0 0 extra"), InvalidRdataText);
-    // Likewise.  Redundant newline is also considered an error.
-    EXPECT_THROW(generic::SOA soa(". . 0 0 0 0 0\n"), InvalidRdataText);
-    EXPECT_THROW(generic::SOA soa("\n. . 0 0 0 0 0"), InvalidRdataText);
     // lexer version defers the check to the upper layer (we pass origin
     // lexer version defers the check to the upper layer (we pass origin
     // to skip the check with the string version).
     // to skip the check with the string version).
-    checkFromText("ns root 2010012601 1H 5M 1000H 20M extra", &origin);
+    checkFromTextSOA<InvalidRdataText, isc::Exception>(
+        "ns.example.com. root.example.com. 2010012601 1H 5M 1000H 20M "
+        "extra", &origin, true, false);
+
+    // Likewise.  Redundant newline is also considered an error.  The lexer
+    // version accepts trailing newline, but not the beginning one (where
+    // the lexer expects a string excluding newline and EOF).
+    checkFromTextSOA<InvalidRdataText, isc::Exception>(
+        "ns.example.com. root.example.com. 2010012601 1H 5M 1000H 20M\n",
+        NULL, true, false);
+    checkFromTextSOA<InvalidRdataText, MasterLexer::LexerError>(
+        "\nns.example.com. root.example.com. 2010012601 1H 5M 1000H 20M",
+        NULL, true, true);
 }
 }
 
 
 TEST_F(Rdata_SOA_Test, createFromWire) {
 TEST_F(Rdata_SOA_Test, createFromWire) {

+ 2 - 2
src/lib/dns/tests/rdata_unittest.cc

@@ -197,8 +197,8 @@ TEST_F(RdataTest, createRdataWithLexer) {
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
     EXPECT_FALSE(createRdata(RRType::AAAA(), RRClass::IN(), lexer, NULL,
                              MasterLoader::MANY_ERRORS, callbacks));
                              MasterLoader::MANY_ERRORS, callbacks));
     callback.check(src_name, line, CreateRdataCallback::ERROR,
     callback.check(src_name, line, CreateRdataCallback::ERROR,
-                   "createRdata from text failed: Failed to convert "
-                   "'192.0.2.1' to IN/AAAA RDATA");
+                   "createRdata from text failed: Bad IN/AAAA RDATA text: "
+                   "'192.0.2.1'");
 
 
     // Input is valid and parse will succeed, but with a warning that the
     // Input is valid and parse will succeed, but with a warning that the
     // file is not ended with a newline.
     // file is not ended with a newline.

+ 41 - 0
src/lib/dns/tests/rdata_unittest.h

@@ -24,6 +24,9 @@
 
 
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 
 
+#include <string>
+#include <sstream>
+
 using namespace isc::util;
 using namespace isc::util;
 
 
 namespace isc {
 namespace isc {
@@ -36,6 +39,44 @@ protected:
                                          const RRClass& rrclass,
                                          const RRClass& rrclass,
                                          const char* datafile,
                                          const char* datafile,
                                          size_t position = 0);
                                          size_t position = 0);
+
+    // Common check to see the result of Rdata construction of given type
+    // (template parameter RdataType) either from std::string or with
+    // MasterLexer object.  If it's expected to succeed the result should be
+    // identical to the commonly used test data (rdata_expected); otherwise it
+    // should result in the exception specified as the template parameter:
+    // ExForString for the string version, and ExForLexer for the lexer
+    // version.  throw_str_version and throw_lexer_version are set to true
+    // iff the string/lexer version is expected to throw, respectively.
+    // Parameter origin can be set to non NULL for the origin parameter of
+    // the lexer version of Rdata constructor.
+    template <typename RdataType, typename ExForString, typename ExForLexer>
+    void checkFromText(const std::string& rdata_txt,
+                       const RdataType& rdata_expected,
+                       bool throw_str_version = true,
+                       bool throw_lexer_version = true,
+                       const Name* origin = NULL)
+    {
+        SCOPED_TRACE(rdata_txt);
+
+        if (throw_str_version) {
+            EXPECT_THROW(RdataType rdata(rdata_txt), ExForString);
+        } else {
+            EXPECT_EQ(0, RdataType(rdata_txt).compare(rdata_expected));
+        }
+
+        std::stringstream ss(rdata_txt);
+        MasterLexer lexer;
+        lexer.pushSource(ss);
+        if (throw_lexer_version) {
+            EXPECT_THROW(RdataType rdata(lexer, origin, MasterLoader::DEFAULT,
+                                         loader_cb), ExForLexer);
+        } else {
+            EXPECT_EQ(0, RdataType(lexer, origin, MasterLoader::DEFAULT,
+                                   loader_cb).compare(rdata_expected));
+        }
+    }
+
     OutputBuffer obuffer;
     OutputBuffer obuffer;
     MessageRenderer renderer;
     MessageRenderer renderer;
     /// This is an RDATA object of some "unknown" RR type so that it can be
     /// This is an RDATA object of some "unknown" RR type so that it can be