[svn] commit: r4070 - in /branches/trac462/src/bin/auth: auth_srv.cc tests/auth_srv_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Dec 29 07:07:43 UTC 2010
Author: jinmei
Date: Wed Dec 29 07:07:43 2010
New Revision: 4070
Log:
fixed the trailing garbage bug: b10-auth's MessageAnswer() shouldn't do anything because it builds everything in the process methods.
test cases were added for the bug, too.
Modified:
branches/trac462/src/bin/auth/auth_srv.cc
branches/trac462/src/bin/auth/tests/auth_srv_unittest.cc
Modified: branches/trac462/src/bin/auth/auth_srv.cc
==============================================================================
--- branches/trac462/src/bin/auth/auth_srv.cc (original)
+++ branches/trac462/src/bin/auth/auth_srv.cc Wed Dec 29 07:07:43 2010
@@ -162,33 +162,20 @@
AuthSrv* server_;
};
-// This is a derived class of \c DNSAnswer, to serve as a
-// callback in the asiolink module. It takes a completed
-// set of answer data from the DNS lookup and assembles it
-// into a wire-format response.
+// This is a derived class of \c DNSAnswer, to serve as a callback in the
+// asiolink module. We actually shouldn't do anything in this class because
+// we build complete response messages in the process methods; otherwise
+// the response message will contain trailing garbage. In future, we should
+// probably even drop the reliance on DNSAnswer. We don't need the coroutine
+// tricks provided in that framework, and its overhead would be significant
+// in terms of performance consideration for the authoritative server
+// implementation.
class MessageAnswer : public DNSAnswer {
public:
- MessageAnswer(AuthSrv* srv) : server_(srv) {}
- virtual void operator()(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer) const
- {
- MessageRenderer renderer(*buffer);
- if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
- ConstEDNSPtr edns(message->getEDNS());
- renderer.setLengthLimit(edns ? edns->getUDPSize() :
- Message::DEFAULT_MAX_UDPSIZE);
- } else {
- renderer.setLengthLimit(65535);
- }
- message->toWire(renderer);
- if (server_->getVerbose()) {
- cerr << "[b10-auth] sending a response (" << renderer.getLength()
- << " bytes):\n" << message->toText() << endl;
- }
- }
-
-private:
- AuthSrv* server_;
+ MessageAnswer(AuthSrv*) {}
+ virtual void operator()(const IOMessage&, MessagePtr,
+ OutputBufferPtr) const
+ {}
};
// This is a derived class of \c SimpleCallback, to serve
Modified: branches/trac462/src/bin/auth/tests/auth_srv_unittest.cc
==============================================================================
--- branches/trac462/src/bin/auth/tests/auth_srv_unittest.cc (original)
+++ branches/trac462/src/bin/auth/tests/auth_srv_unittest.cc Wed Dec 29 07:07:43 2010
@@ -15,6 +15,19 @@
// $Id$
#include <config.h>
+
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include <dns/message.h>
+#include <dns/messagerenderer.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <dns/rdataclass.h>
+
#include <datasrc/memory_datasrc.h>
#include <auth/auth_srv.h>
#include <testutils/srv_unittest.h>
@@ -22,6 +35,7 @@
using namespace isc::cc;
using namespace isc::dns;
+using namespace isc::dns::rdata;
using namespace isc::data;
using namespace isc::xfr;
using namespace asiolink;
@@ -45,7 +59,103 @@
MockXfroutClient xfrout;
AuthSrv server;
const RRClass rrclass;
+ vector<uint8_t> response_data;
};
+
+// A helper function that builds a response to version.bind/TXT/CH that
+// should be identical to the response from our builtin (static) data source
+// by default. The resulting wire-format data will be stored in 'data'.
+void
+createBuiltinVersionResponse(const qid_t qid, vector<uint8_t>& data) {
+ const Name version_name("version.bind");
+ Message message(Message::RENDER);
+
+ UnitTestUtil::createRequestMessage(message, Opcode::QUERY(),
+ qid, version_name,
+ RRClass::CH(), RRType::TXT());
+ message.setHeaderFlag(Message::HEADERFLAG_QR);
+ message.setHeaderFlag(Message::HEADERFLAG_AA);
+ RRsetPtr rrset_version = RRsetPtr(new RRset(version_name, RRClass::CH(),
+ RRType::TXT(), RRTTL(0)));
+ rrset_version->addRdata(generic::TXT(PACKAGE_STRING));
+ message.addRRset(Message::SECTION_ANSWER, rrset_version);
+
+ RRsetPtr rrset_version_ns = RRsetPtr(new RRset(version_name, RRClass::CH(),
+ RRType::NS(), RRTTL(0)));
+ rrset_version_ns->addRdata(generic::NS(version_name));
+ message.addRRset(Message::SECTION_AUTHORITY, rrset_version_ns);
+
+ OutputBuffer obuffer(0);
+ MessageRenderer renderer(obuffer);
+ message.toWire(renderer);
+
+ data.clear();
+ data.assign(static_cast<const uint8_t*>(renderer.getData()),
+ static_cast<const uint8_t*>(renderer.getData()) +
+ renderer.getLength());
+}
+
+// In the following tests we confirm the response data is rendered in
+// wire format in the expected way.
+
+// The most primitive check: checking the result of the processMessage()
+// method
+TEST_F(AuthSrvTest, builtInQuery) {
+ UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+ default_qid, Name("version.bind"),
+ RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP);
+ server.processMessage(*io_message, parse_message, response_obuffer,
+ &dnsserv);
+ createBuiltinVersionResponse(default_qid, response_data);
+ EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+ response_obuffer->getData(),
+ response_obuffer->getLength(),
+ &response_data[0], response_data.size());
+}
+
+// Same test emulating the UDPServer class behavior (defined in libasiolink).
+// This is not a good test in that it assumes internal implementation details
+// of UDPServer, but we've encountered a regression due to the introduction
+// of that class, so we add a test for that case to prevent such a regression
+// in future.
+// Besides, the generalization of UDPServer is probably too much for the
+// authoritative only server in terms of performance, and it's quite likely
+// we need to drop it for the authoritative server implementation.
+// At that point we can drop this test, too.
+TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
+ UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+ default_qid, Name("version.bind"),
+ RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP);
+
+ (*server.getDNSLookupProvider())(*io_message, parse_message,
+ response_obuffer, &dnsserv);
+ (*server.getDNSAnswerProvider())(*io_message, parse_message,
+ response_obuffer);
+
+ createBuiltinVersionResponse(default_qid, response_data);
+ EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+ response_obuffer->getData(),
+ response_obuffer->getLength(),
+ &response_data[0], response_data.size());
+}
+
+// Same type of test as builtInQueryViaDNSServer but for an error response.
+TEST_F(AuthSrvTest, iqueryViaDNSServer) {
+ createDataFromFile("iquery_fromWire.wire");
+ (*server.getDNSLookupProvider())(*io_message, parse_message,
+ response_obuffer, &dnsserv);
+ (*server.getDNSAnswerProvider())(*io_message, parse_message,
+ response_obuffer);
+
+ UnitTestUtil::readWireData("iquery_response_fromWire.wire",
+ response_data);
+ EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+ response_obuffer->getData(),
+ response_obuffer->getLength(),
+ &response_data[0], response_data.size());
+}
// Unsupported requests. Should result in NOTIMP.
TEST_F(AuthSrvTest, unsupportedRequest) {
More information about the bind10-changes
mailing list