[2389] use MasterLexer for IN/A RDATA constructor.

more tests and doc were added, and removed the createFromLexer test
(it doesn't make sense any more).  some other cleanups were also made.

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

@@ -32,7 +32,7 @@ import sys
 #
 # Example:
 #     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'),
                            ('dname', 'generic'),
                            ('hinfo', 'generic'),

src/lib/dns/rdata/in_1/a_1.cc

@@ -15,6 +15,8 @@
 #include <stdint.h>
 #include <string.h>
 
+#include <cerrno>
+#include <cstring>
 #include <string>
 
 #include <arpa/inet.h> // XXX: for inet_pton/ntop(), not exist in C++ standards
@@ -23,8 +25,11 @@
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
+
 #include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
+#include <dns/master_lexer.h>
+#include <dns/master_loader_callbacks.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 
@@ -34,16 +39,83 @@ using namespace isc::util;
 // BEGIN_ISC_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) {
-        isc_throw(InvalidRdataText,
-                  "IN/A RDATA construction from text failed: Address cannot be "
-                  "converted: " << addrstr);
+namespace {
+void
+convertToIPv4Addr(const char* src, size_t src_len, uint32_t* dst) {
+    if (src_len != strlen(src)) {
+        isc_throw(InvalidRdataText, "Bad IN/A RDATA text: extra character: '"
+                  << 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) {
@@ -61,6 +133,7 @@ A::A(InputBuffer& buffer, size_t rdata_len) {
     buffer.readData(&addr_, sizeof(addr_));
 }
 
+/// \brief Copy constructor.
 A::A(const A& other) : Rdata(), addr_(other.addr_)
 {}
 
@@ -74,6 +147,7 @@ A::toWire(AbstractMessageRenderer& renderer) const {
     renderer.writeData(&addr_, sizeof(addr_));
 }
 
+/// \brief Return a textual form of the underlying IPv4 address of the RDATA.
 string
 A::toText() const {
     char addr_string[sizeof("255.255.255.255")];
@@ -86,6 +160,9 @@ A::toText() const {
     return (addr_string);
 }
 
+/// \brief Compare two in::A RDATAs.
+///
+/// In effect, it compares the two RDATA as an unsigned 32-bit integer.
 int
 A::compare(const Rdata& other) const {
     const A& other_a = dynamic_cast<const A&>(other);

src/lib/dns/tests/master_loader_unittest.cc

@@ -419,16 +419,16 @@ struct ErrorCase {
 
     // Parameter ordering errors
     { "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" },
     { "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" },
     { "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" },
     { "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" },
 
     // Missing type and Rdata
@@ -440,19 +440,19 @@ struct ErrorCase {
 
     // Missing Rdata
     { "www A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
     { "www 3600 A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
     { "www IN A",
-      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "createRdata from text failed: unexpected end of input",
       "Missing Rdata" },
     { "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" },
     { "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" },
 
     { "www      3600    IN", NULL, "Unexpected EOLN" },

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
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <dns/rdataclass.h>
+
 #include <util/buffer.h>
 #include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
+#include <dns/master_lexer.h>
+#include <dns/master_loader.h>
 #include <dns/rdata.h>
-#include <dns/rdataclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 
@@ -25,6 +28,8 @@
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/rdata_unittest.h>
 
+#include <sstream>
+
 using isc::UnitTestUtil;
 using namespace std;
 using namespace isc::dns;
@@ -33,20 +38,74 @@ using namespace isc::dns::rdata;
 
 namespace {
 class Rdata_IN_A_Test : public RdataTest {
-    // there's nothing to specialize
+protected:
+    Rdata_IN_A_Test() : rdata_in_a("192.0.2.1") {}
+
+    // 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.
+    template <typename ExForString, typename ExForLexer>
+    void checkFromText(const string& in_a_txt,
+                       bool throw_str_version = true,
+                       bool throw_lexer_version = true)
+    {
+        if (throw_str_version) {
+            EXPECT_THROW(in::A in_a(in_a_txt), ExForString);
+        } else {
+            EXPECT_EQ(0, in::A(in_a_txt).compare(rdata_in_a));
+        }
+
+        std::stringstream ss(in_a_txt);
+        MasterLexer lexer;
+        lexer.pushSource(ss);
+        if (throw_lexer_version) {
+            EXPECT_THROW(in::A soa(lexer, NULL, MasterLoader::DEFAULT,
+                                   loader_cb), ExForLexer);
+        } else {
+                EXPECT_EQ(0, in::A(lexer, NULL, MasterLoader::DEFAULT,
+                           loader_cb).compare(rdata_in_a));
+        }
+    }
+
+    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 };
 
 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<isc::Exception, isc::Exception>("192.0.2.1", false, false);
+
     // should reject an abbreviated form of IPv4 address
-    EXPECT_THROW(in::A("10.1"), InvalidRdataText);
+    checkFromText<InvalidRdataText, InvalidRdataText>("10.1");
     // or an IPv6 address
-    EXPECT_THROW(in::A("2001:db8::1234"), InvalidRdataText);
+    checkFromText<InvalidRdataText, InvalidRdataText>("2001:db8::1234");
     // or any meaningless text as an IP address
-    EXPECT_THROW(in::A("xxx"), InvalidRdataText);
+    checkFromText<InvalidRdataText, InvalidRdataText>("xxx");
+
+    // trailing white space: only string version throws
+    checkFromText<InvalidRdataText, InvalidRdataText>("192.0.2.1  ",
+                                                      true, false);
+    // same for beginning white space.
+    checkFromText<InvalidRdataText, InvalidRdataText>("  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).
+    checkFromText<InvalidRdataText, InvalidRdataText>("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);
+    checkFromText<InvalidRdataText, InvalidRdataText>(nul_after_addr, true,
+                                                      true);
+
+    // a valid address surrounded by parentheses; only okay with lexer
+    checkFromText<InvalidRdataText, InvalidRdataText>("(192.0.2.1)", true,
+                                                      false);
 }
 
 TEST_F(Rdata_IN_A_Test, createFromWire) {
@@ -68,11 +127,6 @@ TEST_F(Rdata_IN_A_Test, createFromWire) {
                  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) {
     rdata_in_a.toWire(obuffer);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
@@ -89,15 +143,17 @@ TEST_F(Rdata_IN_A_Test, toWireRenderer) {
 
 TEST_F(Rdata_IN_A_Test, 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());
 }
 
 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
     // cppcheck-suppress uselessCallsCompare