Browse Source

[5122] Duplicate position printed in exceptions fixed

Tomek Mrugalski 8 years ago
parent
commit
44e812a308

+ 7 - 4
src/bin/dhcp4/json_config_parser.cc

@@ -213,9 +213,11 @@ protected:
             }
         } catch (...) {
             ConstElementPtr next = params->get("next-server");
-            string pos("(missing)");
+            string pos;
             if (next)
                 pos = next->getPosition().str();
+            else
+                pos = params->getPosition().str();
             isc_throw(DhcpConfigError, "invalid parameter next-server : "
                       << next_server << "(" << pos << ")");
         }
@@ -235,7 +237,7 @@ protected:
             size_t slash = subnet4o6.find("/");
             if (slash == std::string::npos) {
                 isc_throw(DhcpConfigError, "Missing / in the 4o6-subnet parameter:"
-                          + subnet4o6 +", expected format: prefix6/length");
+                          << subnet4o6 << ", expected format: prefix6/length");
             }
             string prefix = subnet4o6.substr(0, slash);
             string lenstr = subnet4o6.substr(slash + 1);
@@ -245,7 +247,7 @@ protected:
                 len = boost::lexical_cast<unsigned int>(lenstr.c_str());
             } catch (const boost::bad_lexical_cast &) {
                 isc_throw(DhcpConfigError, "Invalid prefix length specified in "
-                          "4o6-subnet parameter: " + subnet4o6 + ", expected 0..128 value");
+                          "4o6-subnet parameter: " << subnet4o6 << ", expected 0..128 value");
             }
             subnet4->get4o6().setSubnet4o6(IOAddress(prefix), len);
             subnet4->get4o6().enabled(true);
@@ -260,7 +262,8 @@ protected:
             subnet4->get4o6().enabled(true);
         }
 
-        // client-class processing is now generic so in DhcpConfigParser
+        /// client-class processing is now generic and handled in the common
+        /// code (see @ref isc::data::SubnetConfigParser::createSubnet)
     }
 };
 

+ 1 - 1
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -583,7 +583,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed (<string>:20:17): String parameter subnet not found(<string>:20:17)\" }",
+              "\"text\": \"subnet configuration failed: String parameter subnet not found(<string>:20:17)\" }",
               response);
 
     // Check that the config was not lost

+ 10 - 51
src/bin/dhcp6/json_config_parser.cc

@@ -390,6 +390,8 @@ protected:
         // SimpleParser6::setAllDefaults.
         SubnetID subnet_id = static_cast<SubnetID>(getInteger(params, "id"));
 
+        // We want to log whether rapid-commit is enabled, so we get this
+        // before the actual subnet creation.
         bool rapid_commit = getBoolean(params, "rapid-commit");
 
         std::ostringstream output;
@@ -405,6 +407,10 @@ protected:
         // Create a new subnet.
         Subnet6* subnet6 = new Subnet6(addr, len, t1, t2, pref, valid,
                                        subnet_id);
+        subnet_.reset(subnet6);
+
+        // Enable or disable Rapid Commit option support for the subnet.
+        subnet6->setRapidCommit(rapid_commit);
 
         // Get interface-id option content. For now we support string
         // representation only
@@ -419,7 +425,8 @@ protected:
                       "parser error: interface (defined for locally reachable "
                       "subnets) and interface-id (defined for subnets reachable"
                       " via relays) cannot be defined at the same time for "
-                      "subnet " << addr << "/" << (int)len);
+                      "subnet " << addr << "/" << (int)len << "("
+                      << params->getPosition() << ")");
         }
 
         // Configure interface-id for remote interfaces, if defined
@@ -429,12 +436,8 @@ protected:
             subnet6->setInterfaceId(opt);
         }
 
-        // Enable or disable Rapid Commit option support for the subnet.
-        subnet6->setRapidCommit(rapid_commit);
-
-        // client-class processing is now generic so in DhcpConfigParser
-
-        subnet_.reset(subnet6);
+        /// client-class processing is now generic and handled in the common
+        /// code (see @ref isc::data::SubnetConfigParser::createSubnet)
     }
 
 };
@@ -605,50 +608,6 @@ private:
 namespace isc {
 namespace dhcp {
 
-/// @brief creates global parsers
-///
-/// This method creates global parsers that parse global parameters, i.e.
-/// those that take format of Dhcp6/param1, Dhcp6/param2 and so forth.
-///
-/// @param config_id pointer to received global configuration entry
-/// @param element pointer to the element to be parsed
-/// @return parser for specified global DHCPv6 parameter
-/// @throw NotImplemented if trying to create a parser for unknown config
-/// element
-DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
-                                                ConstElementPtr element) {
-    DhcpConfigParser* parser = NULL;
-    // preferred-lifetime, valid-lifetime, renew-timer, rebind-timer,
-    // decline-probation-period and dhcp4o6-port are now migrated to
-    // SimpleParser.
-    // subnet6 has been converted to SimpleParser.
-    // option-data and option-def are no longer needed here. They're now
-    // converted to SimpleParser and are handled in configureDhcp6Server.
-    // interfaces-config has been converted to SimpleParser.
-    // version was removed - it was a leftover from bindctrl.
-
-    // lease-database migrated
-    // hosts-database migrated
-
-    // hooks-libraries is now converted to SimpleParser.
-    // lease-database and hosts-database have been converted to SimpleParser already.
-    // mac-source has been converted to SimpleParser.
-    // dhcp-ddns has been converted to SimpleParser
-    // rsoo has been converted to SimpleParser.
-    // control-socket has been converted to SimpleParser.
-    // expired-leases-processing has been converted to SimpleParser.
-    // client-classes has been converted to SimpleParser.
-    // host-reservation-identifiers have been converted to SimpleParser already.
-    // server-id has been migrated to SimpleParser
-    {
-        isc_throw(DhcpConfigError,
-                "unsupported global configuration parameter: "
-                  << config_id << " (" << element->getPosition() << ")");
-    }
-
-    return (parser);
-}
-
 /// @brief Initialize the command channel based on the staging configuration
 ///
 /// Only close the current channel, if the new channel configuration is

+ 1 - 1
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -446,7 +446,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed (<string>:21:17): String parameter subnet not found(<string>:21:17)\" }",
+              "\"text\": \"subnet configuration failed: String parameter subnet not found(<string>:21:17)\" }",
               response);
 
     // Check that the config was not lost

+ 9 - 3
src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -953,8 +953,7 @@ SubnetConfigParser::parse(ConstElementPtr subnet) {
         createSubnet(subnet);
     } catch (const std::exception& ex) {
         isc_throw(DhcpConfigError,
-                  "subnet configuration failed (" << subnet->getPosition()
-                  << "): " << ex.what());
+                  "subnet configuration failed: " << ex.what());
     }
 
     return (subnet_);
@@ -1015,7 +1014,14 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     // Add pools to it.
     for (PoolStorage::iterator it = pools_->begin(); it != pools_->end();
          ++it) {
-        subnet_->addPool(*it);
+        try {
+            subnet_->addPool(*it);
+        } catch (const BadValue& ex) {
+            // addPool() can throw BadValue if the pool is overlapping or
+            // is out of bounds for the subnet.
+            isc_throw(DhcpConfigError, ex.what() << "(" << params->getPosition()
+                      << ")");
+        }
     }
 
     // Now configure parameters that are common for v4 and v6: