From: JINMEI Tatuya Date: Fri, 22 Feb 2013 23:16:37 +0000 (-0800) Subject: [2389] use MasterLexer for IN/A RDATA constructor. X-Git-Tag: bind10-1.1.0beta1-release~75^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d87005705b51b343f7e1ffc22ea76312ad5c9380;p=thirdparty%2Fkea.git [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. --- diff --git a/src/lib/dns/gen-rdatacode.py.in b/src/lib/dns/gen-rdatacode.py.in index b385bf4192..221d46009c 100755 --- a/src/lib/dns/gen-rdatacode.py.in +++ b/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'), diff --git a/src/lib/dns/rdata/in_1/a_1.cc b/src/lib/dns/rdata/in_1/a_1.cc index 3b15a4c4de..4dbd2bcc21 100644 --- a/src/lib/dns/rdata/in_1/a_1.cc +++ b/src/lib/dns/rdata/in_1/a_1.cc @@ -15,6 +15,8 @@ #include #include +#include +#include #include #include // XXX: for inet_pton/ntop(), not exist in C++ standards @@ -23,8 +25,11 @@ #include #include + #include #include +#include +#include #include #include @@ -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(other); diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc index 636f73d24e..653da05e62 100644 --- a/src/lib/dns/tests/master_loader_unittest.cc +++ b/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" }, diff --git a/src/lib/dns/tests/rdata_in_a_unittest.cc b/src/lib/dns/tests/rdata_in_a_unittest.cc index 7ab342304c..fb8fa21463 100644 --- a/src/lib/dns/tests/rdata_in_a_unittest.cc +++ b/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 + #include #include #include +#include +#include #include -#include #include #include @@ -25,6 +28,8 @@ #include #include +#include + 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 + 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("192.0.2.1", false, false); + // should reject an abbreviated form of IPv4 address - EXPECT_THROW(in::A("10.1"), InvalidRdataText); + checkFromText("10.1"); // or an IPv6 address - EXPECT_THROW(in::A("2001:db8::1234"), InvalidRdataText); + checkFromText("2001:db8::1234"); // or any meaningless text as an IP address - EXPECT_THROW(in::A("xxx"), InvalidRdataText); + checkFromText("xxx"); + + // trailing white space: only string version throws + checkFromText("192.0.2.1 ", + true, false); + // same for beginning white space. + checkFromText(" 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("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(nul_after_addr, true, + true); + + // a valid address surrounded by parentheses; only okay with lexer + checkFromText("(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