[2157] use boost::optional for Opcode in MessageAttributes

src/bin/auth/statistics.cc.pre

@@ -151,8 +151,15 @@ Counters::incRequest(const MessageAttributes& msgattrs) {
     }
 
     // OPCODE
-    server_msg_counter_.inc(
-        opcode_to_msgcounter[msgattrs.getRequestOpCode().getCode()]);
+    try {
+        server_msg_counter_.inc(
+            opcode_to_msgcounter[msgattrs.getRequestOpCode().getCode()]);
+    } catch (const isc::InvalidOperation& ex) {
+        // The exception will be thrown if OpCode is not set
+        //  (e.g. message is short).
+        // Increment MSG_OPCODE_OTHER.
+        server_msg_counter_.inc(MSG_OPCODE_OTHER);
+    }
 }
 
 void
@@ -191,39 +198,47 @@ Counters::incResponse(const MessageAttributes& msgattrs,
         server_msg_counter_.inc(MSG_REQUEST_BADEDNSVER);
     }
 
-    if (msgattrs.getRequestOpCode() == Opcode::QUERY()) {
-        // compound attributes
-        const unsigned int answer_rrs =
-            response.getRRCount(Message::SECTION_ANSWER);
-        const bool is_aa_set = response.getHeaderFlag(Message::HEADERFLAG_AA);
-
-        if (is_aa_set) {
-            // QryAuthAns
-            server_msg_counter_.inc(MSG_QRYAUTHANS);
-        } else {
-            // QryNoAuthAns
-            server_msg_counter_.inc(MSG_QRYNOAUTHANS);
-        }
-
-        if (rcode == Rcode::NOERROR_CODE) {
-            if (answer_rrs > 0) {
-                // QrySuccess
-                server_msg_counter_.inc(MSG_QRYSUCCESS);
+    try {
+        if (msgattrs.getRequestOpCode() == Opcode::QUERY()) {
+            // compound attributes
+            const unsigned int answer_rrs =
+                response.getRRCount(Message::SECTION_ANSWER);
+            const bool is_aa_set =
+                response.getHeaderFlag(Message::HEADERFLAG_AA);
+
+            if (is_aa_set) {
+                // QryAuthAns
+                server_msg_counter_.inc(MSG_QRYAUTHANS);
             } else {
-                if (is_aa_set) {
-                    // QryNxrrset
-                    server_msg_counter_.inc(MSG_QRYNXRRSET);
+                // QryNoAuthAns
+                server_msg_counter_.inc(MSG_QRYNOAUTHANS);
+            }
+
+            if (rcode == Rcode::NOERROR_CODE) {
+                if (answer_rrs > 0) {
+                    // QrySuccess
+                    server_msg_counter_.inc(MSG_QRYSUCCESS);
                 } else {
-                    // QryReferral
-                    server_msg_counter_.inc(MSG_QRYREFERRAL);
+                    if (is_aa_set) {
+                        // QryNxrrset
+                        server_msg_counter_.inc(MSG_QRYNXRRSET);
+                    } else {
+                        // QryReferral
+                        server_msg_counter_.inc(MSG_QRYREFERRAL);
+                    }
+                }
+            } else if (rcode == Rcode::REFUSED_CODE) {
+                if (!response.getHeaderFlag(Message::HEADERFLAG_RD)) {
+                    // AuthRej
+                    server_msg_counter_.inc(MSG_QRYREJECT);
                 }
-            }
-        } else if (rcode == Rcode::REFUSED_CODE) {
-            if (!response.getHeaderFlag(Message::HEADERFLAG_RD)) {
-                // AuthRej
-                server_msg_counter_.inc(MSG_QRYREJECT);
             }
         }
+    } catch (const isc::InvalidOperation& ex) {
+        // The exception will be thrown if OpCode is not set
+        //  (e.g. message is short).
+        // Do not increment the counters above since they are incremented
+        // only if Opcode is Query.
     }
 }
 

src/bin/auth/statistics.h

@@ -23,6 +23,7 @@
 #include <statistics/counter.h>
 
 #include <boost/noncopyable.hpp>
+#include <boost/optional.hpp>
 
 #include <bitset>
 
@@ -57,7 +58,7 @@ private:
     // request attributes
     IPVersion req_ip_version_;                 // IP version
     TransportProtocol req_transport_protocol_; // Transport layer protocol
-    Opcode req_opcode_;                        // OpCode
+    boost::optional<Opcode> req_opcode_;       // OpCode
     enum BitAttributes {
         REQ_IS_EDNS_0,              // request is EDNS ver.0
         REQ_IS_DNSSEC_OK,           // DNSSEC OK (DO) bit is set in request
@@ -74,15 +75,19 @@ public:
     /// \throw None
     MessageAttributes() :
         req_ip_version_(IP_VERSION_UNSPEC),
-        req_transport_protocol_(TRANSPORT_UNSPEC),
-        req_opcode_(Opcode::RESERVED15_CODE), bit_attributes_()
+        req_transport_protocol_(TRANSPORT_UNSPEC), req_opcode_(boost::none),
+        bit_attributes_()
     {}
 
     /// \brief Return request opcode.
     /// \return opcode of the request
-    /// \throw None
+    /// \throw isc::InvalidOperation Opcode is not set
     const Opcode& getRequestOpCode() const {
-        return (req_opcode_);
+        if (req_opcode_) {
+            return (req_opcode_.get());
+        } else {
+            isc_throw(isc::InvalidOperation, "Opcode is not set");
+        }
     }
 
     /// \brief Set request opcode.

src/bin/auth/tests/statistics_unittest.cc.pre

@@ -124,6 +124,18 @@ TEST_F(CountersTest, invalidParameter) {
         ".*");
 }
 
+TEST_F(CountersTest, invalidOperationForGetRequestOpCode) {
+    MessageAttributes msgattrs;
+
+    // getRequestOpCode() will throw isc::InvalidOperation when called before
+    // opcode is set with setRequestOpCode().
+    EXPECT_THROW(msgattrs.getRequestOpCode(), isc::InvalidOperation);
+
+    msgattrs.setRequestOpCode(Opcode::QUERY());
+    // getRequestOpCode() will not throw at this point.
+    EXPECT_NO_THROW(msgattrs.getRequestOpCode());
+}
+
 TEST_F(CountersTest, incrementResponse) {
     Message response(Message::RENDER);
     MessageAttributes msgattrs;