[5097] Improved pd-pools parsing, tentative fix for #3956

src/bin/dhcp6/json_config_parser.cc

@@ -171,12 +171,9 @@ public:
     void parse(ConstElementPtr pd_pool_) {
         std::string addr_str;
         std::string excluded_prefix_str = "::";
-        int64_t prefix_len;
-        int64_t delegated_len;
-        int64_t excluded_prefix_len = 0;
-        bool got_prefix = false;
-        bool got_prefix_len = false;
-        bool got_delegated_len = false;
+        uint8_t prefix_len = 0;
+        uint8_t delegated_len = 0;
+        uint8_t excluded_prefix_len = 0;
 
         // Parse the elements that make up the option definition.
         BOOST_FOREACH(ConfigPair param, pd_pool_->mapValue()) {
@@ -185,24 +182,21 @@ public:
             try {
                 if (entry == "prefix") {
                     addr_str = value->stringValue();
-                    got_prefix = true;
                 } else if (entry == "excluded-prefix") {
                     excluded_prefix_str = value->stringValue();
                 } else if (entry == "prefix-len") {
-                    prefix_len = value->intValue();
-                    got_prefix_len = true;
+                    prefix_len = getUint8(entry, value);
                 } else if (entry == "delegated-len") {
-                    delegated_len = value->intValue();
-                    got_delegated_len = true;
+                    delegated_len = getUint8(entry, value);
                 } else if (entry == "excluded-prefix-len") {
-                    excluded_prefix_len = value->intValue();
+                    excluded_prefix_len = getUint8(entry, value);
                 } else if (entry == "option-data") {
                     OptionDataListParser opts_parser(AF_INET6);
                     opts_parser.parse(options_, value);
                 } else if (entry == "user-context") {
                     user_context_ = value;
                 } else {
-                    isc_throw(DhcpConfigError,
+                    isc_throw(isc::dhcp::DhcpConfigError,
                               "unsupported parameter: " << entry
                               << " (" << value->getPosition() << ")");
                 }
@@ -216,13 +210,9 @@ public:
 
         // Check the pool parameters. It will throw an exception if any
         // of the required parameters are not present or invalid.
-        if (!got_prefix || !got_prefix_len || !got_delegated_len) {
-            isc_throw(isc::dhcp::DhcpConfigError,
-                      "Missing parameter '"
-                      << (!got_prefix ? "prefix" :
-                          (!got_prefix_len ? "prefix-len" : "delegated-len"))
-                      << "' (" << pd_pool_->getPosition() << ")");
-        }
+        require_("prefix", pd_pool_);
+        require_("prefix-len", pd_pool_);
+        require_("delegated-len", pd_pool_);
         try {
             // Attempt to construct the local pool.
             pool_.reset(new Pool6(IOAddress(addr_str),
@@ -248,7 +238,39 @@ public:
         pools_->push_back(pool_);
     }
 
-protected:
+private:
+
+    /// @brief Require a mandatory element
+    ///
+    /// @param name Entry name
+    /// @param config Pools configuration
+    /// @throw isc::dhcp::DhcpConfigError if not present
+    void require_(const std::string& name, ConstElementPtr config) const {
+        if (!config->contains(name)) {
+            isc_throw(isc::dhcp::DhcpConfigError,
+                      "Missing parameter '" << name << "' ("
+                      << config->getPosition() << ")");
+        }
+    }
+
+    /// @brief Get an uint8_t value
+    ///
+    /// @param name Entry name
+    /// @param value Integer element value
+    /// @return uint8_t value
+    /// @throw isc::data::TypeError when it is not an integer
+    /// isc::dhcp::DhcpConfigError when it does not fit in an uint8_t
+    uint8_t getUint8(const std::string& name, ConstElementPtr value) const {
+        int64_t val_int = value->intValue();
+        if ((val_int < std::numeric_limits<uint8_t>::min()) ||
+            (val_int > std::numeric_limits<uint8_t>::max())) {
+            isc_throw(isc::dhcp::DhcpConfigError,
+                      "out of range value (" << val_int
+                      << ") specified for parameter '"
+                      << name << "' (" << value->getPosition() << ")");
+        }
+        return (static_cast<uint8_t>(val_int));
+    }
 
     /// Pointer to the created pool object.
     isc::dhcp::Pool6Ptr pool_;

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -903,34 +903,42 @@ PoolParser::parse(ConstElementPtr pool_structure,
             len = boost::lexical_cast<int>(prefix_len);
         } catch (...)  {
             isc_throw(DhcpConfigError, "Failed to parse pool "
-                      "definition: " << text_pool->stringValue()
-                      << " (" << text_pool->getPosition() << ")");
+                      "definition: " << txt << " ("
+                      << text_pool->getPosition() << ")");
         }
 
-        pool = poolMaker(addr, len);
-        pools_->push_back(pool);
-
-        // If there's user-context specified, store it.
-        ConstElementPtr user_context = pool_structure->get("user-context");
-        if (user_context) {
-            if (user_context->getType() != Element::map) {
-                isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map ("
-                          << user_context->getPosition() << ")");
-            }
-            pool->setUserContext(user_context);
+        try {
+            pool = poolMaker(addr, len);
+            pools_->push_back(pool);
+        } catch (const std::exception& ex) {
+            isc_throw(DhcpConfigError, "Failed to create pool defined by: "
+                      << txt << " (" << text_pool->getPosition() << ")");
         }
 
     } else {
+        isc::asiolink::IOAddress min("::");
+        isc::asiolink::IOAddress max("::");
 
         // Is this min-max notation?
         pos = txt.find("-");
         if (pos != string::npos) {
             // using min-max notation
-            isc::asiolink::IOAddress min(txt.substr(0,pos));
-            isc::asiolink::IOAddress max(txt.substr(pos + 1));
+            try {
+                min = isc::asiolink::IOAddress(txt.substr(0, pos));
+                max = isc::asiolink::IOAddress(txt.substr(pos + 1));
+            } catch (...)  {
+                isc_throw(DhcpConfigError, "Failed to parse pool "
+                          "definition: " << txt << " ("
+                          << text_pool->getPosition() << ")");
+            }
 
-            pool = poolMaker(min, max);
-            pools_->push_back(pool);
+            try {
+                pool = poolMaker(min, max);
+                pools_->push_back(pool);
+            } catch (const std::exception& ex) {
+                isc_throw(DhcpConfigError, "Failed to create pool defined by: "
+                          << txt << " (" << text_pool->getPosition() << ")");
+            }
         }
     }
 
@@ -942,6 +950,16 @@ PoolParser::parse(ConstElementPtr pool_structure,
                   << text_pool->getPosition() << ")");
     }
 
+    // If there's user-context specified, store it.
+    ConstElementPtr user_context = pool_structure->get("user-context");
+    if (user_context) {
+        if (user_context->getType() != Element::map) {
+            isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map ("
+                      << user_context->getPosition() << ")");
+        }
+        pool->setUserContext(user_context);
+    }
+
     // Parser pool specific options.
     ConstElementPtr option_data = pool_structure->get("option-data");
     if (option_data) {