From: Michal 'vorner' Vaner Date: Mon, 13 Feb 2012 11:40:37 +0000 (+0100) Subject: [1596] Reject invalid inputs X-Git-Tag: trac2351_base~97^2~32^2~3^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=809c35dc7dfc5e9fd0a498928f6aaff89049757c;p=thirdparty%2Fkea.git [1596] Reject invalid inputs --- diff --git a/src/bin/auth/command.cc b/src/bin/auth/command.cc index a0362d2fbb..1b440db0f5 100644 --- a/src/bin/auth/command.cc +++ b/src/bin/auth/command.cc @@ -112,18 +112,19 @@ public: if (args && args->getType() == isc::data::Element::map && args->contains("pid")) { // If it is, we check it is the same as our PID - if (args->get("pid")->getType() == isc::data::Element::integer) { - const int pid(args->get("pid")->intValue()); - const pid_t my_pid(getpid()); - if (my_pid != pid) { - // It is not for us - // - // Note that this is completely expected situation, if - // there are multiple instances of the server running and - // another instance is being shut down, we get the message - // too, due to the multicast nature of our message bus. - return; - } + + // This might throw in case the type is not an int, but that's + // OK, as it'll get converted to an error on higher level. + const int pid(args->get("pid")->intValue()); + const pid_t my_pid(getpid()); + if (my_pid != pid) { + // It is not for us + // + // Note that this is completely expected situation, if + // there are multiple instances of the server running and + // another instance is being shut down, we get the message + // too, due to the multicast nature of our message bus. + return; } } LOG_DEBUG(auth_logger, DBG_AUTH_SHUT, AUTH_SHUTDOWN); diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc index ff8b22eca2..fe993278cc 100644 --- a/src/bin/auth/tests/command_unittest.cc +++ b/src/bin/auth/tests/command_unittest.cc @@ -70,7 +70,7 @@ protected: ConstElementPtr result_; // The shutdown command parameter ConstElementPtr param_; - int rcode_; + int rcode_, expect_rcode_; isc::asiolink::IntervalTimer itimer_; public: void stopServer(); // need to be public for boost::bind @@ -128,27 +128,13 @@ TEST_F(AuthCommandTest, shutdownCorrectPID) { EXPECT_EQ(0, rcode_); } -// If we provide something not an int, the PID is not really specified, so -// act as if nothing came. -TEST_F(AuthCommandTest, shutdownNotInt) { - // Put the pid parameter there - ElementPtr param(new isc::data::MapElement()); - param->set("pid", ConstElementPtr(new isc::data::StringElement("pid"))); - param_ = param; - // With the correct PID, it should act exactly the same as in case - // of no parameter - itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1); - server_.getIOService().run(); - EXPECT_EQ(0, rcode_); -} - // This is like stopServer, but the server should not stop after the // command, it should be running void AuthCommandTest::dontStopServer() { result_ = execAuthServerCommand(server_, "shutdown", param_); parseAnswer(rcode_, result_); - EXPECT_EQ(0, rcode_); + EXPECT_EQ(expect_rcode_, rcode_); rcode_ = -1; // We run the stopServer now, to really stop the server. // If it had stopped already, it won't be run and the rcode -1 will @@ -158,10 +144,25 @@ AuthCommandTest::dontStopServer() { itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1); } +// If we provide something not an int, the PID is not really specified, so +// act as if nothing came. +TEST_F(AuthCommandTest, shutdownNotInt) { + // Put the pid parameter there + ElementPtr param(new isc::data::MapElement()); + param->set("pid", ConstElementPtr(new isc::data::StringElement("pid"))); + param_ = param; + expect_rcode_ = 1; + // It should reject to stop if the PID is not an int. + itimer_.setup(boost::bind(&AuthCommandTest::dontStopServer, this), 1); + server_.getIOService().run(); + EXPECT_EQ(0, rcode_); +} + TEST_F(AuthCommandTest, shutdownIncorrectPID) { // The PID = 0 should be taken by init, so we are not init and the // PID should be different param_ = Element::fromJSON("{\"pid\": 0}"); + expect_rcode_ = 0; itimer_.setup(boost::bind(&AuthCommandTest::dontStopServer, this), 1); server_.getIOService().run(); EXPECT_EQ(0, rcode_); diff --git a/src/bin/resolver/main.cc b/src/bin/resolver/main.cc index 1be8b2c1f5..528b31bb45 100644 --- a/src/bin/resolver/main.cc +++ b/src/bin/resolver/main.cc @@ -81,16 +81,16 @@ ConstElementPtr my_command_handler(const string& command, ConstElementPtr args) { ConstElementPtr answer = createAnswer(); - if (command == "print_message") { - LOG_INFO(resolver_logger, RESOLVER_PRINT_COMMAND).arg(args); - /* let's add that message to our answer as well */ - answer = createAnswer(0, args); - } else if (command == "shutdown") { - // Is the pid argument provided? - if (args && args->getType() == isc::data::Element::map && - args->contains("pid")) { - // If it is, we check it is the same as our PID - if (args->get("pid")->getType() == isc::data::Element::integer) { + try { + if (command == "print_message") { + LOG_INFO(resolver_logger, RESOLVER_PRINT_COMMAND).arg(args); + /* let's add that message to our answer as well */ + answer = createAnswer(0, args); + } else if (command == "shutdown") { + // Is the pid argument provided? + if (args && args->getType() == isc::data::Element::map && + args->contains("pid")) { + // If it is, we check it is the same as our PID const int pid(args->get("pid")->intValue()); const pid_t my_pid(getpid()); if (my_pid != pid) { @@ -99,12 +99,14 @@ my_command_handler(const string& command, ConstElementPtr args) { return answer; } } + LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_SHUTDOWN); + io_service.stop(); } - LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_SHUTDOWN); - io_service.stop(); - } - return (answer); + return (answer); + } catch (const std::exception& e) { + return (createAnswer(1, e.what())); + } } void