[3194] Use OptionDefinition::optionFactory to parse V4 Vendor option.

src/bin/dhcp4/dhcp4_srv.cc

@@ -1287,25 +1287,6 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
                       << "-byte long buffer.");
         }
 
-        /// @todo: Not sure if this is needed. Perhaps it would be better to extend
-        /// DHO_VIVSO_SUBOPTIONS definitions in std_option_defs.h to cover
-        /// OptionVendor class?
-        if (opt_type == DHO_VIVSO_SUBOPTIONS) {
-            if (offset + 4 > buf.size()) {
-                // Truncated vendor-option. There is expected at least 4 bytes
-                // long enterprise-id field
-                return (offset);
-            }
-
-            // Parse this as vendor option
-            OptionPtr vendor_opt(new OptionVendor(Option::V4, buf.begin() + offset,
-                                                  buf.begin() + offset + opt_len));
-            options.insert(std::make_pair(opt_type, vendor_opt));
-
-            offset += opt_len;
-            continue;
-        }
-
         // Get all definitions with the particular option code. Note that option code
         // is non-unique within this container however at this point we expect
         // to get one option definition with the particular code. If more are

src/lib/dhcp/option_definition.cc

@@ -26,6 +26,7 @@
 #include <dhcp/option_int_array.h>
 #include <dhcp/option_space.h>
 #include <dhcp/option_string.h>
+#include <dhcp/option_vendor.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
 #include <boost/algorithm/string/classification.hpp>
@@ -115,7 +116,19 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
                                 OptionBufferConstIter begin,
                                 OptionBufferConstIter end,
                                 UnpackOptionsCallback callback) const {
+
     try {
+        // Some of the options are represented by the specialized classes derived
+        // from Option class (e.g. IA_NA, IAADDR). Although, they can be also
+        // represented by the generic classes, we want the object of the specialized
+        // type to be returned. Therefore, we first check that if we are dealing
+        // with such an option. If the instance is returned we just exit at this
+        // point. If not, we will search for a generic option type to return.
+        OptionPtr option = factorySpecialFormatOption(u, begin, end, callback);
+        if (option) {
+            return (option);
+        }
+
         switch(type_) {
         case OPT_EMPTY_TYPE:
             return (factoryEmpty(u, type));
@@ -180,37 +193,10 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
             return (OptionPtr(new OptionString(u, type, begin, end)));
 
         default:
-            if (u == Option::V6) {
-                if ((code_ == D6O_IA_NA || code_ == D6O_IA_PD) &&
-                    haveIA6Format()) {
-                    // Return Option6IA instance for IA_PD and IA_NA option
-                    // types only. We don't want to return Option6IA for other
-                    // options that comprise 3 UINT32 data fields because
-                    // Option6IA accessors' and modifiers' names are derived
-                    // from the IA_NA and IA_PD options' field names: IAID,
-                    // T1, T2. Using functions such as getIAID, getT1 etc. for
-                    // options other than IA_NA and IA_PD would be bad practice
-                    // and cause confusion.
-                    return (factoryIA6(type, begin, end));
-
-                } else if (code_ == D6O_IAADDR && haveIAAddr6Format()) {
-                    // Rerurn Option6IAAddr option instance for the IAADDR
-                    // option only for the same reasons as described in
-                    // for IA_NA and IA_PD above.
-                    return (factoryIAAddr6(type, begin, end));
-                } else if (code_ == D6O_CLIENT_FQDN && haveClientFqdnFormat()) {
-                    // FQDN option requires special processing. Thus, there is
-                    // a specialized class to handle it.
-                    return (OptionPtr(new Option6ClientFqdn(begin, end)));
-                }
-            } else {
-                if ((code_ == DHO_FQDN) && haveFqdn4Format()) {
-                    return (OptionPtr(new Option4ClientFqdn(begin, end)));
-                }
-            }
+            // Do nothing. We will return generic option a few lines down.
+            ;
         }
         return (OptionPtr(new OptionCustom(*this, u, begin, end)));
-
     } catch (const Exception& ex) {
         isc_throw(InvalidOptionValue, ex.what());
     }
@@ -568,6 +554,45 @@ OptionDefinition::factoryIAAddr6(uint16_t type,
     return (option);
 }
 
+OptionPtr
+OptionDefinition::factorySpecialFormatOption(Option::Universe u,
+                                             OptionBufferConstIter begin,
+                                             OptionBufferConstIter end,
+                                             UnpackOptionsCallback) const {
+    if (u == Option::V6) {
+        if ((getCode() == D6O_IA_NA || getCode() == D6O_IA_PD) &&
+            haveIA6Format()) {
+            // Return Option6IA instance for IA_PD and IA_NA option
+            // types only. We don't want to return Option6IA for other
+            // options that comprise 3 UINT32 data fields because
+            // Option6IA accessors' and modifiers' names are derived
+            // from the IA_NA and IA_PD options' field names: IAID,
+            // T1, T2. Using functions such as getIAID, getT1 etc. for
+            // options other than IA_NA and IA_PD would be bad practice
+            // and cause confusion.
+            return (factoryIA6(getCode(), begin, end));
+
+        } else if (getCode() == D6O_IAADDR && haveIAAddr6Format()) {
+            // Rerurn Option6IAAddr option instance for the IAADDR
+            // option only for the same reasons as described in
+            // for IA_NA and IA_PD above.
+            return (factoryIAAddr6(getCode(), begin, end));
+        } else if (getCode() == D6O_CLIENT_FQDN && haveClientFqdnFormat()) {
+            // FQDN option requires special processing. Thus, there is
+            // a specialized class to handle it.
+            return (OptionPtr(new Option6ClientFqdn(begin, end)));
+        }
+    } else {
+        if ((getCode() == DHO_FQDN) && haveFqdn4Format()) {
+            return (OptionPtr(new Option4ClientFqdn(begin, end)));
+
+        } else if (getCode() == DHO_VIVSO_SUBOPTIONS) {
+            return (OptionPtr(new OptionVendor(Option::V4, begin, end)));
+
+        }
+    }
+    return (OptionPtr());
+}
 
 } // end of isc::dhcp namespace
 } // end of isc namespace

