Browse Source

use answer_message for final answer

use the answer_message Message object instead of parsing it out of the
buffer again when constructing the final answer
Jelte Jansen 14 years ago
parent
commit
e9c8b1402a
4 changed files with 64 additions and 46 deletions
  1. 13 40
      src/bin/resolver/resolver.cc
  2. 49 4
      src/lib/asiolink/asiolink.cc
  3. 1 1
      src/lib/asiolink/tcpdns.cc
  4. 1 1
      src/lib/asiolink/udpdns.cc

+ 13 - 40
src/bin/resolver/resolver.cc

@@ -148,6 +148,7 @@ public:
     MessagePtr message_;
 };
 
+// TODO remove?
 class SectionInserter {
 public:
     SectionInserter(MessagePtr message, const Message::Section sect) :
@@ -215,7 +216,6 @@ public:
                             OutputBufferPtr buffer,
                             DNSServer* server) const
     {
-        (void) answer_message,
         server_->processMessage(io_message, message, answer_message,
                                 buffer, server);
     }
@@ -241,47 +241,20 @@ public:
         const Rcode& rcode = message->getRcode();
         vector<QuestionPtr> questions;
         questions.assign(message->beginQuestion(), message->endQuestion());
-        (void)answer_message;
-        
-        message->clear(Message::RENDER);
-        message->setQid(qid);
-        message->setOpcode(opcode);
-        message->setRcode(rcode);
-
-        message->setHeaderFlag(Message::HEADERFLAG_QR);
-        message->setHeaderFlag(Message::HEADERFLAG_RA);
+
+        // Fill in the final details of the answer message
+        //message->clear(Message::RENDER);
+        answer_message->setQid(qid);
+        answer_message->setOpcode(opcode);
+        answer_message->setRcode(rcode);
+
+        answer_message->setHeaderFlag(Message::HEADERFLAG_QR);
+        answer_message->setHeaderFlag(Message::HEADERFLAG_RA);
         if (rd) {
-            message->setHeaderFlag(Message::HEADERFLAG_RD);
+            answer_message->setHeaderFlag(Message::HEADERFLAG_RD);
         }
         if (cd) {
-            message->setHeaderFlag(Message::HEADERFLAG_CD);
-        }
-
-
-        // Copy the question section.
-        for_each(questions.begin(), questions.end(), QuestionInserter(message));
-
-        // If the buffer already has an answer in it, copy RRsets from
-        // that into the new message, then clear the buffer and render
-        // the new message into it.
-        if (buffer->getLength() != 0) {
-            try {
-                Message incoming(Message::PARSE);
-                InputBuffer ibuf(buffer->getData(), buffer->getLength());
-                incoming.fromWire(ibuf);
-                for_each(incoming.beginSection(Message::SECTION_ANSWER),
-                         incoming.endSection(Message::SECTION_ANSWER),
-                         SectionInserter(message, Message::SECTION_ANSWER));
-                for_each(incoming.beginSection(Message::SECTION_AUTHORITY),
-                         incoming.endSection(Message::SECTION_AUTHORITY),
-                         SectionInserter(message, Message::SECTION_AUTHORITY));
-                for_each(incoming.beginSection(Message::SECTION_ADDITIONAL),
-                         incoming.endSection(Message::SECTION_ADDITIONAL),
-                         SectionInserter(message, Message::SECTION_ADDITIONAL));
-            } catch (const Exception& ex) {
-                // Incoming message couldn't be read, we just SERVFAIL
-                message->setRcode(Rcode::SERVFAIL());
-            }
+            answer_message->setHeaderFlag(Message::HEADERFLAG_CD);
         }
 
         // Now we can clear the buffer and render the new message into it
@@ -296,7 +269,7 @@ public:
             renderer.setLengthLimit(65535);
         }
 
