Merge branch 'master' of ssh://bind10.isc.org/var/bind10/git/bind10

ChangeLog

@@ -1,3 +1,14 @@
+  186.  [bug]		jelte
+	b10-resolver could stop with an assertion failure on certain kinds
+	of messages (there was a problem in error message creation). This
+	fixes that.
+	(Trac #607, git 25a5f4ec755bc09b54410fcdff22691283147f32)
+
+  185.  [bug]        vorner
+	Tests use port from private range (53210), lowering chance of
+	a conflict with something else (eg. running bind 10).
+	(Trac #523, git 301da7d26d41e64d87c0cf72727f3347aa61fb40)
+
   184.  [func]*      vorner
 	Listening address and port configuration of b10-auth is the same as
 	for b10-resolver now. That means, it is configured through bindctl

src/bin/auth/benchmarks/query_bench.cc

@@ -77,7 +77,7 @@ protected:
         dummy_socket(IOSocket::getDummyUDPSocket()),
         dummy_endpoint(IOEndpointPtr(IOEndpoint::create(IPPROTO_UDP,
                                                         IOAddress("192.0.2.1"),
-                                                        5300)))
+                                                        53210)))
     {}
 public:
     unsigned int run() {

src/bin/auth/tests/auth_srv_unittest.cc

@@ -644,7 +644,7 @@ TEST_F(AuthSrvTest, queryCounterUnexpected) {
     // Modify the message.
     delete io_message;
     endpoint = IOEndpoint::create(IPPROTO_UDP,
-                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 5300);
+                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 53210);
     io_message = new IOMessage(request_renderer.getData(),
                                request_renderer.getLength(),
                                getDummyUnknownSocket(), *endpoint);

src/bin/resolver/resolver.cc

@@ -185,8 +185,8 @@ public:
 
 // TODO: REMOVE, USE isc::resolve::MakeErrorMessage?
 void
-makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
-                 const Rcode& rcode)
+makeErrorMessage(MessagePtr message, MessagePtr answer_message,
+                 OutputBufferPtr buffer, const Rcode& rcode)
 {
     // extract the parameters that should be kept.
     // XXX: with the current implementation, it's not easy to set EDNS0
@@ -197,6 +197,12 @@ makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
     const Opcode& opcode = message->getOpcode();
     vector<QuestionPtr> questions;
 
+    // answer_message is actually ignored right now,
+    // see the comment in #607
+    answer_message->setRcode(rcode);
+    answer_message->setOpcode(opcode);
+    answer_message->setQid(qid);
+
     // If this is an error to a query or notify, we should also copy the
     // question section.
     if (opcode == Opcode::QUERY() || opcode == Opcode::NOTIFY()) {
@@ -385,12 +391,14 @@ Resolver::processMessage(const IOMessage& io_message,
     } catch (const DNSProtocolError& error) {
         dlog(string("returning ") + error.getRcode().toText() + ": " + 
             error.what());
-        makeErrorMessage(query_message, buffer, error.getRcode());
+        makeErrorMessage(query_message, answer_message,
+                         buffer, error.getRcode());
         server->resume(true);
         return;
     } catch (const Exception& ex) {
         dlog(string("returning SERVFAIL: ") + ex.what());
-        makeErrorMessage(query_message, buffer, Rcode::SERVFAIL());
+        makeErrorMessage(query_message, answer_message,
+                         buffer, Rcode::SERVFAIL());
         server->resume(true);
         return;
     } // other exceptions will be handled at a higher layer.
@@ -400,28 +408,34 @@ Resolver::processMessage(const IOMessage& io_message,
     // Perform further protocol-level validation.
     bool sendAnswer = true;
     if (query_message->getOpcode() == Opcode::NOTIFY()) {
-        makeErrorMessage(query_message, buffer, Rcode::NOTAUTH());
+        makeErrorMessage(query_message, answer_message,
+                         buffer, Rcode::NOTAUTH());
         dlog("Notify arrived, but we are not authoritative");
     } else if (query_message->getOpcode() != Opcode::QUERY()) {
         dlog("Unsupported opcode (got: " + query_message->getOpcode().toText() +
             ", expected: " + Opcode::QUERY().toText());
-        makeErrorMessage(query_message, buffer, Rcode::NOTIMP());
+        makeErrorMessage(query_message, answer_message,
+                         buffer, Rcode::NOTIMP());
     } else if (query_message->getRRCount(Message::SECTION_QUESTION) != 1) {
         dlog("The query contained " +
             boost::lexical_cast<string>(query_message->getRRCount(
             Message::SECTION_QUESTION) + " questions, exactly one expected"));
-        makeErrorMessage(query_message, buffer, Rcode::FORMERR());
+        makeErrorMessage(query_message, answer_message,
+                         buffer, Rcode::FORMERR());
     } else {
         ConstQuestionPtr question = *query_message->beginQuestion();
         const RRType &qtype = question->getType();
         if (qtype == RRType::AXFR()) {
             if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
-                makeErrorMessage(query_message, buffer, Rcode::FORMERR());
+                makeErrorMessage(query_message, answer_message,
+                                 buffer, Rcode::FORMERR());
             } else {
-                makeErrorMessage(query_message, buffer, Rcode::NOTIMP());
+                makeErrorMessage(query_message, answer_message,
+                                 buffer, Rcode::NOTIMP());
             }
         } else if (qtype == RRType::IXFR()) {
-            makeErrorMessage(query_message, buffer, Rcode::NOTIMP());
+            makeErrorMessage(query_message, answer_message,
+                             buffer, Rcode::NOTIMP());
         } else {
             // The RecursiveQuery object will post the "resume" event to the
             // DNSServer when an answer arrives, so we don't have to do it now.

src/bin/resolver/tests/resolver_unittest.cc

@@ -96,6 +96,27 @@ TEST_F(ResolverTest, AXFRFail) {
                 QR_FLAG, 1, 0, 0, 0);
 }
 
+TEST_F(ResolverTest, IXFRFail) {
+    UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
+                                       Name("example.com"), RRClass::IN(),
+                                       RRType::IXFR());
+    createRequestPacket(request_message, IPPROTO_TCP);
+    // IXFR is not implemented and should always send NOTIMP.
+    server.processMessage(*io_message,
+                          parse_message,
+                          response_message,
+                          response_obuffer,
+                          &dnsserv);
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    // the second check is what we'll need in the end (with the values
+    // from the first one), but right now the first one is for what
+    // will actually be returned to the client
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(), opcode.getCode(),
+                QR_FLAG, 1, 0, 0, 0);
+    headerCheck(*response_message, default_qid, Rcode::NOTIMP(), opcode.getCode(),
+                0, 0, 0, 0, 0);
+}
+
 TEST_F(ResolverTest, notifyFail) {
     // Notify should always return NOTAUTH
     request_message.clear(Message::RENDER);

src/lib/asiolink/tests/io_endpoint_unittest.cc

@@ -22,9 +22,9 @@ using namespace asiolink;
 
 TEST(IOEndpointTest, createUDPv4) {
     const IOEndpoint* ep;
-    ep = IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5300);
+    ep = IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 53210);
     EXPECT_EQ("192.0.2.1", ep->getAddress().toText());
-    EXPECT_EQ(5300, ep->getPort());
+    EXPECT_EQ(53210, ep->getPort());
     EXPECT_EQ(AF_INET, ep->getFamily());
     EXPECT_EQ(AF_INET, ep->getAddress().getFamily());
     EXPECT_EQ(IPPROTO_UDP, ep->getProtocol());
@@ -62,7 +62,7 @@ TEST(IOEndpointTest, createTCPv6) {
 
 TEST(IOEndpointTest, createIPProto) {
     EXPECT_THROW(IOEndpoint::create(IPPROTO_IP, IOAddress("192.0.2.1"),
-                                    5300)->getAddress().toText(),
+                                    53210)->getAddress().toText(),
                  IOError);
 }
 

src/lib/asiolink/tests/io_service_unittest.cc

@@ -28,7 +28,7 @@ const char* const TEST_IPV4_ADDR = "127.0.0.1";
 TEST(IOServiceTest, badPort) {
     IOService io_service;
     EXPECT_THROW(DNSService(io_service, *"65536", true, false, NULL, NULL, NULL), IOError);
-    EXPECT_THROW(DNSService(io_service, *"5300.0", true, false, NULL, NULL, NULL), IOError);
+    EXPECT_THROW(DNSService(io_service, *"53210.0", true, false, NULL, NULL, NULL), IOError);
     EXPECT_THROW(DNSService(io_service, *"-1", true, false, NULL, NULL, NULL), IOError);
     EXPECT_THROW(DNSService(io_service, *"domain", true, false, NULL, NULL, NULL), IOError);
 }

src/lib/testutils/portconfig.h

@@ -51,8 +51,8 @@ listenAddresses(Server& server) {
 
     // Try putting there some addresses
     AddressList addresses;
-    addresses.push_back(AddressPair("127.0.0.1", 5321));
-    addresses.push_back(AddressPair("::1", 5321));
+    addresses.push_back(AddressPair("127.0.0.1", 53210));
+    addresses.push_back(AddressPair("::1", 53210));
     server.setListenAddresses(addresses);
     EXPECT_EQ(2, server.getListenAddresses().size());
     EXPECT_EQ("::1", server.getListenAddresses()[1].first);
@@ -85,7 +85,7 @@ listenAddressConfig(Server& server) {
                                         "\"listen_on\": ["
                                         "   {"
                                         "       \"address\": \"127.0.0.1\","
-                                        "       \"port\": 5321"
+                                        "       \"port\": 53210"
                                         "   }"
                                         "]"
                                         "}"));
@@ -93,7 +93,7 @@ listenAddressConfig(Server& server) {
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
     ASSERT_EQ(1, server.getListenAddresses().size());
     EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
-    EXPECT_EQ(5321, server.getListenAddresses()[0].second);
+    EXPECT_EQ(53210, server.getListenAddresses()[0].second);
 
     // As this is example address, the machine should not have it on
     // any interface
@@ -101,7 +101,7 @@ listenAddressConfig(Server& server) {
                                "\"listen_on\": ["
                                "   {"
                                "       \"address\": \"192.0.2.0\","
-                               "       \"port\": 5321"
+                               "       \"port\": 53210"
                                "   }"
                                "]"
                                "}");
@@ -109,7 +109,7 @@ listenAddressConfig(Server& server) {
     EXPECT_FALSE(result->equals(*isc::config::createAnswer()));
     ASSERT_EQ(1, server.getListenAddresses().size());
     EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
-    EXPECT_EQ(5321, server.getListenAddresses()[0].second);
+    EXPECT_EQ(53210, server.getListenAddresses()[0].second);
 
 }
 

src/lib/testutils/srv_test.cc

@@ -60,7 +60,7 @@ SrvTestBase::createDataFromFile(const char* const datafile,
     delete endpoint;
 
     endpoint = IOEndpoint::create(protocol,
-                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 5300);
+                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 53210);
     UnitTestUtil::readWireData(datafile, data);
     io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() :
         &IOSocket::getDummyTCPSocket();
@@ -76,7 +76,7 @@ SrvTestBase::createRequestPacket(Message& message,
     delete io_message;
 
     endpoint = IOEndpoint::create(protocol,
-                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 5300);
+                                  IOAddress(DEFAULT_REMOTE_ADDRESS), 53210);
     io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() :
         &IOSocket::getDummyTCPSocket();
     io_message = new IOMessage(request_renderer.getData(),