Browse Source

[1596] Reject invalid inputs

Michal 'vorner' Vaner 13 years ago
parent
commit
809c35dc7d
3 changed files with 46 additions and 42 deletions
  1. 13 12
      src/bin/auth/command.cc
  2. 17 16
      src/bin/auth/tests/command_unittest.cc
  3. 16 14
      src/bin/resolver/main.cc

+ 13 - 12
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);

+ 17 - 16
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_);

+ 16 - 14
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