-        message->toWire(renderer);
+        answer_message->toWire(renderer);
 
         dlog(string("sending a response (") +
             boost::lexical_cast<string>(renderer.getLength()) + "bytes): \n" +

+ 49 - 4
src/lib/asiolink/asiolink.cc

@@ -50,6 +50,44 @@ using namespace isc::dns;
 using isc::log::dlog;
 using namespace boost;
 
+// Is this something we can use in libdns++?
+namespace {
+    class SectionInserter {
+    public:
+        SectionInserter(MessagePtr message, const Message::Section sect) :
+            message_(message), section_(sect)
+        {}
+        void operator()(const RRsetPtr rrset) {
+            //dlog("Adding RRSet to message section " +
+            //    boost::lexical_cast<string>(section_));
+            message_->addRRset(section_, rrset, true);
+        }
+        MessagePtr message_;
+        const Message::Section section_;
+    };
+
+
+    /// \brief Copies the parts relevant for a DNS answer to the
+    /// target message
+    ///
+    /// This adds all the RRsets in the answer, authority and
+    /// additional sections to the target, as well as the response
+    /// code
+    void copyAnswerMessage(const Message& source, MessagePtr target) {
+        target->setRcode(source.getRcode());
+
+        for_each(source.beginSection(Message::SECTION_ANSWER),
+                 source.endSection(Message::SECTION_ANSWER),
+                 SectionInserter(target, Message::SECTION_ANSWER));
+        for_each(source.beginSection(Message::SECTION_AUTHORITY),
+                 source.endSection(Message::SECTION_AUTHORITY),
+                 SectionInserter(target, Message::SECTION_AUTHORITY));
+        for_each(source.beginSection(Message::SECTION_ADDITIONAL),
+                 source.endSection(Message::SECTION_ADDITIONAL),
+                 SectionInserter(target, Message::SECTION_ADDITIONAL));
+    }
+}
+
 namespace asiolink {
 
 typedef pair<string, uint16_t> addr_t;
@@ -302,6 +340,10 @@ private:
 
     // Info for (re)sending the query (the question and destination)
     Question question_;
+
+    // This is where we build and store our final answer
+    MessagePtr answer_message_;
+
     // currently we use upstream as the current list of NS records
     // we should differentiate between forwarding and resolving
     shared_ptr<AddressVector> upstream_;
@@ -349,11 +391,12 @@ private:
     }
 public:
     RunningQuery(asio::io_service& io, const Question &question,
-        shared_ptr<AddressVector> upstream,
+        MessagePtr answer_message, shared_ptr<AddressVector> upstream,
         OutputBufferPtr buffer, DNSServer* server, int timeout,
         unsigned retries) :
         io_(io),
         question_(question),
+        answer_message_(answer_message),
         upstream_(upstream),
         buffer_(buffer),
         server_(server->clone()),
@@ -390,6 +433,7 @@ public:
             if (incoming.getRcode() == Rcode::NOERROR()) {
                 if (incoming.getRRCount(Message::SECTION_ANSWER) > 0) {
                     dlog("[XX] this looks like the final result");
+                    copyAnswerMessage(incoming, answer_message_);
                     done = true;
                 } else {
                     dlog("[XX] this looks like a delegation");
@@ -434,10 +478,12 @@ public:
                     } else {
                         dlog("[XX] no ready-made addresses in additional. need nsas.");
                         // this will result in answering with the delegation. oh well
+                        copyAnswerMessage(incoming, answer_message_);
                         done = true;
                     }
                 }
             } else {
+                copyAnswerMessage(incoming, answer_message_);
                 done = true;
             }
             
@@ -464,9 +510,8 @@ RecursiveQuery::sendQuery(const Question& question,
     // we're only going to handle UDP.
     asio::io_service& io = dns_service_.get_io_service();
     // It will delete itself when it is done
-    (void) answer_message;
-    new RunningQuery(io, question, upstream_, buffer, server,
-         timeout_, retries_);
+    new RunningQuery(io, question, answer_message, upstream_, buffer,
+                     server, timeout_, retries_);
 }
 
 class IntervalTimerImpl {

+ 1 - 1
src/lib/asiolink/tcpdns.cc

@@ -145,7 +145,7 @@ TCPServer::operator()(error_code ec, size_t length) {
         // DNS lookup and the write call.
         respbuf_.reset(new OutputBuffer(0));
         message_.reset(new Message(Message::PARSE));
-        answer_message_.reset(new Message(Message::PARSE));
+        answer_message_.reset(new Message(Message::RENDER));
 
         // Schedule a DNS lookup, and yield.  When the lookup is
         // finished, the coroutine will resume immediately after

+ 1 - 1
src/lib/asiolink/udpdns.cc

@@ -133,7 +133,7 @@ UDPServer::operator()(error_code ec, size_t length) {
         // asynchronous DNS lookup and/or by the send call.
         respbuf_.reset(new OutputBuffer(0));
         message_.reset(new Message(Message::PARSE));
-        answer_message_.reset(new Message(Message::PARSE));
+        answer_message_.reset(new Message(Message::RENDER));
 
         // Schedule a DNS lookup, and yield.  When the lookup is
         // finished, the coroutine will resume immediately after