From: Tomek Mrugalski <tomasz@isc.org>
Date: Fri, 5 Feb 2016 14:32:54 +0000 (+0100)
Subject: [4226] getType() and getName() in Pkt4 are now exception safe.
X-Git-Tag: trac4248_base~15^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=669bf23e9b4bad4157c8ab3bbcbaac5061591f8a;p=thirdparty%2Fkea.git

[4226] getType() and getName() in Pkt4 are now exception safe.
---

diff --git a/src/lib/dhcp/dhcp4.h b/src/lib/dhcp/dhcp4.h
index e987a80d4f..73118aa862 100644
--- a/src/lib/dhcp/dhcp4.h
+++ b/src/lib/dhcp/dhcp4.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2015 Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (C) 2004-2016 Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1995-2003 by Internet Software Consortium
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
@@ -219,6 +219,7 @@ enum DHCPOptionType {
 
 /* DHCP message types. */
 enum DHCPMessageType {
+    DHCP_NOTYPE         =  0, ///< Message Type option missing
     DHCPDISCOVER        =  1,
     DHCPOFFER           =  2,
     DHCPREQUEST         =  3,
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 623550c9d4..9b5719fa0e 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -238,7 +238,7 @@ Pkt4::unpack() {
 uint8_t Pkt4::getType() const {
     OptionPtr generic = getOption(DHO_DHCP_MESSAGE_TYPE);
     if (!generic) {
-        isc_throw(Unexpected, "Missing DHCP Message Type option");
+        return (DHCP_NOTYPE);
     }
 
     // Check if Message Type is specified as OptionInt<uint8_t>
@@ -328,7 +328,15 @@ Pkt4::getName(const uint8_t type) {
 
 const char*
 Pkt4::getName() const {
-    return (Pkt4::getName(getType()));
+
+    uint8_t msg_type = 0;
+    try {
+        msg_type = getType();
+    } catch (...) {
+        // Message Type option is missing.
+    }
+
+    return (Pkt4::getName(msg_type));
 }
 
 std::string
@@ -392,14 +400,11 @@ Pkt4::toText() const {
         << ", remote_adress=" << remote_addr_
         << ":" << remote_port_ << ", msg_type=";
 
-    // Try to obtain message type. This may throw if the Message Type option is
-    // not present. Therefore we guard it with try-catch, because we don't want
-    // toText method to throw.
-    try {
-        uint8_t msg_type = getType();
+    // Try to obtain message type.
+    uint8_t msg_type = getType();
+    if (msg_type != DHCP_NOTYPE) {
         output << getName(msg_type) << " (" << static_cast<int>(msg_type) << ")";
-
-    } catch (...) {
+    } else {
         // Message Type option is missing.
         output << "(missing)";
     }
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index be36a28a53..f5903a6293 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -226,6 +226,9 @@ public:
 
     /// @brief Returns DHCP message type (e.g. 1 = DHCPDISCOVER).
     ///
+    /// This method is exception safe. For packets without DHCP Message Type
+    /// option, it returns DHCP_NOTYPE (0).
+    ///
     /// @return message type
     uint8_t getType() const;
 
@@ -236,6 +239,9 @@ public:
 
     /// @brief Returns name of the DHCP message for a given type number.
     ///
+    /// This method is exception safe. For messages without DHCP Message Type
+    /// options, it returns UNKNOWN.
+    ///
     /// @param type DHCPv4 message type which name should be returned.
     ///
     /// @return Pointer to the "const" string containing DHCP message name.
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 183581a643..ac3f1b66ef 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -1130,4 +1130,19 @@ TEST_F(Pkt4Test, toText) {
 
 }
 
+// Sanity check. Verifies that the getName() and getType()
+// don't throw.
+TEST_F(Pkt4Test, getType) {
+
+    Pkt4 pkt(DHCPDISCOVER, 2543);
+    pkt.delOption(DHO_DHCP_MESSAGE_TYPE);
+
+    ASSERT_NO_THROW(pkt.getType());
+    ASSERT_NO_THROW(pkt.getName());
+
+    // The method has to return something that is not NULL,
+    // even if the packet doesn't have Message Type option.
+    EXPECT_TRUE(pkt.getName());
+}
+
 } // end of anonymous namespace