BIND 10 trac2157_2, updated. dc6a6a9ef65b75bdb4cc79a67a83c04645657055 [2157] increment opcode counter if message header parse succeeded

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Jan 31 13:33:41 UTC 2013


The branch, trac2157_2 has been updated
       via  dc6a6a9ef65b75bdb4cc79a67a83c04645657055 (commit)
      from  7d28f0a9b596bd3a4580372c93408904d8f2d8a5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit dc6a6a9ef65b75bdb4cc79a67a83c04645657055
Author: Yoshitaka Aharen <aharen at jprs.co.jp>
Date:   Thu Jan 31 22:31:49 2013 +0900

    [2157] increment opcode counter if message header parse succeeded

-----------------------------------------------------------------------

Summary of changes:
 src/bin/auth/auth_srv.cc                |    6 +++---
 src/bin/auth/b10-auth.xml.pre           |   15 ---------------
 src/bin/auth/statistics.cc.pre          |   11 +++++++++--
 src/bin/auth/statistics.h               |    2 +-
 src/bin/auth/tests/auth_srv_unittest.cc |   24 ++++++++++++++++++++++--
 5 files changed, 35 insertions(+), 23 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 6a58cae..81ae05d 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -525,6 +525,9 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         return;
     }
 
+    const Opcode opcode = message.getOpcode();
+    stats_attrs.setRequestOpCode(opcode);
+
     try {
         // Parse the message.
         message.fromWire(request_buffer);
@@ -573,7 +576,6 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         return;
     }
 
-    const Opcode opcode = message.getOpcode();
     bool send_answer = true;
     try {
         // note: This can only be reliable after TSIG check succeeds.
@@ -584,8 +586,6 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         }
 
         // note: This can only be reliable after TSIG check succeeds.
-        stats_attrs.setRequestOpCode(opcode);
-
         if (opcode == Opcode::NOTIFY()) {
             send_answer = impl_->processNotify(io_message, message, buffer,
                                                tsig_context, stats_attrs);
diff --git a/src/bin/auth/b10-auth.xml.pre b/src/bin/auth/b10-auth.xml.pre
index 0ecc49c..45fb684 100644
--- a/src/bin/auth/b10-auth.xml.pre
+++ b/src/bin/auth/b10-auth.xml.pre
@@ -196,21 +196,6 @@
 
 <!-- ### STATISTICS DATA PLACEHOLDER ### -->
 
-    <note><para>
-      DNS parameters (e.g. opcode) in the request message will not be
-      considered if <command>b10-auth</command> failed to parse message,
-      including but not limited to the following circumstance:
-      <itemizedlist>
-        <listitem><para>
-          EDNS version of a message is unknown to
-          <command>b10-auth</command>.
-        </para></listitem>
-        <listitem><para>
-          TSIG signature verification fails.
-        </para></listitem>
-      </itemizedlist>
-    </para></note>
-
   </refsect1>
 
   <refsect1>
diff --git a/src/bin/auth/statistics.cc.pre b/src/bin/auth/statistics.cc.pre
index 199068e..7df39de 100644
--- a/src/bin/auth/statistics.cc.pre
+++ b/src/bin/auth/statistics.cc.pre
@@ -155,7 +155,8 @@ Counters::incRequest(const MessageAttributes& msgattrs) {
     // OPCODE
     const boost::optional<isc::dns::Opcode>& opcode =
         msgattrs.getRequestOpCode();
-    // Increment opcode counter only if the opcode exists.
+    // Increment opcode counter only if the opcode exists; it can happen if
+    // short message which does not contain DNS header received.
     if (opcode) {
         server_msg_counter_.inc(opcode_to_msgcounter[opcode.get().getCode()]);
     }
@@ -199,7 +200,13 @@ Counters::incResponse(const MessageAttributes& msgattrs,
 
     const boost::optional<isc::dns::Opcode>& opcode =
         msgattrs.getRequestOpCode();
-    if (opcode && opcode.get() == Opcode::QUERY()) {
+    if (!opcode) {
+        isc_throw(isc::Unexpected, "Opcode of the request is empty while it is"
+                                   " responded");
+    }
+    if (!msgattrs.getRequestSigBadSig() &&
+               opcode.get() == Opcode::QUERY())
+    {
         // compound attributes
         const unsigned int answer_rrs =
             response.getRRCount(Message::SECTION_ANSWER);
diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h
index 18d2f83..c845a6e 100644
--- a/src/bin/auth/statistics.h
+++ b/src/bin/auth/statistics.h
@@ -286,7 +286,7 @@ public:
     /// \param msgattrs DNS message attributes.
     /// \param response DNS response message.
     /// \param done DNS response was sent to the client.
-    /// \throw None
+    /// \throw isc::Unexpected Internal condition check failed.
     void inc(const MessageAttributes& msgattrs,
              const isc::dns::Message& response, const bool done);
 
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 70f98fb..aede681 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -309,13 +309,31 @@ TEST_F(AuthSrvTest, response) {
 // Query with a broken question
 TEST_F(AuthSrvTest, shortQuestion) {
     shortQuestion();
-    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
+    ConstElementPtr stats_after = server.getStatistics()->get("zones")->
+        get("_SERVER_");
+    std::map<std::string, int> expect;
+    expect["request.v4"] = 1;
+    expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
+    expect["responses"] = 1;
+    expect["rcode.formerr"] = 1;
+    expect["qrynoauthans"] = 1;
+    checkStatisticsCounters(stats_after, expect);
 }
 
 // Query with a broken answer section
 TEST_F(AuthSrvTest, shortAnswer) {
     shortAnswer();
-    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
+    ConstElementPtr stats_after = server.getStatistics()->get("zones")->
+        get("_SERVER_");
+    std::map<std::string, int> expect;
+    expect["request.v4"] = 1;
+    expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
+    expect["responses"] = 1;
+    expect["rcode.formerr"] = 1;
+    expect["qrynoauthans"] = 1;
+    checkStatisticsCounters(stats_after, expect);
 }
 
 // Query with unsupported version of EDNS.
@@ -328,8 +346,10 @@ TEST_F(AuthSrvTest, ednsBadVers) {
     expect["request.v4"] = 1;
     expect["request.badednsver"] = 1;
     expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
     expect["responses"] = 1;
     expect["rcode.badvers"] = 1;
+    expect["qrynoauthans"] = 1;
     checkStatisticsCounters(stats_after, expect);
 }
 



More information about the bind10-changes mailing list