Merge branch 'trac497'

src/bin/resolver/Makefile.am

@@ -37,7 +37,6 @@ spec_config.h: spec_config.h.pre
 BUILT_SOURCES = spec_config.h 
 pkglibexec_PROGRAMS = b10-resolver
 b10_resolver_SOURCES = resolver.cc resolver.h
-b10_resolver_SOURCES += response_classifier.cc response_classifier.h
 b10_resolver_SOURCES += response_scrubber.cc response_scrubber.h
 b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/change_user.h
 b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/common.h

src/bin/resolver/resolver.h

@@ -65,7 +65,10 @@ public:
     /// send the reply.
     ///
     /// \param io_message The raw message received
-    /// \param message Pointer to the \c Message object
+    /// \param query_message Pointer to the query Message object we
+    /// received from the client
+    /// \param answer_message Pointer to the anwer Message object we
+    /// shall return to the client
     /// \param buffer Pointer to an \c OutputBuffer for the resposne
     /// \param server Pointer to the \c DNSServer
     void processMessage(const asiolink::IOMessage& io_message,
@@ -146,7 +149,11 @@ public:
      * \short Set options related to timeouts.
      *
      * This sets the time of timeout and number of retries.
-     * \param timeout The time in milliseconds. The value -1 disables timeouts.
+     * \param query_timeout The timeout we use for queries we send
+     * \param client_timeout The timeout at which point we send back a
+     * SERVFAIL (while continuing to resolve the query)
+     * \param lookup_timeout The timeout at which point we give up and
+     * stop.
      * \param retries The number of retries (0 means try the first time only,
      *     do not retry).
      */

src/bin/resolver/tests/Makefile.am

@@ -19,11 +19,9 @@ 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 += ../resolver.h ../resolver.cc
-run_unittests_SOURCES += ../response_classifier.h ../response_classifier.cc
 run_unittests_SOURCES += ../response_scrubber.h ../response_scrubber.cc
 run_unittests_SOURCES += resolver_unittest.cc
 run_unittests_SOURCES += resolver_config_unittest.cc
-run_unittests_SOURCES += response_classifier_unittest.cc
 run_unittests_SOURCES += response_scrubber_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
@@ -31,8 +29,8 @@ 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/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

src/lib/asiolink/asiolink.cc

@@ -50,42 +50,6 @@ 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) {
-            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;
@@ -365,6 +329,12 @@ private:
     //shared_ptr<DNSServer> server_;
     isc::resolve::ResolverInterface::CallbackPtr resolvercallback_;
 
+    // To prevent both unreasonably long cname chains and cname loops,
+    // we simply keep a counter of the number of CNAMEs we have
+    // followed so far (and error if it exceeds RESOLVER_MAX_CNAME_CHAIN
+    // from lib/resolve/response_classifier.h)
+    unsigned cname_count_;
+
     /*
      * TODO Do something more clever with timeouts. In the long term, some
      *     computation of average RTT, increase with each retry, etc.
@@ -392,6 +362,11 @@ private:
     // If we timed out ourselves (lookup timeout), stop issuing queries
     bool done_;
 
+    // If we have a client timeout, we send back an answer, but don't
+    // stop. We use this variable to make sure we don't send another
+    // answer if we do find one later (or if we have a lookup_timeout)
+    bool answer_sent_;
+
     // (re)send the query to the server.
     void send() {
         const int uc = upstream_->size();
@@ -429,25 +404,61 @@ private:
     // Note that the footprint may change as this function may
     // need to append data to the answer we are building later.
     //
-    // returns true if we are done
+    // returns true if we are done (either we have an answer or an
+    //              error message)
     // returns false if we are not done
     bool handleRecursiveAnswer(const Message& incoming) {
-        if (incoming.getRRCount(Message::SECTION_ANSWER) > 0) {
-            dlog("Got final result, copying answer.");
-            copyAnswerMessage(incoming, answer_message_);
+        dlog("Handle response");
+        // In case we get a CNAME, we store the target
+        // here (classify() will set it when it walks through
+        // the cname chain to verify it).
+        Name cname_target(question_.getName());
+        
+        isc::resolve::ResponseClassifier::Category category =
+            isc::resolve::ResponseClassifier::classify(
+                question_, incoming, cname_target, cname_count_, true);
+
+        bool found_ns_address = false;
+
+        switch (category) {
+        case isc::resolve::ResponseClassifier::ANSWER:
+        case isc::resolve::ResponseClassifier::ANSWERCNAME:
+            // Done. copy and return.
+            isc::resolve::copyResponseMessage(incoming, answer_message_);
             return true;
-        } else {
-            dlog("Got delegation, continuing");
-            // ok we need to do some more processing.
-            // the ns list should contain all nameservers
-            // while the additional may contain addresses for
-            // them.
-            // this needs to tie into NSAS of course
-            // for this very first mockup, hope there is an
-            // address in additional and just use that
-
-            // send query to the addresses in the delegation
-            bool found_ns_address = false;
+            break;
+        case isc::resolve::ResponseClassifier::CNAME:
+            dlog("Response is CNAME!");
+            // (unfinished) CNAME. We set our question_ to the CNAME
+            // target, then start over at the beginning (for now, that
+            // is, we reset our 'current servers' to the root servers).
+            if (cname_count_ >= RESOLVER_MAX_CNAME_CHAIN) {
+                // just give up
+                dlog("CNAME chain too long");
+                isc::resolve::makeErrorMessage(answer_message_,
+                                               Rcode::SERVFAIL());
+                return true;
+            }
+
+            answer_message_->appendSection(Message::SECTION_ANSWER,
+                                           incoming);
+            setZoneServersToRoot();
+
+            question_ = Question(cname_target, question_.getClass(),
+                                 question_.getType());
+
+            dlog("Following CNAME chain to " + question_.toText());
+            send();
+            return false;
+            break;
+        case isc::resolve::ResponseClassifier::NXDOMAIN:
+            // NXDOMAIN, just copy and return.
+            isc::resolve::copyResponseMessage(incoming, answer_message_);
+            return true;
+            break;
+        case isc::resolve::ResponseClassifier::REFERRAL:
+            // Referral. For now we just take the first glue address
+            // we find and continue with that
             zone_servers_.clear();
 
             for (RRsetIterator rrsi = incoming.beginSection(Message::SECTION_ADDITIONAL);
@@ -466,7 +477,7 @@ private:
                         // to that address and yield, when it
                         // returns, loop again.
                         
-                        // should use NSAS
+                        // TODO should use NSAS
                         zone_servers_.push_back(addr_t(addr_str, 53));
                         found_ns_address = true;
                     }
@@ -478,14 +489,34 @@ private:
                 return false;
             } 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_);
+                // TODO this will result in answering with the delegation. oh well
+                isc::resolve::copyResponseMessage(incoming, answer_message_);
                 return true;
             }
+            break;
+        case isc::resolve::ResponseClassifier::EMPTY:
+        case isc::resolve::ResponseClassifier::EXTRADATA:
+        case isc::resolve::ResponseClassifier::INVNAMCLASS:
+        case isc::resolve::ResponseClassifier::INVTYPE:
+        case isc::resolve::ResponseClassifier::MISMATQUEST:
+        case isc::resolve::ResponseClassifier::MULTICLASS:
+        case isc::resolve::ResponseClassifier::NOTONEQUEST:
+        case isc::resolve::ResponseClassifier::NOTRESPONSE:
+        case isc::resolve::ResponseClassifier::NOTSINGLE:
+        case isc::resolve::ResponseClassifier::OPCODE:
+        case isc::resolve::ResponseClassifier::RCODE:
+        case isc::resolve::ResponseClassifier::TRUNCATED:
+            // Should we try a different server rather than SERVFAIL?
+            isc::resolve::makeErrorMessage(answer_message_,
+                                           Rcode::SERVFAIL());
+            return true;
+            break;
         }
+        // should not be reached. assert here?
+        dlog("[FATAL] unreachable code");
+        return true;
     }
     
-
 public:
     RunningQuery(asio::io_service& io, const Question &question,
         MessagePtr answer_message, shared_ptr<AddressVector> upstream,
@@ -501,12 +532,14 @@ public:
         upstream_root_(upstream_root),
         buffer_(buffer),
         resolvercallback_(cb),
+        cname_count_(0),
         query_timeout_(query_timeout),
         retries_(retries),
         client_timer(io),
         lookup_timer(io),
         queries_out_(0),
-        done_(false)
+        done_(false),
+        answer_sent_(false)
     {
         // Setup the timer to stop trying (lookup_timeout)
         if (lookup_timeout >= 0) {
@@ -525,31 +558,35 @@ public:
         // should use NSAS for root servers
         // Adding root servers if not a forwarder
         if (upstream_->empty()) {
-            if (upstream_root_->empty()) { //if no root ips given, use this
-                zone_servers_.push_back(addr_t("192.5.5.241", 53));
-            }
-            else
-            {
-              //copy the list
-              dlog("Size is " + 
-                    boost::lexical_cast<string>(upstream_root_->size()) + 
-                    "\n");
-              //Use BOOST_FOREACH here? Is it faster?
-              for(AddressVector::iterator it = upstream_root_->begin();
-                   it < upstream_root_->end(); it++) {
-                zone_servers_.push_back(addr_t(it->first,it->second));
-                dlog("Put " + zone_servers_.back().first + "into root list\n");
-              }
-            }
+            setZoneServersToRoot();
         }
 
         send();
     }
 
+    void setZoneServersToRoot() {
+        zone_servers_.clear();
+        if (upstream_root_->empty()) { //if no root ips given, use this
+            zone_servers_.push_back(addr_t("192.5.5.241", 53));
+        } else {
+            // copy the list
+            dlog("Size is " + 
+                boost::lexical_cast<string>(upstream_root_->size()) + 
+                "\n");
+            for(AddressVector::iterator it = upstream_root_->begin();
+                it < upstream_root_->end(); ++it) {
+            zone_servers_.push_back(addr_t(it->first,it->second));
+            dlog("Put " + zone_servers_.back().first + "into root list\n");
+            }
+        }
+    }
     virtual void clientTimeout() {
-        // right now, just stop (should make SERVFAIL and send that
-        // back, but not stop)
-        stop(false);
+        // Return a SERVFAIL, but do not stop until
+        // we have an answer or timeout ourselves
+        isc::resolve::makeErrorMessage(answer_message_,
+                                       Rcode::SERVFAIL());
+        resolvercallback_->success(answer_message_);
+        answer_sent_ = true;
     }
 
     virtual void stop(bool resume) {
@@ -561,7 +598,7 @@ public:
         // same goes if we have an outstanding query (can't delete
         // until that one comes back to us)
         done_ = true;
-        if (resume) {
+        if (resume && !answer_sent_) {
             resolvercallback_->success(answer_message_);
         } else {
             resolvercallback_->failure();
@@ -592,7 +629,7 @@ public:
                 incoming.getRcode() == Rcode::NOERROR()) {
                 done_ = handleRecursiveAnswer(incoming);
             } else {
-                copyAnswerMessage(incoming, answer_message_);
+                isc::resolve::copyResponseMessage(incoming, answer_message_);
                 done_ = true;
             }
             

src/lib/asiolink/asiolink.h

@@ -466,10 +466,12 @@ public:
     /// class.
     ///
     /// \param io_message The event message to handle
-    /// \param message The DNS MessagePtr that needs handling
-    /// \param buffer The result is put here
+    /// \param query_message The DNS MessagePtr of the original query
+    /// \param answer_message The DNS MessagePtr of the answer we are
+    /// building
+    /// \param buffer Intermediate data results are put here
     virtual void operator()(const IOMessage& io_message,
-                            isc::dns::MessagePtr message,
+                            isc::dns::MessagePtr query_message,
                             isc::dns::MessagePtr answer_message,
                             isc::dns::OutputBufferPtr buffer) const = 0;
 };
@@ -546,9 +548,10 @@ public:
     ///        to forward queries to.
     /// \param upstream_root Addresses and ports of the root servers
     ///        to use when resolving.
-    /// \param timeout How long to timeout the query, in ms
-    ///     -1 means never timeout (but do not use that).
-    ///     TODO: This should be computed somehow dynamically in future
+    /// \param query_timeout Timeout value for queries we sent, in ms
+    /// \param client_timeout Timeout value for when we send back an
+    ///        error, in ms
+    /// \param lookup_timeout Timeout value for when we give up, in ms
     /// \param retries how many times we try again (0 means just send and
     ///     and return if it returs).
     RecursiveQuery(DNSService& dns_service,

src/lib/asiolink/tests/asiolink_unittest.cc

@@ -520,6 +520,39 @@ protected:
             bool* done_;
     };
 
+    // This version of mock server just stops the io_service when it is resumed
+    // the second time. (Used in the clientTimeout test, where resume
+    // is called initially with the error answer, and later when the
+    // lookup times out, it is called without an answer to send back)
+    class MockServerStop2 : public MockServer {
+        public:
+            explicit MockServerStop2(IOService& io_service,
+                                     bool* done1, bool* done2) :
+                MockServer(io_service),
+                done1_(done1),
+                done2_(done2),
+                stopped_once_(false)
+            {}
+
+            void resume(const bool done) {
+                if (stopped_once_) {
+                    *done2_ = done;
+                    io_.stop();
+                } else {
+                    *done1_ = done;
+                    stopped_once_ = true;
+                }
+            }
+
+            DNSServer* clone() {
+                return (new MockServerStop2(*this));
+            }
+        private:
+            bool* done1_;
+            bool* done2_;
+            bool stopped_once_;
+    };
+
 private:
     class ASIOCallBack : public SimpleCallback {
     public:
@@ -809,8 +842,9 @@ TEST_F(ASIOLinkTest, forwardClientTimeout) {
     sock_ = createTestSocket();
 
     // Prepare the server
-    bool done(true);
-    MockServerStop server(*io_service_, &done);
+    bool done1(true);
+    bool done2(true);
+    MockServerStop2 server(*io_service_, &done1, &done2);
 
     MessagePtr answer(new Message(Message::RENDER));
 
@@ -818,11 +852,11 @@ TEST_F(ASIOLinkTest, forwardClientTimeout) {
     const uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
     // Set it up to retry twice before client timeout fires
     // Since the lookup timer has not fired, it should retry
-    // a third time
+    // four times
     RecursiveQuery query(*dns_service_,
                          singleAddress(TEST_IPV4_ADDR, port),
                          singleAddress(TEST_IPV4_ADDR, port),
-                         50, 120, 1000, 3);
+                         50, 120, 1000, 4);
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     query.resolve(question, answer, buffer, &server);
@@ -833,17 +867,15 @@ TEST_F(ASIOLinkTest, forwardClientTimeout) {
     // we know it'll fail, so make it a shorter timeout
     int recv_options = setSocketTimeout(sock_, 1, 0);
 
-    // Try to read 5 times, should stop after 3 reads
+    // Try to read 5 times
     int num = 0;
     bool read_success = tryRead(sock_, recv_options, 5, &num);
 
-    // The query should fail (for resolver it should send back servfail,
-    // but currently, and perhaps for forwarder in general, the effect
-    // will be the same as on a lookup timeout, i.e. no answer is sent
-    // back)
-    EXPECT_FALSE(done);
-    EXPECT_EQ(3, num);
-    EXPECT_FALSE(read_success);
+    // The query should fail, but we should have kept on trying
+    EXPECT_TRUE(done1);
+    EXPECT_FALSE(done2);
+    EXPECT_EQ(5, num);
+    EXPECT_TRUE(read_success);
 }
 
 // If we set lookup timeout to lower than querytimeout*retries, we should

src/lib/dns/message.cc

@@ -338,6 +338,14 @@ Message::removeRRset(const Section section, RRsetIterator& iterator) {
     return (removed);
 }
 
+void
+Message::clearSection(const Section section) {
+    if (section >= MessageImpl::NUM_SECTIONS) {
+        isc_throw(OutOfRange, "Invalid message section: " << section);
+    }
+    impl_->rrsets_[section].clear();
+    impl_->counts_[section] = 0;
+}
 
 void
 Message::addQuestion(const QuestionPtr question) {
@@ -768,6 +776,27 @@ Message::clear(Mode mode) {
     impl_->mode_ = mode;
 }
 
+void
+Message::appendSection(const Section section, const Message& source) {
+    if (section >= MessageImpl::NUM_SECTIONS) {
+        isc_throw(OutOfRange, "Invalid message section: " << section);
+    }
+
+    if (section == SECTION_QUESTION) {
+        for (QuestionIterator qi = source.beginQuestion();
+             qi != source.endQuestion();
+             ++qi) {
+            addQuestion(*qi);
+        }
+    } else {
+        for (RRsetIterator rrsi = source.beginSection(section);
+             rrsi != source.endSection(section);
+             ++rrsi) {
+            addRRset(section, *rrsi);
+        }
+    }
+}
+
 void
 Message::makeResponse() {
     if (impl_->mode_ != Message::PARSE) {

src/lib/dns/message.h

@@ -483,6 +483,11 @@ public:
     /// found in the specified section.
     bool removeRRset(const Section section, RRsetIterator& iterator);
 
+    /// \brief Remove all RRSets from the given Section
+    ///
+    /// \param section Section to remove all rrsets from
+    void clearSection(const Section section);
+
     // The following methods are not currently implemented.
     //void removeQuestion(QuestionPtr question);
     // notyet:
@@ -493,6 +498,13 @@ public:
     /// specified mode.
     void clear(Mode mode);
 
+    /// \brief Adds all rrsets from the source the given section in the
+    /// source message to the same section of this message
+    ///
+    /// \param section the section to append
+    /// \param target The source Message
+    void appendSection(const Section section, const Message& source);
+
     /// \brief Prepare for making a response from a request.
     ///
     /// This will clear the DNS header except those fields that should be kept

src/lib/dns/tests/message_unittest.cc

@@ -297,6 +297,75 @@ TEST_F(MessageTest, removeRRset) {
     EXPECT_EQ(2, message_render.getRRCount(Message::SECTION_ANSWER));
 }
 
+TEST_F(MessageTest, clearQuestionSection) {
+    QuestionPtr q(new Question(Name("www.example.com"), RRClass::IN(),
+                               RRType::A()));
+    message_render.addQuestion(q);
+    ASSERT_EQ(1, message_render.getRRCount(Message::SECTION_QUESTION));
+
+    message_render.clearSection(Message::SECTION_QUESTION);
+    EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_QUESTION));
+}
+
+
+TEST_F(MessageTest, clearAnswerSection) {
+    // Add two RRsets, check they are present, clear the section,
+    // check if they are gone.
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_a);
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::A()));
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_ANSWER));
+
+    message_render.clearSection(Message::SECTION_ANSWER);
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::A()));
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_ANSWER));
+}
+
+TEST_F(MessageTest, clearAuthoritySection) {
+    // Add two RRsets, check they are present, clear the section,
+    // check if they are gone.
+    message_render.addRRset(Message::SECTION_AUTHORITY, rrset_a);
+    message_render.addRRset(Message::SECTION_AUTHORITY, rrset_aaaa);
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
+        RRClass::IN(), RRType::A()));
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_AUTHORITY));
+
+    message_render.clearSection(Message::SECTION_AUTHORITY);
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
+        RRClass::IN(), RRType::A()));
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_AUTHORITY));
+}
+
+TEST_F(MessageTest, clearAdditionalSection) {
+    // Add two RRsets, check they are present, clear the section,
+    // check if they are gone.
+    message_render.addRRset(Message::SECTION_ADDITIONAL, rrset_a);
+    message_render.addRRset(Message::SECTION_ADDITIONAL, rrset_aaaa);
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
+        RRClass::IN(), RRType::A()));
+    ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_ADDITIONAL));
+
+    message_render.clearSection(Message::SECTION_ADDITIONAL);
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
+        RRClass::IN(), RRType::A()));
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_ADDITIONAL));
+}
+
+
 TEST_F(MessageTest, badBeginSection) {
     // valid cases are tested via other tests
     EXPECT_THROW(message_render.beginSection(Message::SECTION_QUESTION),
@@ -311,6 +380,63 @@ TEST_F(MessageTest, badEndSection) {
     EXPECT_THROW(message_render.endSection(bogus_section), OutOfRange);
 }
 
+TEST_F(MessageTest, appendSection) {
+    Message target(Message::RENDER);
+
+    // Section check
+    EXPECT_THROW(target.appendSection(bogus_section, message_render),
+                 OutOfRange);
+