From a2bcdf7b77c0618ee4ba4d257be9e854e1888733 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 26 Dec 2010 02:54:41 +0000 Subject: [PATCH 01/21] work/review branch for trac #448 git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4010 e5f2f494-b856-4b98-b285-d166d9295462 From 9d0b49c75b47fbb0d8542658cb78d5b7cab8d95d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 26 Dec 2010 03:47:58 +0000 Subject: [PATCH 02/21] made asiolink-tests clang++ friendly: - avoid relying on ASIO details in the main test (this is good anyway in terms of the original intent of asio link) - move internal-dependent tests to internal/tests with a workaround compiler flag Also fixed a potentially common bug: use blocking recv() in recvUDP() with a fail safe timeout. When compiled with clang++ this test sometimes failed due to the failure of recv(), and after looking at the code closely I found it possible to happen for other environments. git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4011 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/asiolink/Makefile.am | 2 +- .../asiolink/internal/tests/run_unittests.cc | 21 ++++ .../{ => internal}/tests/udpdns_unittest.cc | 0 src/lib/asiolink/tests/Makefile.am | 1 - src/lib/asiolink/tests/asiolink_unittest.cc | 97 +++++++++++-------- 5 files changed, 79 insertions(+), 42 deletions(-) create mode 100644 src/lib/asiolink/internal/tests/run_unittests.cc rename src/lib/asiolink/{ => internal}/tests/udpdns_unittest.cc (100%) diff --git a/src/lib/asiolink/Makefile.am b/src/lib/asiolink/Makefile.am index 02abfe08c4..26d9a6875c 100644 --- a/src/lib/asiolink/Makefile.am +++ b/src/lib/asiolink/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests +SUBDIRS = . tests internal AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) diff --git a/src/lib/asiolink/internal/tests/run_unittests.cc b/src/lib/asiolink/internal/tests/run_unittests.cc new file mode 100644 index 0000000000..5f1195d9da --- /dev/null +++ b/src/lib/asiolink/internal/tests/run_unittests.cc @@ -0,0 +1,21 @@ +// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +int +main(int argc, char* argv[]) { + ::testing::InitGoogleTest(&argc, argv); + return (RUN_ALL_TESTS()); +} diff --git a/src/lib/asiolink/tests/udpdns_unittest.cc b/src/lib/asiolink/internal/tests/udpdns_unittest.cc similarity index 100% rename from src/lib/asiolink/tests/udpdns_unittest.cc rename to src/lib/asiolink/internal/tests/udpdns_unittest.cc diff --git a/src/lib/asiolink/tests/Makefile.am b/src/lib/asiolink/tests/Makefile.am index 8b599d5737..3c6cd3ee2e 100644 --- a/src/lib/asiolink/tests/Makefile.am +++ b/src/lib/asiolink/tests/Makefile.am @@ -18,7 +18,6 @@ TESTS += run_unittests run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc run_unittests_SOURCES += asiolink_unittest.cc -run_unittests_SOURCES += udpdns_unittest.cc run_unittests_SOURCES += run_unittests.cc run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) diff --git a/src/lib/asiolink/tests/asiolink_unittest.cc b/src/lib/asiolink/tests/asiolink_unittest.cc index 1dc10c6dfc..f012d4c196 100644 --- a/src/lib/asiolink/tests/asiolink_unittest.cc +++ b/src/lib/asiolink/tests/asiolink_unittest.cc @@ -17,6 +17,9 @@ #include +#include +#include + #include #include @@ -31,19 +34,21 @@ #include #include +// IMPORTANT: We shouldn't directly use ASIO definitions in this test. +// In particular, we must not include asio.hpp in this file. +// The asiolink module is primarily intended to be a wrapper that hide the +// details of the underlying implementations. We need to test the wrapper +// level behaviors. In addition, some compilers reject to compile this file +// if we include asio.hpp unless we specify a special compiler option. +// If we need to test something at the level of underlying ASIO and need +// their definition, that test should go to asiolink/internal/tests. #include #include -#include -#include - -#include using isc::UnitTestUtil; using namespace std; using namespace asiolink; using namespace isc::dns; -using namespace asio; -using asio::ip::udp; namespace { const char* const TEST_SERVER_PORT = "53535"; @@ -326,10 +331,20 @@ protected: // ... and this one will block until the send has completed io_service_->run_one(); - // Now we attempt to recv() whatever was sent - const int ret = recv(sock_, buffer, size, MSG_DONTWAIT); + // Now we attempt to recv() whatever was sent. + // XXX: there's no guarantee the receiving socket can immediately get + // the packet. Normally we can perform blocking recv to wait for it, + // but in theory it's even possible that the packet is lost. + // In order to prevent the test from hanging in such a worst case + // we add an ad hoc timeout. + const struct timeval timeo = { 10, 0 }; + if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, + sizeof(timeo))) { + isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + } + const int ret = recv(sock_, buffer, size, 0); if (ret < 0) { - isc_throw(IOError, "recvfrom failed"); + isc_throw(IOError, "recvfrom failed: " << strerror(errno)); } // Pass the message size back via the size parameter @@ -407,8 +422,7 @@ protected: // has completed. class MockServer : public DNSServer { public: - explicit MockServer(asio::io_service& io_service, - const asio::ip::address& addr, const uint16_t port, + explicit MockServer(IOService& io_service, SimpleCallback* checkin = NULL, DNSLookup* lookup = NULL, DNSAnswer* answer = NULL) : @@ -422,9 +436,7 @@ protected: size_t length = 0) {} - void resume(const bool done) { - done_ = done; - io_.post(*this); + void resume(const bool) { // in our test this shouldn't be called } DNSServer* clone() { @@ -439,7 +451,7 @@ protected: } protected: - asio::io_service& io_; + IOService& io_; bool done_; private: @@ -458,8 +470,8 @@ protected: // This version of mock server just stops the io_service when it is resumed class MockServerStop : public MockServer { public: - explicit MockServerStop(asio::io_service& io_service, bool* done) : - MockServer(io_service, asio::ip::address(), 0), + explicit MockServerStop(IOService& io_service, bool* done) : + MockServer(io_service), done_(done) {} @@ -507,7 +519,6 @@ protected: string callback_address_; vector callback_data_; int sock_; -private: struct addrinfo* res_; }; @@ -636,14 +647,12 @@ TEST_F(ASIOLinkTest, recursiveSetupV6) { // full code coverage including error cases. TEST_F(ASIOLinkTest, recursiveSend) { setDNSService(true, false); - asio::io_service& io = io_service_->get_io_service(); // Note: We use the test prot plus one to ensure we aren't binding // to the same port as the actual server uint16_t port = boost::lexical_cast(TEST_CLIENT_PORT); - asio::ip::address addr = asio::ip::address::from_string(TEST_IPV4_ADDR); - MockServer server(io, addr, port, NULL, NULL, NULL); + MockServer server(*io_service_); RecursiveQuery rq(*dns_service_, singleAddress(TEST_IPV4_ADDR, port)); Question q(Name("example.com"), RRClass::IN(), RRType::TXT()); @@ -652,7 +661,7 @@ TEST_F(ASIOLinkTest, recursiveSend) { char data[4096]; size_t size = sizeof(data); - EXPECT_NO_THROW(recvUDP(AF_INET, data, size)); + ASSERT_NO_THROW(recvUDP(AF_INET, data, size)); Message m(Message::PARSE); InputBuffer ibuf(data, size); @@ -668,34 +677,27 @@ TEST_F(ASIOLinkTest, recursiveSend) { EXPECT_EQ(q.getClass(), q2->getClass()); } -void -receive_and_inc(udp::socket* socket, int* num) { - (*num) ++; - static char inbuff[512]; - socket->async_receive(asio::buffer(inbuff, 512), - boost::bind(receive_and_inc, socket, num)); -} - // Test it tries the correct amount of times before giving up TEST_F(ASIOLinkTest, recursiveTimeout) { // Prepare the service (we do not use the common setup, we do not answer setDNSService(); - asio::io_service& service = io_service_->get_io_service(); // Prepare the socket - uint16_t port = boost::lexical_cast(TEST_CLIENT_PORT); - udp::socket socket(service, udp::v4()); - socket.set_option(socket_base::reuse_address(true)); - socket.bind(udp::endpoint(ip::address::from_string(TEST_IPV4_ADDR), port)); - // And count the answers - int num = -1; // One is counted before the receipt of the first one - receive_and_inc(&socket, &num); + res_ = resolveAddress(AF_INET, IPPROTO_UDP, true); + sock_ = socket(res_->ai_family, res_->ai_socktype, res_->ai_protocol); + if (sock_ < 0) { + isc_throw(IOError, "failed to open test socket"); + } + if (bind(sock_, res_->ai_addr, res_->ai_addrlen) < 0) { + isc_throw(IOError, "failed to bind test socket"); + } // Prepare the server bool done(true); - MockServerStop server(service, &done); + MockServerStop server(*io_service_, &done); // Do the answer + const uint16_t port = boost::lexical_cast(TEST_CLIENT_PORT); RecursiveQuery query(*dns_service_, singleAddress(TEST_IPV4_ADDR, port), 10, 2); Question question(Name("example.net"), RRClass::IN(), RRType::A()); @@ -703,7 +705,22 @@ TEST_F(ASIOLinkTest, recursiveTimeout) { query.sendQuery(question, buffer, &server); // Run the test - service.run(); + io_service_->run(); + + // Read up to 3 packets. Use some ad hoc timeout to prevent an infinite + // block (see also recvUDP()). + const struct timeval timeo = { 10, 0 }; + if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) { + isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + } + int num = 0; + do { + char inbuff[512]; + if (recv(sock_, inbuff, sizeof(inbuff), 0) < 0) { + num = -1; + break; + } + } while (++num < 3); // The query should fail EXPECT_FALSE(done); From 182fdca6e3fd6b7e2058f42fe35fc5af6588d290 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 26 Dec 2010 03:52:45 +0000 Subject: [PATCH 03/21] made lib/testutils clang++-friendly: - replaced the ugly macro with real methods the base class using the template method pattern. as a result srv_unittest.h is not needed anymore and removed. - avoided the use of an unnamed namespace in srv_test.h (another bad practice anyway) - avoided the use of 'using namespace' in srv_test.h (yet another bad practice anyway) git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4012 e5f2f494-b856-4b98-b285-d166d9295462 --- configure.ac | 2 + src/bin/auth/tests/Makefile.am | 1 + src/bin/auth/tests/auth_srv_unittest.cc | 31 +- src/bin/recurse/tests/Makefile.am | 1 + .../recurse/tests/recursor_config_unittest.cc | 18 +- src/bin/recurse/tests/recursor_unittest.cc | 29 +- src/lib/testutils/Makefile.am | 14 +- src/lib/testutils/README | 2 - src/lib/testutils/srv_test.cc | 272 ++++++++++++++++++ src/lib/testutils/srv_test.h | 214 ++++++-------- src/lib/testutils/srv_unittest.h | 158 ---------- 11 files changed, 427 insertions(+), 315 deletions(-) create mode 100644 src/lib/testutils/srv_test.cc delete mode 100644 src/lib/testutils/srv_unittest.h diff --git a/configure.ac b/configure.ac index 70c690a933..2103b4b8ab 100644 --- a/configure.ac +++ b/configure.ac @@ -577,6 +577,8 @@ AC_CONFIG_FILES([Makefile src/lib/Makefile src/lib/asiolink/Makefile src/lib/asiolink/tests/Makefile + src/lib/asiolink/internal/Makefile + src/lib/asiolink/internal/tests/Makefile src/lib/bench/Makefile src/lib/bench/example/Makefile src/lib/bench/tests/Makefile diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am index aa44b28e88..09f515fc76 100644 --- a/src/bin/auth/tests/Makefile.am +++ b/src/bin/auth/tests/Makefile.am @@ -38,6 +38,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la +run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la endif noinst_PROGRAMS = $(TESTS) diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 91d8132883..f9e0d57c19 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -17,13 +17,16 @@ #include #include #include -#include + +#include +#include using namespace isc::cc; using namespace isc::dns; using namespace isc::data; using namespace isc::xfr; using namespace asiolink; +using namespace isc::testutils; using isc::UnitTestUtil; namespace { @@ -39,6 +42,10 @@ protected: AuthSrvTest() : server(true, xfrout), rrclass(RRClass::IN()) { server.setXfrinSession(¬ify_session); } + virtual void processMessage() { + server.processMessage(*io_message, parse_message, response_obuffer, + &dnsserv); + } MockXfroutClient xfrout; AuthSrv server; const RRClass rrclass; @@ -46,48 +53,52 @@ protected: // Unsupported requests. Should result in NOTIMP. TEST_F(AuthSrvTest, unsupportedRequest) { - UNSUPPORTED_REQUEST_TEST; + unsupportedRequest(); } // Simple API check TEST_F(AuthSrvTest, verbose) { - VERBOSE_TEST; + EXPECT_FALSE(server.getVerbose()); + server.setVerbose(true); + EXPECT_TRUE(server.getVerbose()); + server.setVerbose(false); + EXPECT_FALSE(server.getVerbose()); } // Multiple questions. Should result in FORMERR. TEST_F(AuthSrvTest, multiQuestion) { - MULTI_QUESTION_TEST; + multiQuestion(); } // Incoming data doesn't even contain the complete header. Must be silently // dropped. TEST_F(AuthSrvTest, shortMessage) { - SHORT_MESSAGE_TEST; + shortMessage(); } // Response messages. Must be silently dropped, whether it's a valid response // or malformed or could otherwise cause a protocol error. TEST_F(AuthSrvTest, response) { - RESPONSE_TEST; + response(); } // Query with a broken question TEST_F(AuthSrvTest, shortQuestion) { - SHORT_QUESTION_TEST; + shortQuestion(); } // Query with a broken answer section TEST_F(AuthSrvTest, shortAnswer) { - SHORT_ANSWER_TEST; + shortAnswer(); } // Query with unsupported version of EDNS. TEST_F(AuthSrvTest, ednsBadVers) { - EDNS_BADVERS_TEST; + ednsBadVers(); } TEST_F(AuthSrvTest, AXFROverUDP) { - AXFR_OVER_UDP_TEST; + axfrOverUDP(); } TEST_F(AuthSrvTest, AXFRSuccess) { diff --git a/src/bin/recurse/tests/Makefile.am b/src/bin/recurse/tests/Makefile.am index 973c1018ea..5e253ee579 100644 --- a/src/bin/recurse/tests/Makefile.am +++ b/src/bin/recurse/tests/Makefile.am @@ -33,6 +33,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la +run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la endif diff --git a/src/bin/recurse/tests/recursor_config_unittest.cc b/src/bin/recurse/tests/recursor_config_unittest.cc index e07f729687..4b192ba025 100644 --- a/src/bin/recurse/tests/recursor_config_unittest.cc +++ b/src/bin/recurse/tests/recursor_config_unittest.cc @@ -12,10 +12,24 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -// $Id$ +#include + +#include + +#include + +#include #include -#include + +#include +#include + +using namespace std; +using namespace isc::data; +using namespace isc::testutils; +using namespace asiolink; +using isc::UnitTestUtil; namespace { class RecursorConfig : public ::testing::Test { diff --git a/src/bin/recurse/tests/recursor_unittest.cc b/src/bin/recurse/tests/recursor_unittest.cc index 84bbcc9e73..84ddbf4d1f 100644 --- a/src/bin/recurse/tests/recursor_unittest.cc +++ b/src/bin/recurse/tests/recursor_unittest.cc @@ -12,10 +12,15 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -// $Id$ +#include #include -#include +#include +#include + +using namespace isc::dns; +using namespace isc::testutils; +using isc::UnitTestUtil; namespace { const char* const TEST_PORT = "53535"; @@ -23,48 +28,52 @@ const char* const TEST_PORT = "53535"; class RecursorTest : public SrvTestBase{ protected: RecursorTest() : server(){} + virtual void processMessage() { + server.processMessage(*io_message, parse_message, response_obuffer, + &dnsserv); + } Recursor server; }; // Unsupported requests. Should result in NOTIMP. TEST_F(RecursorTest, unsupportedRequest) { - UNSUPPORTED_REQUEST_TEST; + unsupportedRequest(); } // Multiple questions. Should result in FORMERR. TEST_F(RecursorTest, multiQuestion) { - MULTI_QUESTION_TEST; + multiQuestion(); } // Incoming data doesn't even contain the complete header. Must be silently // dropped. TEST_F(RecursorTest, shortMessage) { - SHORT_MESSAGE_TEST; + shortMessage(); } // Response messages. Must be silently dropped, whether it's a valid response // or malformed or could otherwise cause a protocol error. TEST_F(RecursorTest, response) { - RESPONSE_TEST; + response(); } // Query with a broken question TEST_F(RecursorTest, shortQuestion) { - SHORT_QUESTION_TEST; + shortQuestion(); } // Query with a broken answer section TEST_F(RecursorTest, shortAnswer) { - SHORT_ANSWER_TEST; + shortAnswer(); } // Query with unsupported version of EDNS. TEST_F(RecursorTest, ednsBadVers) { - EDNS_BADVERS_TEST; + ednsBadVers(); } TEST_F(RecursorTest, AXFROverUDP) { - AXFR_OVER_UDP_TEST; + axfrOverUDP(); } TEST_F(RecursorTest, AXFRFail) { diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index 8593204af8..fc235e8530 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -1,5 +1,11 @@ -SUBDIRS = testdata +SUBDIRS = . testdata -EXTRA_DIST = srv_test.h -EXTRA_DIST += srv_unittest.h -EXTRA_DIST += mockups.h +AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib +AM_CPPFLAGS += $(BOOST_INCLUDES) +AM_CXXFLAGS=$(B10_CXXFLAGS) + +lib_LTLIBRARIES = libtestutils.la + +libtestutils_la_SOURCES = srv_test.h srv_test.cc +libtestutils_la_SOURCES += srv_unittest.h +libtestutils_la_SOURCES += mockups.h diff --git a/src/lib/testutils/README b/src/lib/testutils/README index 8845b057cc..578365d2a8 100644 --- a/src/lib/testutils/README +++ b/src/lib/testutils/README @@ -1,4 +1,2 @@ Here is some code used by more than one test. No code is used for bind10 itself, only for testing. - -As it contains headers only currently, it does not compile here. diff --git a/src/lib/testutils/srv_test.cc b/src/lib/testutils/srv_test.cc new file mode 100644 index 0000000000..1deb8f0bb8 --- /dev/null +++ b/src/lib/testutils/srv_test.cc @@ -0,0 +1,272 @@ +// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +#include +#include + +#include + +#include + +#include + +using namespace isc::dns; +using namespace asiolink; + +namespace isc { +namespace testutils { +const char* const DEFAULT_REMOTE_ADDRESS = "192.0.2.1"; + +const unsigned int QR_FLAG = 0x1; +const unsigned int AA_FLAG = 0x2; +const unsigned int TC_FLAG = 0x4; +const unsigned int RD_FLAG = 0x8; +const unsigned int RA_FLAG = 0x10; +const unsigned int AD_FLAG = 0x20; +const unsigned int CD_FLAG = 0x40; + +SrvTestBase::SrvTestBase() : request_message(Message::RENDER), + parse_message(new Message(Message::PARSE)), + default_qid(0x1035), + opcode(Opcode(Opcode::QUERY())), + qname("www.example.com"), + qclass(RRClass::IN()), + qtype(RRType::A()), io_sock(NULL), + io_message(NULL), endpoint(NULL), + request_obuffer(0), + request_renderer(request_obuffer), + response_obuffer(new OutputBuffer(0)) +{} + +SrvTestBase::~SrvTestBase() { + delete io_message; + delete endpoint; +} + +void +SrvTestBase::createDataFromFile(const char* const datafile, + const int protocol) +{ + delete io_message; + data.clear(); + + delete endpoint; + + endpoint = IOEndpoint::create(protocol, + IOAddress(DEFAULT_REMOTE_ADDRESS), 5300); + UnitTestUtil::readWireData(datafile, data); + io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() : + &IOSocket::getDummyTCPSocket(); + io_message = new IOMessage(&data[0], data.size(), *io_sock, *endpoint); +} + +void +SrvTestBase::createRequestPacket(Message& message, + const int protocol) +{ + message.toWire(request_renderer); + + delete io_message; + + endpoint = IOEndpoint::create(protocol, + IOAddress(DEFAULT_REMOTE_ADDRESS), 5300); + io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() : + &IOSocket::getDummyTCPSocket(); + io_message = new IOMessage(request_renderer.getData(), + request_renderer.getLength(), + *io_sock, *endpoint); +} + +// Unsupported requests. Should result in NOTIMP. +void +SrvTestBase::unsupportedRequest() { + for (unsigned int i = 0; i < 16; ++i) { + // set Opcode to 'i', which iterators over all possible codes except + // the standard query and notify + if (i == isc::dns::Opcode::QUERY().getCode() || + i == isc::dns::Opcode::NOTIFY().getCode()) { + continue; + } + createDataFromFile("simplequery_fromWire.wire"); + data[2] = ((i << 3) & 0xff); + + parse_message->clear(isc::dns::Message::PARSE); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + headerCheck(*parse_message, default_qid, isc::dns::Rcode::NOTIMP(), i, + QR_FLAG, 0, 0, 0, 0); + } +} + +// Multiple questions. Should result in FORMERR. +void +SrvTestBase::multiQuestion() { + createDataFromFile("multiquestion_fromWire.wire"); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + headerCheck(*parse_message, default_qid, isc::dns::Rcode::FORMERR(), + opcode.getCode(), QR_FLAG, 2, 0, 0, 0); + + isc::dns::QuestionIterator qit = parse_message->beginQuestion(); + EXPECT_EQ(isc::dns::Name("example.com"), (*qit)->getName()); + EXPECT_EQ(isc::dns::RRClass::IN(), (*qit)->getClass()); + EXPECT_EQ(isc::dns::RRType::A(), (*qit)->getType()); + ++qit; + EXPECT_EQ(isc::dns::Name("example.com"), (*qit)->getName()); + EXPECT_EQ(isc::dns::RRClass::IN(), (*qit)->getClass()); + EXPECT_EQ(isc::dns::RRType::AAAA(), (*qit)->getType()); + ++qit; + EXPECT_TRUE(qit == parse_message->endQuestion()); +} + +// Incoming data doesn't even contain the complete header. Must be silently +// dropped. +void +SrvTestBase::shortMessage() { + createDataFromFile("shortmessage_fromWire"); + processMessage(); + EXPECT_FALSE(dnsserv.hasAnswer()); +} + +// Response messages. Must be silently dropped, whether it's a valid response +// or malformed or could otherwise cause a protocol error. +void +SrvTestBase::response() { + // A valid (although unusual) response + createDataFromFile("simpleresponse_fromWire.wire"); + processMessage(); + EXPECT_FALSE(dnsserv.hasAnswer()); + + // A response with a broken question section. must be dropped rather than + //returning FORMERR. + createDataFromFile("shortresponse_fromWire"); + processMessage(); + EXPECT_FALSE(dnsserv.hasAnswer()); + + // A response to iquery. must be dropped rather than returning NOTIMP. + createDataFromFile("iqueryresponse_fromWire.wire"); + processMessage(); + EXPECT_FALSE(dnsserv.hasAnswer()); +} + +// Query with a broken question +void +SrvTestBase::shortQuestion() { + createDataFromFile("shortquestion_fromWire"); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + // Since the query's question is broken, the question section of the + // response should be empty. + headerCheck(*parse_message, default_qid, isc::dns::Rcode::FORMERR(), + opcode.getCode(), QR_FLAG, 0, 0, 0, 0); +} + +// Query with a broken answer section +void +SrvTestBase::shortAnswer() { + createDataFromFile("shortanswer_fromWire.wire"); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + + // This is a bogus query, but question section is valid. So the response + // should copy the question section. + headerCheck(*parse_message, default_qid, isc::dns::Rcode::FORMERR(), + opcode.getCode(), QR_FLAG, 1, 0, 0, 0); + + isc::dns::QuestionIterator qit = parse_message->beginQuestion(); + EXPECT_EQ(isc::dns::Name("example.com"), (*qit)->getName()); + EXPECT_EQ(isc::dns::RRClass::IN(), (*qit)->getClass()); + EXPECT_EQ(isc::dns::RRType::A(), (*qit)->getType()); + ++qit; + EXPECT_TRUE(qit == parse_message->endQuestion()); +} + +// Query with unsupported version of EDNS. +void +SrvTestBase::ednsBadVers() { + createDataFromFile("queryBadEDNS_fromWire.wire"); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + + // The response must have an EDNS OPT RR in the additional section, + // it will be added automatically at the render time. + // Note that the DNSSEC DO bit is cleared even if this bit in the query + // is set. This is a limitation of the current implementation. + headerCheck(*parse_message, default_qid, isc::dns::Rcode::BADVERS(), + opcode.getCode(), QR_FLAG, 1, 0, 0, 1); + EXPECT_FALSE(parse_message->getEDNS()); // EDNS isn't added at this point + + isc::dns::InputBuffer ib(response_obuffer->getData(), + response_obuffer->getLength()); + isc::dns::Message parsed(isc::dns::Message::PARSE); + parsed.fromWire(ib); + EXPECT_EQ(isc::dns::Rcode::BADVERS(), parsed.getRcode()); + isc::dns::ConstEDNSPtr edns(parsed.getEDNS()); + ASSERT_TRUE(edns); + EXPECT_FALSE(edns->getDNSSECAwareness()); +} + +void +SrvTestBase::axfrOverUDP() { + // AXFR over UDP is invalid and should result in FORMERR. + UnitTestUtil::createRequestMessage(request_message, opcode, default_qid, + isc::dns::Name("example.com"), + isc::dns::RRClass::IN(), + isc::dns::RRType::AXFR()); + createRequestPacket(request_message, IPPROTO_UDP); + processMessage(); + EXPECT_TRUE(dnsserv.hasAnswer()); + headerCheck(*parse_message, default_qid, isc::dns::Rcode::FORMERR(), + opcode.getCode(), QR_FLAG, 1, 0, 0, 0); +} + +void +headerCheck(const Message& message, const qid_t qid, const Rcode& rcode, + const uint16_t opcodeval, const unsigned int flags, + const unsigned int qdcount, + const unsigned int ancount, const unsigned int nscount, + const unsigned int arcount) +{ + EXPECT_EQ(qid, message.getQid()); + EXPECT_EQ(rcode, message.getRcode()); + EXPECT_EQ(opcodeval, message.getOpcode().getCode()); + EXPECT_EQ((flags & QR_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_QR)); + EXPECT_EQ((flags & AA_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_AA)); + EXPECT_EQ((flags & TC_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_TC)); + EXPECT_EQ((flags & RA_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_RA)); + EXPECT_EQ((flags & RD_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_RD)); + EXPECT_EQ((flags & AD_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_AD)); + EXPECT_EQ((flags & CD_FLAG) != 0, + message.getHeaderFlag(Message::HEADERFLAG_CD)); + + EXPECT_EQ(qdcount, message.getRRCount(Message::SECTION_QUESTION)); + EXPECT_EQ(ancount, message.getRRCount(Message::SECTION_ANSWER)); + EXPECT_EQ(nscount, message.getRRCount(Message::SECTION_AUTHORITY)); + EXPECT_EQ(arcount, message.getRRCount(Message::SECTION_ADDITIONAL)); +} +} // end of namespace testutils +} // end of namespace isc + + +// Local Variables: +// mode: c++ +// End: diff --git a/src/lib/testutils/srv_test.h b/src/lib/testutils/srv_test.h index e4bbb3bc52..be8b95ad7b 100644 --- a/src/lib/testutils/srv_test.h +++ b/src/lib/testutils/srv_test.h @@ -12,10 +12,6 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -// $Id: auth_srv_unittest.cc 3310 2010-10-21 23:10:24Z each $ - -#include - #include #include @@ -27,139 +23,99 @@ #include #include -#include -#include - -#include - -#include -#include - -#include #include "mockups.h" -using namespace std; -using namespace isc::cc; -using namespace isc::dns; -using namespace isc::data; -using namespace isc::xfr; -using namespace asiolink; -using isc::UnitTestUtil; +namespace asiolink { +class IOSocket; +class IOMessage; +class IOEndpoint; +} -namespace { -const char* const DEFAULT_REMOTE_ADDRESS = "192.0.2.1"; +namespace isc { +namespace testutils { +extern const char* const DEFAULT_REMOTE_ADDRESS; + +// These are flags to indicate whether the corresponding flag bit of the +// DNS header is to be set in the test cases. (The flag values +// is irrelevant to their wire-format values) +extern const unsigned int QR_FLAG; +extern const unsigned int AA_FLAG; +extern const unsigned int TC_FLAG; +extern const unsigned int RD_FLAG; +extern const unsigned int RA_FLAG; +extern const unsigned int AD_FLAG; +extern const unsigned int CD_FLAG; + +void +headerCheck(const isc::dns::Message& message, const isc::dns::qid_t qid, + const isc::dns::Rcode& rcode, + const uint16_t opcodeval, const unsigned int flags, + const unsigned int qdcount, + const unsigned int ancount, const unsigned int nscount, + const unsigned int arcount); // The base class for Auth and Recurse test case class SrvTestBase : public ::testing::Test { protected: - SrvTestBase() : request_message(Message::RENDER), - parse_message(new Message(Message::PARSE)), - default_qid(0x1035), opcode(Opcode(Opcode::QUERY())), - qname("www.example.com"), qclass(RRClass::IN()), - qtype(RRType::A()), io_sock(NULL), - io_message(NULL), endpoint(NULL), - request_obuffer(0), request_renderer(request_obuffer), - response_obuffer(new OutputBuffer(0)) - {} - ~SrvTestBase() { - delete io_message; - delete endpoint; - } + SrvTestBase(); + virtual ~SrvTestBase(); + + /// Let the server process a DNS message. + /// + /// The derived class implementation is expected to pass \c io_message, + /// \c parse_message, \c response_obuffer, and \c dnsserv to the server + /// implementation it is testing. + virtual void processMessage() = 0; + + /// The following methods implement server independent test logic using + /// the template method pattern. Each test calls \c processMessage() + /// to delegate the server-dependent behavior to the actual implementation + /// classes. + void unsupportedRequest(); + void multiQuestion(); + void shortMessage(); + void response(); + void shortQuestion(); + void shortAnswer(); + void ednsBadVers(); + void axfrOverUDP(); + + /// Create DNS packet data from a file. + /// + /// It constructs wire-format DNS packet data from \c datafile in the + /// form of \c IOMessage in \c io_message. + /// The existing content of \c io_message, if any, will be deleted. + void createDataFromFile(const char* const datafile, + int protocol = IPPROTO_UDP); + + /// Create DNS packet data from a message. + /// + /// It constructs wire-format DNS packet data from \c message in the + /// form of \c IOMessage in \c io_message. + /// The existing content of \c io_message, if any, will be deleted. + void createRequestPacket(isc::dns::Message& message, + const int protocol = IPPROTO_UDP); + MockSession notify_session; MockServer dnsserv; - Message request_message; - MessagePtr parse_message; - const qid_t default_qid; - const Opcode opcode; - const Name qname; - const RRClass qclass; - const RRType qtype; - IOSocket* io_sock; - IOMessage* io_message; - const IOEndpoint* endpoint; - OutputBuffer request_obuffer; - MessageRenderer request_renderer; - OutputBufferPtr response_obuffer; - vector data; - - void createDataFromFile(const char* const datafile, int protocol); - void createRequestPacket(Message& message, const int protocol); + isc::dns::Message request_message; + isc::dns::MessagePtr parse_message; + const isc::dns::qid_t default_qid; + const isc::dns::Opcode opcode; + const isc::dns::Name qname; + const isc::dns::RRClass qclass; + const isc::dns::RRType qtype; + asiolink::IOSocket* io_sock; + asiolink::IOMessage* io_message; + const asiolink::IOEndpoint* endpoint; + isc::dns::OutputBuffer request_obuffer; + isc::dns::MessageRenderer request_renderer; + isc::dns::OutputBufferPtr response_obuffer; + std::vector data; }; +} // end of namespace testutils +} // end of namespace isc -void -SrvTestBase::createDataFromFile(const char* const datafile, - const int protocol = IPPROTO_UDP) -{ - delete io_message; - data.clear(); - - delete endpoint; - - endpoint = IOEndpoint::create(protocol, - IOAddress(DEFAULT_REMOTE_ADDRESS), 5300); - UnitTestUtil::readWireData(datafile, data); - io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() : - &IOSocket::getDummyTCPSocket(); - io_message = new IOMessage(&data[0], data.size(), *io_sock, *endpoint); -} - -void -SrvTestBase::createRequestPacket(Message& message, - const int protocol = IPPROTO_UDP) -{ - message.toWire(request_renderer); - - delete io_message; - - endpoint = IOEndpoint::create(protocol, - IOAddress(DEFAULT_REMOTE_ADDRESS), 5300); - io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() : - &IOSocket::getDummyTCPSocket(); - io_message = new IOMessage(request_renderer.getData(), - request_renderer.getLength(), - *io_sock, *endpoint); -} - -// These are flags to indicate whether the corresponding flag bit of the -// DNS header is to be set in the test cases. (Note that the flag values -// is irrelevant to their wire-format values) -const unsigned int QR_FLAG = 0x1; -const unsigned int AA_FLAG = 0x2; -const unsigned int TC_FLAG = 0x4; -const unsigned int RD_FLAG = 0x8; -const unsigned int RA_FLAG = 0x10; -const unsigned int AD_FLAG = 0x20; -const unsigned int CD_FLAG = 0x40; - -void -headerCheck(const Message& message, const qid_t qid, const Rcode& rcode, - const uint16_t opcodeval, const unsigned int flags, - const unsigned int qdcount, - const unsigned int ancount, const unsigned int nscount, - const unsigned int arcount) -{ - EXPECT_EQ(qid, message.getQid()); - EXPECT_EQ(rcode, message.getRcode()); - EXPECT_EQ(opcodeval, message.getOpcode().getCode()); - EXPECT_EQ((flags & QR_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_QR)); - EXPECT_EQ((flags & AA_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_AA)); - EXPECT_EQ((flags & TC_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_TC)); - EXPECT_EQ((flags & RA_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_RA)); - EXPECT_EQ((flags & RD_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_RD)); - EXPECT_EQ((flags & AD_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_AD)); - EXPECT_EQ((flags & CD_FLAG) != 0, - message.getHeaderFlag(Message::HEADERFLAG_CD)); - - EXPECT_EQ(qdcount, message.getRRCount(Message::SECTION_QUESTION)); - EXPECT_EQ(ancount, message.getRRCount(Message::SECTION_ANSWER)); - EXPECT_EQ(nscount, message.getRRCount(Message::SECTION_AUTHORITY)); - EXPECT_EQ(arcount, message.getRRCount(Message::SECTION_ADDITIONAL)); -} - -} +// Local Variables: +// mode: c++ +// End: diff --git a/src/lib/testutils/srv_unittest.h b/src/lib/testutils/srv_unittest.h deleted file mode 100644 index 40bfb7a9a2..0000000000 --- a/src/lib/testutils/srv_unittest.h +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE -// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -// $Id: auth_srv_unittest.cc 3310 2010-10-21 23:10:24Z each $ - -#include "srv_test.h" - -namespace { - -// Unsupported requests. Should result in NOTIMP. -#define UNSUPPORTED_REQUEST_TEST \ - for (unsigned int i = 0; i < 16; ++i) { \ - /* set Opcode to 'i', which iterators over all possible codes except \ - the standard query and notify */ \ - if (i == Opcode::QUERY().getCode() || \ - i == Opcode::NOTIFY().getCode()) { \ - continue; \ - } \ - createDataFromFile("simplequery_fromWire.wire"); \ - data[2] = ((i << 3) & 0xff); \ - \ - parse_message->clear(Message::PARSE); \ - server.processMessage(*io_message, parse_message, response_obuffer, \ - &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - headerCheck(*parse_message, default_qid, Rcode::NOTIMP(), i, QR_FLAG, \ - 0, 0, 0, 0); \ - } - -// Simple API check -#define VERBOSE_TEST \ - EXPECT_FALSE(server.getVerbose()); \ - server.setVerbose(true); \ - EXPECT_TRUE(server.getVerbose()); \ - server.setVerbose(false); \ - EXPECT_FALSE(server.getVerbose()); \ - - -// Multiple questions. Should result in FORMERR. -#define MULTI_QUESTION_TEST \ - createDataFromFile("multiquestion_fromWire.wire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - headerCheck(*parse_message, default_qid, Rcode::FORMERR(), opcode.getCode(), \ - QR_FLAG, 2, 0, 0, 0); \ - \ - QuestionIterator qit = parse_message->beginQuestion(); \ - EXPECT_EQ(Name("example.com"), (*qit)->getName()); \ - EXPECT_EQ(RRClass::IN(), (*qit)->getClass()); \ - EXPECT_EQ(RRType::A(), (*qit)->getType()); \ - ++qit; \ - EXPECT_EQ(Name("example.com"), (*qit)->getName()); \ - EXPECT_EQ(RRClass::IN(), (*qit)->getClass()); \ - EXPECT_EQ(RRType::AAAA(), (*qit)->getType()); \ - ++qit; \ - EXPECT_TRUE(qit == parse_message->endQuestion()); - -// Incoming data doesn't even contain the complete header. Must be silently -// dropped. -#define SHORT_MESSAGE_TEST \ - createDataFromFile("shortmessage_fromWire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_FALSE(dnsserv.hasAnswer()); - -// Response messages. Must be silently dropped, whether it's a valid response -// or malformed or could otherwise cause a protocol error. -#define RESPONSE_TEST \ - /* A valid (although unusual) response */\ - createDataFromFile("simpleresponse_fromWire.wire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_FALSE(dnsserv.hasAnswer()); \ - \ - /* A response with a broken question section. must be dropped rather than \ - returning FORMERR. */\ - createDataFromFile("shortresponse_fromWire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_FALSE(dnsserv.hasAnswer()); \ - \ - /* A response to iquery. must be dropped rather than returning NOTIMP. */\ - createDataFromFile("iqueryresponse_fromWire.wire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_FALSE(dnsserv.hasAnswer()); - -// Query with a broken question -#define SHORT_QUESTION_TEST \ - createDataFromFile("shortquestion_fromWire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - /* Since the query's question is broken, the question section of the \ - response should be empty. */\ - headerCheck(*parse_message, default_qid, Rcode::FORMERR(), opcode.getCode(), \ - QR_FLAG, 0, 0, 0, 0); - -// Query with a broken answer section -#define SHORT_ANSWER_TEST \ - createDataFromFile("shortanswer_fromWire.wire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - \ - /* This is a bogus query, but question section is valid. So the response \ - should copy the question section. */ \ - headerCheck(*parse_message, default_qid, Rcode::FORMERR(), opcode.getCode(), \ - QR_FLAG, 1, 0, 0, 0); \ - \ - QuestionIterator qit = parse_message->beginQuestion(); \ - EXPECT_EQ(Name("example.com"), (*qit)->getName()); \ - EXPECT_EQ(RRClass::IN(), (*qit)->getClass()); \ - EXPECT_EQ(RRType::A(), (*qit)->getType()); \ - ++qit; \ - EXPECT_TRUE(qit == parse_message->endQuestion()); - -// Query with unsupported version of EDNS. -#define EDNS_BADVERS_TEST \ - createDataFromFile("queryBadEDNS_fromWire.wire"); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - \ - /* The response must have an EDNS OPT RR in the additional section, \ - it will be added automatically at the render time. - Note that the DNSSEC DO bit is cleared even if this bit in the query \ - is set. This is a limitation of the current implementation. */ \ - headerCheck(*parse_message, default_qid, Rcode::BADVERS(), opcode.getCode(), \ - QR_FLAG, 1, 0, 0, 1); \ - EXPECT_FALSE(parse_message->getEDNS()); /* EDNS isn't added at this point */ \ - \ - InputBuffer ib(response_obuffer->getData(), response_obuffer->getLength()); \ - Message parsed(Message::PARSE); \ - parsed.fromWire(ib); \ - EXPECT_EQ(Rcode::BADVERS(), parsed.getRcode()); \ - ConstEDNSPtr edns(parsed.getEDNS()); \ - ASSERT_TRUE(edns); \ - EXPECT_FALSE(edns->getDNSSECAwareness()); - - -#define AXFR_OVER_UDP_TEST \ - /* AXFR over UDP is invalid and should result in FORMERR. */\ - UnitTestUtil::createRequestMessage(request_message, opcode, default_qid, \ - Name("example.com"), RRClass::IN(), \ - RRType::AXFR()); \ - createRequestPacket(request_message, IPPROTO_UDP); \ - server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv); \ - EXPECT_TRUE(dnsserv.hasAnswer()); \ - headerCheck(*parse_message, default_qid, Rcode::FORMERR(), opcode.getCode(), \ - QR_FLAG, 1, 0, 0, 0); - -} - From 4c409c2281d8845e3590d5299e425fac93275d06 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 26 Dec 2010 03:55:01 +0000 Subject: [PATCH 04/21] missing files git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4013 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/asiolink/internal/Makefile.am | 1 + src/lib/asiolink/internal/tests/Makefile.am | 37 +++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 src/lib/asiolink/internal/Makefile.am create mode 100644 src/lib/asiolink/internal/tests/Makefile.am diff --git a/src/lib/asiolink/internal/Makefile.am b/src/lib/asiolink/internal/Makefile.am new file mode 100644 index 0000000000..3c6155b9c6 --- /dev/null +++ b/src/lib/asiolink/internal/Makefile.am @@ -0,0 +1 @@ +SUBDIRS = tests diff --git a/src/lib/asiolink/internal/tests/Makefile.am b/src/lib/asiolink/internal/tests/Makefile.am new file mode 100644 index 0000000000..6e32602766 --- /dev/null +++ b/src/lib/asiolink/internal/tests/Makefile.am @@ -0,0 +1,37 @@ +AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib +AM_CPPFLAGS += $(BOOST_INCLUDES) + +AM_CXXFLAGS = $(B10_CXXFLAGS) + +if USE_STATIC_LINK +AM_LDFLAGS = -static +endif + +CLEANFILES = *.gcno *.gcda + +TESTS = +if HAVE_GTEST +TESTS += run_unittests +run_unittests_SOURCES = udpdns_unittest.cc +run_unittests_SOURCES += run_unittests.cc +run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) +run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) +run_unittests_LDADD = $(GTEST_LDADD) +run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la +run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la +run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la +run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la +# Note: the ordering matters: -Wno-... must follow -Wextra (defined in +# B10_CXXFLAGS) +run_unittests_CXXFLAGS = $(AM_CXXFLAGS) +if USE_GXX +run_unittests_CXXFLAGS += -Wno-unused-parameter +endif +if USE_CLANGPP +# We need to disable -Werror for any test that uses internal definitions of +# ASIO when using clang++ +run_unittests_CXXFLAGS += -Wno-error +endif +endif + +noinst_PROGRAMS = $(TESTS) From 6c330ea5b74512542959a48d8f6961137b5b121b Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 20:30:30 +0000 Subject: [PATCH 05/21] made sure we don't try to build the library unless we enable the gtest git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4054 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/testutils/Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index fc235e8530..89fda2005d 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -4,8 +4,10 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS=$(B10_CXXFLAGS) +if HAVE_GTEST lib_LTLIBRARIES = libtestutils.la libtestutils_la_SOURCES = srv_test.h srv_test.cc libtestutils_la_SOURCES += srv_unittest.h libtestutils_la_SOURCES += mockups.h +endif From 70585d0e35c4470a98a6bcc0d8edb9d89ae85405 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 20:32:25 +0000 Subject: [PATCH 06/21] reordered linking to minimize the risk of "undefined symbols" with static link git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4055 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/asiolink/internal/tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/asiolink/internal/tests/Makefile.am b/src/lib/asiolink/internal/tests/Makefile.am index 6e32602766..449cab740a 100644 --- a/src/lib/asiolink/internal/tests/Makefile.am +++ b/src/lib/asiolink/internal/tests/Makefile.am @@ -17,9 +17,9 @@ run_unittests_SOURCES += run_unittests.cc run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) run_unittests_LDADD = $(GTEST_LDADD) -run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la -run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la +run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la +run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la # Note: the ordering matters: -Wno-... must follow -Wextra (defined in # B10_CXXFLAGS) From 27966638a14159da40f834dd03d88e998efe047f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 20:41:49 +0000 Subject: [PATCH 07/21] added workaround for Solaris: if SO_RCVTIMEO is rejected due to EOPNOTSUPP switch to "no wait recv()" git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4056 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/asiolink/tests/asiolink_unittest.cc | 23 +++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/lib/asiolink/tests/asiolink_unittest.cc b/src/lib/asiolink/tests/asiolink_unittest.cc index f012d4c196..2e2608c4f3 100644 --- a/src/lib/asiolink/tests/asiolink_unittest.cc +++ b/src/lib/asiolink/tests/asiolink_unittest.cc @@ -338,11 +338,21 @@ protected: // In order to prevent the test from hanging in such a worst case // we add an ad hoc timeout. const struct timeval timeo = { 10, 0 }; + int recv_options = 0; if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) { - isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + if (errno == EOPNOTSUPP) { + // Workaround for Solaris: it doesn't accept SO_RCVTIMEO + // with the error of EOPNOTSUPP. Since this is a workaround + // for rare error cases anyway, we simply switch to the + // "don't wait" mode. If we still find an error in recv() + // can happen often we'll consider a more complete solution. + recv_options = MSG_DONTWAIT; + } else { + isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + } } - const int ret = recv(sock_, buffer, size, 0); + const int ret = recv(sock_, buffer, size, recv_options); if (ret < 0) { isc_throw(IOError, "recvfrom failed: " << strerror(errno)); } @@ -710,13 +720,18 @@ TEST_F(ASIOLinkTest, recursiveTimeout) { // Read up to 3 packets. Use some ad hoc timeout to prevent an infinite // block (see also recvUDP()). const struct timeval timeo = { 10, 0 }; + int recv_options = 0; if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) { - isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + if (errno == EOPNOTSUPP) { // see ASIOLinkTest::recvUDP() + recv_options = MSG_DONTWAIT; + } else { + isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); + } } int num = 0; do { char inbuff[512]; - if (recv(sock_, inbuff, sizeof(inbuff), 0) < 0) { + if (recv(sock_, inbuff, sizeof(inbuff), recv_options) < 0) { num = -1; break; } From ce6482880d25c387923fe191c6c939d3ffea7b23 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 20:51:49 +0000 Subject: [PATCH 08/21] oops, the errno should have been ENOPROTOOPT git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4058 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/asiolink/tests/asiolink_unittest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/asiolink/tests/asiolink_unittest.cc b/src/lib/asiolink/tests/asiolink_unittest.cc index 2e2608c4f3..45e42d0ae4 100644 --- a/src/lib/asiolink/tests/asiolink_unittest.cc +++ b/src/lib/asiolink/tests/asiolink_unittest.cc @@ -341,9 +341,9 @@ protected: int recv_options = 0; if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) { - if (errno == EOPNOTSUPP) { + if (errno == ENOPROTOOPT) { // Workaround for Solaris: it doesn't accept SO_RCVTIMEO - // with the error of EOPNOTSUPP. Since this is a workaround + // with the error of ENOPROTOOPT. Since this is a workaround // for rare error cases anyway, we simply switch to the // "don't wait" mode. If we still find an error in recv() // can happen often we'll consider a more complete solution. @@ -722,7 +722,7 @@ TEST_F(ASIOLinkTest, recursiveTimeout) { const struct timeval timeo = { 10, 0 }; int recv_options = 0; if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) { - if (errno == EOPNOTSUPP) { // see ASIOLinkTest::recvUDP() + if (errno == ENOPROTOOPT) { // see ASIOLinkTest::recvUDP() recv_options = MSG_DONTWAIT; } else { isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno)); From 199db30d8160d9b69dec5c123edaf91357737289 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 20:58:24 +0000 Subject: [PATCH 09/21] explicitly include GTEST_INCLUDES git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4059 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/testutils/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index 89fda2005d..f0e172d788 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -10,4 +10,5 @@ lib_LTLIBRARIES = libtestutils.la libtestutils_la_SOURCES = srv_test.h srv_test.cc libtestutils_la_SOURCES += srv_unittest.h libtestutils_la_SOURCES += mockups.h +libtestutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) endif From 51e85a3730104bce4eb047abdefa6483a4fe0f7d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 22:24:53 +0000 Subject: [PATCH 10/21] more link order fix for static link git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4061 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am index 09f515fc76..9055d12bb4 100644 --- a/src/bin/auth/tests/Makefile.am +++ b/src/bin/auth/tests/Makefile.am @@ -31,6 +31,7 @@ run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) run_unittests_LDADD = $(GTEST_LDADD) run_unittests_LDADD += $(SQLITE_LIBS) +run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la run_unittests_LDADD += $(top_builddir)/src/lib/datasrc/libdatasrc.la run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la @@ -38,7 +39,6 @@ run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la -run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la endif noinst_PROGRAMS = $(TESTS) From 2890dbb828e5b1d94ee5c5625080d7c8c074acb7 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 28 Dec 2010 22:25:11 +0000 Subject: [PATCH 11/21] another link order fix for static links git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4062 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/recurse/tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/recurse/tests/Makefile.am b/src/bin/recurse/tests/Makefile.am index 5e253ee579..b66d27fcbc 100644 --- a/src/bin/recurse/tests/Makefile.am +++ b/src/bin/recurse/tests/Makefile.am @@ -27,13 +27,13 @@ run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) run_unittests_LDADD = $(GTEST_LDADD) run_unittests_LDADD += $(SQLITE_LIBS) +run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la run_unittests_LDADD += $(top_builddir)/src/lib/datasrc/libdatasrc.la run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la -run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la endif From 313782b82421c97c4b21fcfb3d84350ad2fcec4e Mon Sep 17 00:00:00 2001 From: Jerry Date: Wed, 29 Dec 2010 02:01:53 +0000 Subject: [PATCH 12/21] branch for #453 (delegation processing and additional processing for NS) git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4065 e5f2f494-b856-4b98-b285-d166d9295462 From ec99fa2a77103688304f11c5721d932e836dc299 Mon Sep 17 00:00:00 2001 From: Jerry Date: Wed, 29 Dec 2010 04:01:52 +0000 Subject: [PATCH 13/21] unittest for delegation processing and additional processing for NS git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4067 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/tests/query_unittest.cc | 50 +++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index 21708f6746..edc3872032 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -31,6 +31,12 @@ using namespace isc::auth; RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"), RRClass::IN(), RRType::A(), RRTTL(3600))); +RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.a.example.com"), + RRClass::IN(), RRType::A(), + RRTTL(3600)))), +RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.aaaa.example.com"), + RRClass::IN(), RRType::AAAA(), + RRTTL(3600)))), namespace { // This is a mock Zone class for testing. // It is a derived class of Zone, and simply hardcode the results of find() @@ -41,8 +47,16 @@ namespace { // else return DNAME class MockZone : public Zone { public: - MockZone() : origin_(Name("example.com")) - {} + MockZone() : origin_(Name("example.com")), + ns_rrset(RRsetPtr(new RRset(Name("delegation.example.com"), + RRClass::IN(), RRType::NS(), + RRTTL(3600)))) + { + ns_rrset.addRdata(rdata::generic::NS( + Name("glue.a.example.com"))); + ns_rrset.addRdata(rdata::generic::NS( + Name("glue.aaaa.example.com"))); + } virtual const isc::dns::Name& getOrigin() const; virtual const isc::dns::RRClass& getClass() const; @@ -51,6 +65,7 @@ public: private: Name origin_; + RRsetPtr ns_rrset; }; const Name& @@ -67,17 +82,21 @@ Zone::FindResult MockZone::find(const Name& name, const RRType&) const { // hardcode the find results if (name == Name("www.example.com")) { - return FindResult(SUCCESS, a_rrset); + return (FindResult(SUCCESS, a_rrset)); + } else if (name == Name("glue.a.example.com")) { + return (FindResult(SUCCESS, glue_a_rrset)); + } else if (name == Name("glue.aaaa.example.com")) { + return (FindResult(SUCCESS, glue_aaaa_rrset)); } else if (name == Name("delegation.example.com")) { - return FindResult(DELEGATION, RRsetPtr()); + return (FindResult(DELEGATION, ns_rrset)); } else if (name == Name("nxdomain.example.com")) { - return FindResult(NXDOMAIN, RRsetPtr()); + return (FindResult(NXDOMAIN, RRsetPtr())); } else if (name == Name("nxrrset.example.com")) { return FindResult(NXRRSET, RRsetPtr()); - } else if (name == Name("cname.example.com")) { - return FindResult(CNAME, RRsetPtr()); + } else if ((name == Name("cname.example.com"))) { + return (FindResult(CNAME, RRsetPtr())); } else { - return FindResult(DNAME, RRsetPtr()); + return (FindResult(DNAME, RRsetPtr())); } } @@ -114,6 +133,21 @@ TEST_F(QueryTest, matchZone) { Name("www.example.com"), RRClass::IN(), RRType::A())); + // Delegation + const Name delegation_name(Name("delegation.example.com")); + Query delegation_query(memory_datasrc, delegation_name, qtype, response); + delegation_query.process(); + EXPECT_EQ(Rcode::NOERROR(), response.getRcode()); + EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY, + Name("delegation.example.com"), + RRClass::IN(), RRType::NS())); + EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL, + Name("glue.a.example.com"), + RRClass::IN(), RRType::A())); + EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL, + Name("glue.aaaa.example.com"), + RRClass::IN(), RRType::AAAA())); + // NXDOMAIN const Name nxdomain_name(Name("nxdomain.example.com")); Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response); From 8c5d4f5541187a19e83b9d9bf56dccdf6b028c04 Mon Sep 17 00:00:00 2001 From: Jerry Date: Wed, 29 Dec 2010 08:37:01 +0000 Subject: [PATCH 14/21] initial work for delegation processing (waiting for Zone cut handling) git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4071 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/query.cc | 40 +++++++++++++++++++++++++++- src/bin/auth/query.h | 30 +++++++++++++++++++++ src/bin/auth/tests/query_unittest.cc | 11 ++++---- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc index b2567e1a3f..387f1c950d 100644 --- a/src/bin/auth/query.cc +++ b/src/bin/auth/query.cc @@ -14,6 +14,7 @@ #include #include +#include #include @@ -21,10 +22,44 @@ using namespace isc::dns; using namespace isc::datasrc; +using namespace isc::dns::rdata; namespace isc { namespace auth { +void +Query::getAdditional(const isc::datasrc::Zone& zone, + const isc::dns::RRset& rrset) const +{ + if (rrset.getType() == RRType::NS()) { + // Need to perform the search in the "GLUE OK" mode. + RdataIteratorPtr rdata_iterator = rrset.getRdataIterator(); + for (; !rdata_iterator->isLast(); rdata_iterator->next()) { + const Rdata& rdata(rdata_iterator->getCurrent()); + const generic::NS& ns = dynamic_cast(rdata); + findAddrs(zone, ns.getNSName()); + } + } +} + +void +Query::findAddrs(const isc::datasrc::Zone& zone, + const isc::dns::Name& qname) const +{ + // Find A rrset + Zone::FindResult a_result = zone.find(qname, RRType::A()); + if (a_result.code == Zone::SUCCESS) { + response_.addRRset(Message::SECTION_ADDITIONAL, + boost::const_pointer_cast(a_result.rrset)); + } + // Find AAAA rrset + Zone::FindResult aaaa_result = zone.find(qname, RRType::AAAA()); + if (aaaa_result.code == Zone::SUCCESS) { + response_.addRRset(Message::SECTION_ADDITIONAL, + boost::const_pointer_cast(aaaa_result.rrset)); + } +} + void Query::process() const { bool keep_doing = true; @@ -56,7 +91,10 @@ Query::process() const { // TODO : fill in authority and addtional sections. break; case Zone::DELEGATION: - // TODO : add NS to authority section, fill in additional section. + response_.setRcode(Rcode::NOERROR()); + response_.addRRset(Message::SECTION_AUTHORITY, + boost::const_pointer_cast(db_result.rrset)); + getAdditional(*result.zone, *db_result.rrset); break; case Zone::NXDOMAIN: response_.setRcode(Rcode::NXDOMAIN()); diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h index f88554be5d..f1fe9a174e 100644 --- a/src/bin/auth/query.h +++ b/src/bin/auth/query.h @@ -19,10 +19,12 @@ namespace dns { class Message; class Name; class RRType; +class RRset; } namespace datasrc { class MemoryDataSrc; +class Zone; } namespace auth { @@ -104,6 +106,34 @@ public: /// future version. void process() const; + /// Look up additional data (i.e., address records for the names included + /// in NS or MX records). + /// + /// Right now this method never throws an exception. + /// + /// \param zone The Zone wherein the additional data to the query is bo be + /// found. + /// \param rrset The RRset (i.e., NS or MX rrset) which require additional + /// processing. + void getAdditional(const isc::datasrc::Zone& zone, + const isc::dns::RRset& rrset) const; + + /// Find address records for a specified name. + /// + /// Search the specified zone for AAAA/A RRs of each of the NS/MX RDATA + /// (domain name), and insert the found ones into the additional section + /// if address records are available. + /// + /// Note: we need to perform the search in the "GLUE OK" mode for NS RDATA, + /// which means that we should include A/AAAA RRs under a zone cut. + /// The glue records must exactly match the name in the NS RDATA, without + /// CNAME or wildcard processing. + /// + /// \param zone The Zone wherein the address records is to be found. + /// \param qname The name in rrset RDATA. + void findAddrs(const isc::datasrc::Zone& zone, + const isc::dns::Name& qname) const; + private: const isc::datasrc::MemoryDataSrc& memory_datasrc_; const isc::dns::Name& qname_; diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index edc3872032..6ddc4d0501 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -33,10 +34,10 @@ RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"), RRTTL(3600))); RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.a.example.com"), RRClass::IN(), RRType::A(), - RRTTL(3600)))), + RRTTL(3600)))); RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.aaaa.example.com"), RRClass::IN(), RRType::AAAA(), - RRTTL(3600)))), + RRTTL(3600)))); namespace { // This is a mock Zone class for testing. // It is a derived class of Zone, and simply hardcode the results of find() @@ -44,7 +45,7 @@ namespace { // return NXDOMAIN for "nxdomain.example.com", // return NXRRSET for "nxrrset.example.com", // return CNAME for "cname.example.com", -// else return DNAME +// otherwise return DNAME class MockZone : public Zone { public: MockZone() : origin_(Name("example.com")), @@ -52,9 +53,9 @@ public: RRClass::IN(), RRType::NS(), RRTTL(3600)))) { - ns_rrset.addRdata(rdata::generic::NS( + ns_rrset->addRdata(rdata::generic::NS( Name("glue.a.example.com"))); - ns_rrset.addRdata(rdata::generic::NS( + ns_rrset->addRdata(rdata::generic::NS( Name("glue.aaaa.example.com"))); } virtual const isc::dns::Name& getOrigin() const; From 819afa3dc6523c9765262452e4445f81440ea8dc Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 30 Dec 2010 02:51:19 +0000 Subject: [PATCH 15/21] update comments git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4087 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/query.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc index 387f1c950d..4392f987fc 100644 --- a/src/bin/auth/query.cc +++ b/src/bin/auth/query.cc @@ -33,6 +33,7 @@ Query::getAdditional(const isc::datasrc::Zone& zone, { if (rrset.getType() == RRType::NS()) { // Need to perform the search in the "GLUE OK" mode. + // Currently, we simply search the whole zone for address records. RdataIteratorPtr rdata_iterator = rrset.getRdataIterator(); for (; !rdata_iterator->isLast(); rdata_iterator->next()) { const Rdata& rdata(rdata_iterator->getCurrent()); From 01d4022b44729b53931f21f074dd7a8c443980b0 Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 30 Dec 2010 04:03:08 +0000 Subject: [PATCH 16/21] clear the AA bit for delegation reply git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4090 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/query.cc | 1 + src/bin/auth/tests/query_unittest.cc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc index 4392f987fc..e1eb5ef9f9 100644 --- a/src/bin/auth/query.cc +++ b/src/bin/auth/query.cc @@ -92,6 +92,7 @@ Query::process() const { // TODO : fill in authority and addtional sections. break; case Zone::DELEGATION: + response_.setHeaderFlag(Message::HEADERFLAG_AA, false); response_.setRcode(Rcode::NOERROR()); response_.addRRset(Message::SECTION_AUTHORITY, boost::const_pointer_cast(db_result.rrset)); diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index 6ddc4d0501..cc0321501c 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -129,6 +129,7 @@ TEST_F(QueryTest, matchZone) { // match qname, normal query memory_datasrc.addZone(ZonePtr(new MockZone())); query.process(); + EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA)); EXPECT_EQ(Rcode::NOERROR(), response.getRcode()); EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER, Name("www.example.com"), RRClass::IN(), @@ -138,6 +139,7 @@ TEST_F(QueryTest, matchZone) { const Name delegation_name(Name("delegation.example.com")); Query delegation_query(memory_datasrc, delegation_name, qtype, response); delegation_query.process(); + EXPECT_FALSE(response.getHeaderFlag(Message::HEADERFLAG_AA)); EXPECT_EQ(Rcode::NOERROR(), response.getRcode()); EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY, Name("delegation.example.com"), From f20193d545251670a23cc386753a41e129346b06 Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 30 Dec 2010 06:45:46 +0000 Subject: [PATCH 17/21] update code and unittest according to review comments git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4091 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/query.cc | 6 +++ src/bin/auth/query.h | 5 ++- src/bin/auth/tests/query_unittest.cc | 65 ++++++++++++++++++++-------- 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc index e1eb5ef9f9..38ac180f82 100644 --- a/src/bin/auth/query.cc +++ b/src/bin/auth/query.cc @@ -47,6 +47,12 @@ void Query::findAddrs(const isc::datasrc::Zone& zone, const isc::dns::Name& qname) const { + // Out of zone name + NameComparisonResult result = zone.getOrigin().compare(qname); + if ((result.getRelation() != NameComparisonResult::SUPERDOMAIN) && + (result.getRelation() != NameComparisonResult::EQUAL)) + return; + // Find A rrset Zone::FindResult a_result = zone.find(qname, RRType::A()); if (a_result.code == Zone::SUCCESS) { diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h index f1fe9a174e..b2ed067976 100644 --- a/src/bin/auth/query.h +++ b/src/bin/auth/query.h @@ -106,10 +106,12 @@ public: /// future version. void process() const; +private: /// Look up additional data (i.e., address records for the names included /// in NS or MX records). /// - /// Right now this method never throws an exception. + /// This method may throw a exception because its underlying methods may + /// throw exceptions. /// /// \param zone The Zone wherein the additional data to the query is bo be /// found. @@ -134,7 +136,6 @@ public: void findAddrs(const isc::datasrc::Zone& zone, const isc::dns::Name& qname) const; -private: const isc::datasrc::MemoryDataSrc& memory_datasrc_; const isc::dns::Name& qname_; const isc::dns::RRType& qtype_; diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index cc0321501c..86a96ec22e 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -32,12 +32,24 @@ using namespace isc::auth; RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"), RRClass::IN(), RRType::A(), RRTTL(3600))); -RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.a.example.com"), +RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"), RRClass::IN(), RRType::A(), RRTTL(3600)))); -RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.aaaa.example.com"), +RRsetPtr ns_rrset(RRsetPtr(new RRset(Name("ns.example.com"), + RRClass::IN(), RRType::NS(), + RRTTL(3600)))); +RRsetPtr delegation_rrset(RRsetPtr(new RRset(Name("delegation.example.com"), + RRClass::IN(), RRType::NS(), + RRTTL(3600)))); +RRsetPtr noglue_rrset(RRsetPtr(new RRset(Name("noglue.example.com"), + RRClass::IN(), RRType::A(), + RRTTL(3600)))); +RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"), RRClass::IN(), RRType::AAAA(), RRTTL(3600)))); +RRsetPtr cname_rrset(RRsetPtr(new RRset(Name("cname.example.com"), + RRClass::IN(), RRType::CNAME(), + RRTTL(3600)))); namespace { // This is a mock Zone class for testing. // It is a derived class of Zone, and simply hardcode the results of find() @@ -48,15 +60,18 @@ namespace { // otherwise return DNAME class MockZone : public Zone { public: - MockZone() : origin_(Name("example.com")), - ns_rrset(RRsetPtr(new RRset(Name("delegation.example.com"), - RRClass::IN(), RRType::NS(), - RRTTL(3600)))) + MockZone() : origin_(Name("example.com")) { - ns_rrset->addRdata(rdata::generic::NS( - Name("glue.a.example.com"))); - ns_rrset->addRdata(rdata::generic::NS( - Name("glue.aaaa.example.com"))); + delegation_rrset->addRdata(rdata::generic::NS( + Name("glue.ns.example.com"))); + delegation_rrset->addRdata(rdata::generic::NS( + Name("noglue.example.com"))); + delegation_rrset->addRdata(rdata::generic::NS( + Name("cname.example.com"))); + delegation_rrset->addRdata(rdata::generic::NS( + Name("example.org"))); + cname_rrset->addRdata(rdata::generic::CNAME( + Name("www.example.com"))); } virtual const isc::dns::Name& getOrigin() const; virtual const isc::dns::RRClass& getClass() const; @@ -66,7 +81,6 @@ public: private: Name origin_; - RRsetPtr ns_rrset; }; const Name& @@ -80,22 +94,26 @@ MockZone::getClass() const { } Zone::FindResult -MockZone::find(const Name& name, const RRType&) const { +MockZone::find(const Name& name, const RRType& type) const { // hardcode the find results if (name == Name("www.example.com")) { return (FindResult(SUCCESS, a_rrset)); - } else if (name == Name("glue.a.example.com")) { + } else if (name == Name("glue.ns.example.com") && type == RRType::A()) { return (FindResult(SUCCESS, glue_a_rrset)); - } else if (name == Name("glue.aaaa.example.com")) { + } else if (name == Name("noglue.example.com") && type == RRType::A()) { + return (FindResult(SUCCESS, noglue_rrset)); + } else if (name == Name("glue.ns.example.com") && type == RRType::AAAA()) { return (FindResult(SUCCESS, glue_aaaa_rrset)); } else if (name == Name("delegation.example.com")) { + return (FindResult(DELEGATION, delegation_rrset)); + } else if (name == Name("ns.example.com")) { return (FindResult(DELEGATION, ns_rrset)); } else if (name == Name("nxdomain.example.com")) { return (FindResult(NXDOMAIN, RRsetPtr())); } else if (name == Name("nxrrset.example.com")) { return FindResult(NXRRSET, RRsetPtr()); } else if ((name == Name("cname.example.com"))) { - return (FindResult(CNAME, RRsetPtr())); + return (FindResult(CNAME, cname_rrset)); } else { return (FindResult(DNAME, RRsetPtr())); } @@ -144,12 +162,25 @@ TEST_F(QueryTest, matchZone) { EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY, Name("delegation.example.com"), RRClass::IN(), RRType::NS())); + // glue address records EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL, - Name("glue.a.example.com"), + Name("glue.ns.example.com"), RRClass::IN(), RRType::A())); EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL, - Name("glue.aaaa.example.com"), + Name("glue.ns.example.com"), RRClass::IN(), RRType::AAAA())); + // noglue address records + EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL, + Name("noglue.example.com"), + RRClass::IN(), RRType::A())); + // NS name has a CNAME + EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL, + Name("www.example.com"), + RRClass::IN(), RRType::A())); + // NS name is out of zone + EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL, + Name("example.org"), + RRClass::IN(), RRType::A())); // NXDOMAIN const Name nxdomain_name(Name("nxdomain.example.com")); From 6fa2e86bfd60a417c37cc4a1ff768c0f21b47565 Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 30 Dec 2010 09:00:36 +0000 Subject: [PATCH 18/21] fix code style git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4092 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/query.h | 59 ++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h index b2ed067976..2532aa91ee 100644 --- a/src/bin/auth/query.h +++ b/src/bin/auth/query.h @@ -62,6 +62,36 @@ namespace auth { /// accidentally, and since it's considered a temporary development state, /// we keep this name at the moment. class Query { +private: + /// Look up additional data (i.e., address records for the names included + /// in NS or MX records). + /// + /// This method may throw a exception because its underlying methods may + /// throw exceptions. + /// + /// \param zone The Zone wherein the additional data to the query is bo be + /// found. + /// \param rrset The RRset (i.e., NS or MX rrset) which require additional + /// processing. + void getAdditional(const isc::datasrc::Zone& zone, + const isc::dns::RRset& rrset) const; + + /// Find address records for a specified name. + /// + /// Search the specified zone for AAAA/A RRs of each of the NS/MX RDATA + /// (domain name), and insert the found ones into the additional section + /// if address records are available. + /// + /// Note: we need to perform the search in the "GLUE OK" mode for NS RDATA, + /// which means that we should include A/AAAA RRs under a zone cut. + /// The glue records must exactly match the name in the NS RDATA, without + /// CNAME or wildcard processing. + /// + /// \param zone The Zone wherein the address records is to be found. + /// \param qname The name in rrset RDATA. + void findAddrs(const isc::datasrc::Zone& zone, + const isc::dns::Name& qname) const; + public: /// Constructor from query parameters. /// @@ -107,35 +137,6 @@ public: void process() const; private: - /// Look up additional data (i.e., address records for the names included - /// in NS or MX records). - /// - /// This method may throw a exception because its underlying methods may - /// throw exceptions. - /// - /// \param zone The Zone wherein the additional data to the query is bo be - /// found. - /// \param rrset The RRset (i.e., NS or MX rrset) which require additional - /// processing. - void getAdditional(const isc::datasrc::Zone& zone, - const isc::dns::RRset& rrset) const; - - /// Find address records for a specified name. - /// - /// Search the specified zone for AAAA/A RRs of each of the NS/MX RDATA - /// (domain name), and insert the found ones into the additional section - /// if address records are available. - /// - /// Note: we need to perform the search in the "GLUE OK" mode for NS RDATA, - /// which means that we should include A/AAAA RRs under a zone cut. - /// The glue records must exactly match the name in the NS RDATA, without - /// CNAME or wildcard processing. - /// - /// \param zone The Zone wherein the address records is to be found. - /// \param qname The name in rrset RDATA. - void findAddrs(const isc::datasrc::Zone& zone, - const isc::dns::Name& qname) const; - const isc::datasrc::MemoryDataSrc& memory_datasrc_; const isc::dns::Name& qname_; const isc::dns::RRType& qtype_; From 78a330249aa56fe6130c506b455b1c97046b877e Mon Sep 17 00:00:00 2001 From: Jerry Date: Fri, 31 Dec 2010 01:13:56 +0000 Subject: [PATCH 19/21] update MockZone according to review comments git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac453@4106 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/auth/tests/query_unittest.cc | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index 86a96ec22e..b6ec41aba8 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -32,24 +32,18 @@ using namespace isc::auth; RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"), RRClass::IN(), RRType::A(), RRTTL(3600))); -RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"), - RRClass::IN(), RRType::A(), - RRTTL(3600)))); RRsetPtr ns_rrset(RRsetPtr(new RRset(Name("ns.example.com"), RRClass::IN(), RRType::NS(), RRTTL(3600)))); -RRsetPtr delegation_rrset(RRsetPtr(new RRset(Name("delegation.example.com"), - RRClass::IN(), RRType::NS(), - RRTTL(3600)))); -RRsetPtr noglue_rrset(RRsetPtr(new RRset(Name("noglue.example.com"), +RRsetPtr glue_a_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"), RRClass::IN(), RRType::A(), RRTTL(3600)))); RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"), RRClass::IN(), RRType::AAAA(), RRTTL(3600)))); -RRsetPtr cname_rrset(RRsetPtr(new RRset(Name("cname.example.com"), - RRClass::IN(), RRType::CNAME(), - RRTTL(3600)))); +RRsetPtr noglue_a_rrset(RRsetPtr(new RRset(Name("noglue.example.com"), + RRClass::IN(), RRType::A(), + RRTTL(3600)))); namespace { // This is a mock Zone class for testing. // It is a derived class of Zone, and simply hardcode the results of find() @@ -60,7 +54,14 @@ namespace { // otherwise return DNAME class MockZone : public Zone { public: - MockZone() : origin_(Name("example.com")) + MockZone() : + origin_(Name("example.com")), + delegation_rrset(RRsetPtr(new RRset(Name("delegation.example.com"), + RRClass::IN(), RRType::NS(), + RRTTL(3600)))), + cname_rrset(RRsetPtr(new RRset(Name("cname.example.com"), + RRClass::IN(), RRType::CNAME(), + RRTTL(3600)))) { delegation_rrset->addRdata(rdata::generic::NS( Name("glue.ns.example.com"))); @@ -81,6 +82,8 @@ public: private: Name origin_; + RRsetPtr delegation_rrset; + RRsetPtr cname_rrset; }; const Name& @@ -101,7 +104,7 @@ MockZone::find(const Name& name, const RRType& type) const { } else if (name == Name("glue.ns.example.com") && type == RRType::A()) { return (FindResult(SUCCESS, glue_a_rrset)); } else if (name == Name("noglue.example.com") && type == RRType::A()) { - return (FindResult(SUCCESS, noglue_rrset)); + return (FindResult(SUCCESS, noglue_a_rrset)); } else if (name == Name("glue.ns.example.com") && type == RRType::AAAA()) { return (FindResult(SUCCESS, glue_aaaa_rrset)); } else if (name == Name("delegation.example.com")) { From 9c13aed74ec4c68bee8dfe4a7a286a6db6a36a03 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Jan 2011 19:40:16 +0000 Subject: [PATCH 20/21] changelog for trac #448 git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@4134 e5f2f494-b856-4b98-b285-d166d9295462 --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index d31cc02215..e2b2bff153 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ + 143. [build] jinmei + Fixed build problems with clang++ in unit tests due to recent + changes. No behavior change. (Trac #448, svn r4133) + 142. [func] jinmei b10-auth: updated query benchmark so that it can test in memory data source. Also fixed a bug that the output buffer isn't From db2a8bd66e46e76c62cfba8c32bde1d304b14dfa Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Jan 2011 20:20:08 +0000 Subject: [PATCH 21/21] removed srv_unittest.h from libtestutils_la_SOURCES (we don't need the header file and it has been removed from the repository) git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@4137 e5f2f494-b856-4b98-b285-d166d9295462 --- src/lib/testutils/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index f0e172d788..b1962abc60 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -8,7 +8,6 @@ if HAVE_GTEST lib_LTLIBRARIES = libtestutils.la libtestutils_la_SOURCES = srv_test.h srv_test.cc -libtestutils_la_SOURCES += srv_unittest.h libtestutils_la_SOURCES += mockups.h libtestutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) endif