BIND 10 trac1708, updated. e1d0d73d043c58305c1affbee3abe52cdee92289 [1708] DHCPv{4, 6} start-up fixes:
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jul 23 14:36:18 UTC 2012
The branch, trac1708 has been updated
via e1d0d73d043c58305c1affbee3abe52cdee92289 (commit)
from f973e64243cb9b616201247f86caffa8965a6928 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit e1d0d73d043c58305c1affbee3abe52cdee92289
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Mon Jul 23 16:35:54 2012 +0200
[1708] DHCPv{4,6} start-up fixes:
- better exception handling in dhcpv{4,6}Srv constructors
- port number is now parsed with boost::lexical_cast
- new tests for port numbers implemented
- ControlledDhcpv{4,6}Srv objects are now automatic
- runDhcpv4() method renamed to runCommand() to better prepare
for upcoming 4/6 test merge.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_srv.cc | 19 ++++++++++++-------
src/bin/dhcp4/main.cc | 20 ++++++++++++--------
src/bin/dhcp4/tests/dhcp4_test.py | 34 ++++++++++++++++++++++++++++------
src/bin/dhcp6/dhcp6_srv.cc | 27 ++++++++++++++-------------
src/bin/dhcp6/main.cc | 19 ++++++++++++-------
src/bin/dhcp6/tests/dhcp6_test.py | 26 +++++++++++++++++++++++---
6 files changed, 101 insertions(+), 44 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 4fb0a77..a86b010 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -37,16 +37,21 @@ const std::string HARDCODED_SERVER_ID = "192.0.2.1";
Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
cout << "Initialization: opening sockets on port " << port << endl;
- // first call to instance() will create IfaceMgr (it's a singleton)
- // it may throw something if things go wrong
- IfaceMgr::instance();
+ try {
+ // first call to instance() will create IfaceMgr (it's a singleton)
+ // it may throw something if things go wrong
+ IfaceMgr::instance();
- /// @todo: instantiate LeaseMgr here once it is imlpemented.
- IfaceMgr::instance().printIfaces();
+ /// @todo: instantiate LeaseMgr here once it is imlpemented.
- IfaceMgr::instance().openSockets4(port);
+ IfaceMgr::instance().openSockets4(port);
- setServerID();
+ setServerID();
+ } catch (const std::exception &e) {
+ cerr << "Error during DHCPv4 server startup: " << e.what() << endl;
+ shutdown_ = true;
+ return;
+ }
shutdown_ = false;
}
diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc
index 472f46f..1087cc7 100644
--- a/src/bin/dhcp4/main.cc
+++ b/src/bin/dhcp4/main.cc
@@ -17,6 +17,7 @@
#include <log/dummylog.h>
#include <log/logger_support.h>
#include <dhcp4/ctrl_dhcp4_srv.h>
+#include <boost/lexical_cast.hpp>
using namespace std;
using namespace isc::dhcp;
@@ -64,8 +65,14 @@ main(int argc, char* argv[]) {
stand_alone = true;
break;
case 'p':
- port_number = strtol(optarg, NULL, 10);
- if (port_number == 0) {
+ try {
+ port_number = boost::lexical_cast<int>(optarg);
+ } catch (const boost::bad_lexical_cast &) {
+ cerr << "Failed to parse port number: [" << optarg
+ << "], 1-65535 allowed." << endl;
+ usage();
+ }
+ if (port_number <= 0 || port_number > 65535) {
cerr << "Failed to parse port number: [" << optarg
<< "], 1-65535 allowed." << endl;
usage();
@@ -97,11 +104,11 @@ main(int argc, char* argv[]) {
cout << "[b10-dhcp4] Initiating DHCPv4 server operation." << endl;
/// @todo: pass verbose to the actul server once logging is implemented
- ControlledDhcpv4Srv* server = new ControlledDhcpv4Srv(port_number);
+ ControlledDhcpv4Srv server(port_number);
if (!stand_alone) {
try {
- server->establishSession();
+ server.establishSession();
} catch (const std::exception& ex) {
cerr << "Failed to establish BIND10 session. "
"Running in stand-alone mode:" << ex.what() << endl;
@@ -112,10 +119,7 @@ main(int argc, char* argv[]) {
cout << "Skipping connection to the BIND10 msgq." << endl;
}
- server->run();
- delete server;
- server = NULL;
-
+ server.run();
} catch (const std::exception& ex) {
cerr << "[b10-dhcp4] Server failed: " << ex.what() << endl;
ret = EXIT_FAILURE;
diff --git a/src/bin/dhcp4/tests/dhcp4_test.py b/src/bin/dhcp4/tests/dhcp4_test.py
index 065c80c..935bba6 100644
--- a/src/bin/dhcp4/tests/dhcp4_test.py
+++ b/src/bin/dhcp4/tests/dhcp4_test.py
@@ -34,7 +34,7 @@ class TestDhcpv4Daemon(unittest.TestCase):
def tearDown(self):
pass
- def runDhcp4(self, params, wait=1):
+ def runCommand(self, params, wait=1):
"""
This method runs dhcp4 and returns a touple: (returncode, stdout, stderr)
"""
@@ -127,14 +127,14 @@ class TestDhcpv4Daemon(unittest.TestCase):
print("Note: Purpose of some of the tests is to check if DHCPv4 server can be started,")
print(" not that is can bind sockets correctly. Please ignore binding errors.")
- (returncode, output, error) = self.runDhcp4(["../b10-dhcp4", "-v"])
+ (returncode, output, error) = self.runCommand(["../b10-dhcp4", "-v"])
self.assertEqual( str(output).count("[b10-dhcp4] Initiating DHCPv4 server operation."), 1)
def test_portnumber_0(self):
print("Check that specifying port number 0 is not allowed.")
- (returncode, output, error) = self.runDhcp4(['../b10-dhcp4', '-p', '0'])
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-p', '0'])
# When invalid port number is specified, return code must not be success
self.assertTrue(returncode != 0)
@@ -145,7 +145,7 @@ class TestDhcpv4Daemon(unittest.TestCase):
def test_portnumber_missing(self):
print("Check that -p option requires a parameter.")
- (returncode, output, error) = self.runDhcp4(['../b10-dhcp4', '-p'])
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-p'])
# When invalid port number is specified, return code must not be success
self.assertTrue(returncode != 0)
@@ -153,10 +153,32 @@ class TestDhcpv4Daemon(unittest.TestCase):
# Check that there is an error message about invalid port number printed on stderr
self.assertEqual( str(error).count("option requires an argument"), 1)
+ def test_portnumber_invalid1(self):
+ print("Check that -p option is check against bogus port number (999999).")
+
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-p','999999'])
+
+ # When invalid port number is specified, return code must not be success
+ self.assertTrue(returncode != 0)
+
+ # Check that there is an error message about invalid port number printed on stderr
+ self.assertEqual( str(error).count("Failed to parse port number"), 1)
+
+ def test_portnumber_invalid2(self):
+ print("Check that -p option is check against bogus port number (123garbage).")
+
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-p','123garbage'])
+
+ # When invalid port number is specified, return code must not be success
+ self.assertTrue(returncode != 0)
+
+ # Check that there is an error message about invalid port number printed on stderr
+ self.assertEqual( str(error).count("Failed to parse port number"), 1)
+
def test_portnumber_nonroot(self):
print("Check that specifying unprivileged port number will work.")
- (returncode, output, error) = self.runDhcp4(['../b10-dhcp4', '-p', '10057'])
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-s', '-p', '10057'])
# When invalid port number is specified, return code must not be success
# TODO: Temporarily commented out as socket binding on systems that do not have
@@ -169,7 +191,7 @@ class TestDhcpv4Daemon(unittest.TestCase):
def test_skip_msgq(self):
print("Check that connection to BIND10 msgq can be disabled.")
- (returncode, output, error) = self.runDhcp4(['../b10-dhcp4', '-s', '-p', '10057'])
+ (returncode, output, error) = self.runCommand(['../b10-dhcp4', '-s', '-p', '10057'])
# When invalid port number is specified, return code must not be success
# TODO: Temporarily commented out as socket binding on systems that do not have
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index d28d9b7..9b43f53 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -45,23 +45,24 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) {
// first call to instance() will create IfaceMgr (it's a singleton)
// it may throw something if things go wrong
try {
- IfaceMgr::instance();
- } catch (const std::exception &e) {
- cout << "Failed to instantiate InterfaceManager:" << e.what() << ". Aborting." << endl;
- shutdown_ = true;
- }
- if (IfaceMgr::instance().countIfaces() == 0) {
- cout << "Failed to detect any network interfaces. Aborting." << endl;
- shutdown_ = true;
- }
+ if (IfaceMgr::instance().countIfaces() == 0) {
+ cout << "Failed to detect any network interfaces. Aborting." << endl;
+ shutdown_ = true;
+ return;
+ }
- // Now try to open IPv6 sockets on detected interfaces.
- IfaceMgr::instance().openSockets6(port);
+ IfaceMgr::instance().openSockets6(port);
- /// @todo: instantiate LeaseMgr here once it is imlpemented.
+ setServerID();
- setServerID();
+ /// @todo: instantiate LeaseMgr here once it is imlpemented.
+
+ } catch (const std::exception &e) {
+ cerr << "Error during DHCPv4 server startup: " << e.what() << endl;
+ shutdown_ = true;
+ return;
+ }
shutdown_ = false;
}
diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc
index 96e2dd1..aebee90 100644
--- a/src/bin/dhcp6/main.cc
+++ b/src/bin/dhcp6/main.cc
@@ -17,6 +17,7 @@
#include <log/dummylog.h>
#include <log/logger_support.h>
#include <dhcp6/ctrl_dhcp6_srv.h>
+#include <boost/lexical_cast.hpp>
using namespace std;
using namespace isc::dhcp;
@@ -64,8 +65,14 @@ main(int argc, char* argv[]) {
stand_alone = true;
break;
case 'p':
- port_number = strtol(optarg, NULL, 10);
- if (port_number == 0) {
+ try {
+ port_number = boost::lexical_cast<int>(optarg);
+ } catch (const boost::bad_lexical_cast &) {
+ cerr << "Failed to parse port number: [" << optarg
+ << "], 1-65535 allowed." << endl;
+ usage();
+ }
+ if (port_number <= 0 || port_number > 65535) {
cerr << "Failed to parse port number: [" << optarg
<< "], 1-65535 allowed." << endl;
usage();
@@ -97,11 +104,11 @@ main(int argc, char* argv[]) {
cout << "b10-dhcp6: Initiating DHCPv6 server operation." << endl;
/// @todo: pass verbose to the actual server once logging is implemented
- ControlledDhcpv6Srv* server = new ControlledDhcpv6Srv(port_number);
+ ControlledDhcpv6Srv server(port_number);
if (!stand_alone) {
try {
- server->establishSession();
+ server.establishSession();
} catch (const std::exception& ex) {
cerr << "Failed to establish BIND10 session. "
"Running in stand-alone mode:" << ex.what() << endl;
@@ -112,9 +119,7 @@ main(int argc, char* argv[]) {
cout << "Skipping connection to the BIND10 msgq." << endl;
}
- server->run();
- delete server;
- server = NULL;
+ server.run();
} catch (const std::exception& ex) {
cerr << "[b10-dhcp6] Server failed: " << ex.what() << endl;
diff --git a/src/bin/dhcp6/tests/dhcp6_test.py b/src/bin/dhcp6/tests/dhcp6_test.py
index 123b105..399c370 100644
--- a/src/bin/dhcp6/tests/dhcp6_test.py
+++ b/src/bin/dhcp6/tests/dhcp6_test.py
@@ -155,17 +155,38 @@ class TestDhcpv6Daemon(unittest.TestCase):
# Check that there is an error message about invalid port number printed on stderr
self.assertEqual( str(error).count("option requires an argument"), 1)
+ def test_portnumber_invalid1(self):
+ print("Check that -p option is check against bogus port number (999999).")
+
+ (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-p','999999'])
+
+ # When invalid port number is specified, return code must not be success
+ self.assertTrue(returncode != 0)
+
+ # Check that there is an error message about invalid port number printed on stderr
+ self.assertEqual( str(error).count("Failed to parse port number"), 1)
+
+ def test_portnumber_invalid2(self):
+ print("Check that -p option is check against bogus port number (123garbage).")
+
+ (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-p','123garbage'])
+
+ # When invalid port number is specified, return code must not be success
+ self.assertTrue(returncode != 0)
+
+ # Check that there is an error message about invalid port number printed on stderr
+ self.assertEqual( str(error).count("Failed to parse port number"), 1)
+
def test_portnumber_nonroot(self):
print("Check that specifying unprivileged port number will work.")
- (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-p', '10547'])
+ (returncode, output, error) = self.runCommand(['../b10-dhcp6', '-s', '-p', '10547'])
# When invalid port number is specified, return code must not be success
# TODO: Temporarily commented out as socket binding on systems that do not have
# interface detection implemented currently fails.
# self.assertTrue(returncode == 0)
- # Check that there is a message on stdout about opening proper port
self.assertEqual( str(output).count("opening sockets on port 10547"), 1)
def test_skip_msgq(self):
@@ -178,7 +199,6 @@ class TestDhcpv6Daemon(unittest.TestCase):
# interface detection implemented currently fails.
# self.assertTrue(returncode == 0)
- # Check that there is an error message about invalid port number printed on stderr
self.assertEqual( str(output).count("Skipping connection to the BIND10 msgq."), 1)
More information about the bind10-changes
mailing list