src/lib/dhcp/option_definition.h

@@ -493,6 +493,31 @@ public:
 
 private:
 
+    /// @brief Creates an instance of an option having special format.
+    ///
+    /// The option with special formats are encapsulated by the dedicated
+    /// classes derived from @c Option class. In particular these are:
+    /// - IA_NA
+    /// - IAADDR
+    /// - FQDN
+    /// - VIVSO.
+    ///
+    /// @param u A universe (V4 or V6).
+    /// @param begin beginning of the option buffer.
+    /// @param end end of the option buffer.
+    /// @param callback An instance of the function which parses packet options.
+    /// If this is set to non NULL value this function will be used instead of
+    /// @c isc::dhcp::LibDHCP::unpackOptions6 and
+    /// isc::dhcp::LibDHCP::unpackOptions4.
+    ///
+    /// @return An instance of the option having special format or NULL if
+    /// such an option can't be created because an option with the given
+    /// option code hasn't got the special format.
+    OptionPtr factorySpecialFormatOption(Option::Universe u,
+                                         OptionBufferConstIter begin,
+                                         OptionBufferConstIter end,
+                                         UnpackOptionsCallback callback) const;
+
     /// @brief Check if specified option format is a record with 3 fields
     /// where first one is custom, and two others are uint32.
     ///

src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -27,6 +27,7 @@
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
 #include <dhcp/option_string.h>
+#include <dhcp/option_vendor.h>
 #include <util/buffer.h>
 
 #include <gtest/gtest.h>
@@ -767,7 +768,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(Option));
 
     LibDhcpTest::testStdOptionDefs4(DHO_VIVSO_SUBOPTIONS, begin, end,
-                                    typeid(Option));
+                                    typeid(OptionVendor));
 }
 
 // Test that definitions of standard options have been initialized