diff --git a/server/actions.py b/server/actions.py index 4d021a71..ddcc83d6 100644 --- a/server/actions.py +++ b/server/actions.py @@ -77,7 +77,8 @@ class Actions(JailThread): for action in self.__actions: if action.getName() == name: self.__actions.remove(action) - break + return + raise KeyError("Invalid Action name: %s" % name) ## # Returns an action. @@ -91,7 +92,7 @@ class Actions(JailThread): for action in self.__actions: if action.getName() == name: return action - raise KeyError + raise KeyError("Invalid Action name") ## # Returns the last defined action. @@ -131,7 +132,7 @@ class Actions(JailThread): # Unban the IP. self.__unBan(ticket) return ip - return 'None' + raise ValueError("IP %s is not banned" % ip) ## # Main loop. diff --git a/server/filter.py b/server/filter.py index ebde3199..f842c269 100644 --- a/server/filter.py +++ b/server/filter.py @@ -93,6 +93,7 @@ class Filter(JailThread): self.__failRegex.append(regex) except RegexException, e: logSys.error(e) + raise e def delFailRegex(self, index): @@ -126,6 +127,7 @@ class Filter(JailThread): self.__ignoreRegex.append(regex) except RegexException, e: logSys.error(e) + raise e def delIgnoreRegex(self, index): try: diff --git a/server/transmitter.py b/server/transmitter.py index 3f880c07..dc121f93 100644 --- a/server/transmitter.py +++ b/server/transmitter.py @@ -112,14 +112,18 @@ class Transmitter: return self.__server.getLogLevel() elif name == "logtarget": value = command[1] - self.__server.setLogTarget(value) - return self.__server.getLogTarget() + if self.__server.setLogTarget(value): + return self.__server.getLogTarget() + else: + raise Exception("Failed to change log target") # Jail elif command[1] == "idle": if command[2] == "on": self.__server.setIdleJail(name, True) elif command[2] == "off": self.__server.setIdleJail(name, False) + else: + raise Exception("Invalid idle option, must be 'yes' or 'no'") return self.__server.getIdleJail(name) # Filter elif command[1] == "addignoreip": @@ -275,7 +279,7 @@ class Transmitter: def status(self, command): if len(command) == 0: return self.__server.status() - else: + elif len(command) == 1: name = command[0] return self.__server.statusJail(name) raise Exception("Invalid command (no status)") diff --git a/testcases/servertestcase.py b/testcases/servertestcase.py index 00f56b81..4268df38 100644 --- a/testcases/servertestcase.py +++ b/testcases/servertestcase.py @@ -29,6 +29,7 @@ __license__ = "GPL" import unittest, socket, time, tempfile, os from server.server import Server +from common.exceptions import UnknownJailException class StartStop(unittest.TestCase): @@ -141,6 +142,9 @@ class Transmitter(unittest.TestCase): self.__transm.proceed(["get", jail, cmd]), (0, outValues[n+1:])) + def testStopServer(self): + self.assertEqual(self.__transm.proceed(["stop"]), (0, None)) + def testPing(self): self.assertEqual(self.__transm.proceed(["ping"]), (0, "pong")) @@ -162,11 +166,7 @@ class Transmitter(unittest.TestCase): # If path is invalid, do not change logtarget value = "/this/path/should/not/exist" - self.assertEqual( - self.__transm.proceed(["set", "logtarget", value]), - (0, logTarget)) #NOTE: Shouldn't this return 1 - self.assertEqual( - self.__transm.proceed(["get", "logtarget"]), (0, logTargets[-1])) + self.setGetTestNOK("logtarget", value) self.__transm.proceed(["set", "/dev/null"]) for logTarget in logTargets: @@ -196,6 +196,28 @@ class Transmitter(unittest.TestCase): self.assertEqual( self.__transm.proceed(["add", "all", "polling"])[0], 1) + def testStartStopJail(self): + self.assertEqual( + self.__transm.proceed(["start", self.jailName]), (0, None)) + time.sleep(1) + self.assertEqual( + self.__transm.proceed(["stop", self.jailName]), (0, None)) + self.assertRaises( + UnknownJailException, self.__server.isAlive, self.jailName) + + def testStartStopAllJail(self): + self.__server.addJail("TestJail2", "auto") + self.assertEqual( + self.__transm.proceed(["start", self.jailName]), (0, None)) + self.assertEqual( + self.__transm.proceed(["start", "TestJail2"]), (0, None)) + self.assertEqual(self.__transm.proceed(["stop", "all"]), (0, None)) + time.sleep(1) + self.assertRaises( + UnknownJailException, self.__server.isAlive, self.jailName) + self.assertRaises( + UnknownJailException, self.__server.isAlive, "TestJail2") + def testJailIdle(self): self.assertEqual( self.__transm.proceed(["set", self.jailName, "idle", "on"]), @@ -205,7 +227,7 @@ class Transmitter(unittest.TestCase): (0, False)) self.assertEqual( self.__transm.proceed(["set", self.jailName, "idle", "CAT"])[0], - 0) #NOTE: Should this return 1 + 1) def testJailFindTime(self): self.setGetTest("findtime", "120", 120, jail=self.jailName) @@ -249,8 +271,7 @@ class Transmitter(unittest.TestCase): # Unban IP which isn't banned self.assertEqual( self.__transm.proceed( - ["set", self.jailName, "unbanip", "192.168.1.1"]), - (0, "None")) #NOTE: Should this return 1? + ["set", self.jailName, "unbanip", "192.168.1.1"])[0],1) def testJailMaxRetry(self): self.setGetTest("maxretry", "5", 5, jail=self.jailName) @@ -335,8 +356,8 @@ class Transmitter(unittest.TestCase): self.assertEqual( self.__transm.proceed( - ["set", self.jailName, "addfailregex", "No host regex"]), - (0, [])) #NOTE: Shouldn't this return 1? + ["set", self.jailName, "addfailregex", "No host regex"])[0], + 1) self.assertEqual( self.__transm.proceed( ["set", self.jailName, "addfailregex", 654])[0], @@ -357,6 +378,10 @@ class Transmitter(unittest.TestCase): self.jailName ) + self.assertEqual( + self.__transm.proceed( + ["set", self.jailName, "addignoreregex", "Invalid [regex"])[0], + 1) self.assertEqual( self.__transm.proceed( ["set", self.jailName, "addignoreregex", 50])[0], @@ -409,6 +434,9 @@ class Transmitter(unittest.TestCase): self.assertEqual( self.__transm.proceed(["set", self.jailName, "addaction", action]), (0, action)) + self.assertEqual( + self.__transm.proceed(["get", self.jailName, "addaction", action]), + (0, action)) for cmd, value in zip(cmdList, cmdValueList): self.assertEqual( self.__transm.proceed( @@ -439,5 +467,19 @@ class Transmitter(unittest.TestCase): (0, None)) self.assertEqual( self.__transm.proceed( - ["set", self.jailName, "delaction", "Doesn't exist"]), - (0, None)) #NOTE: Should this return 1? + ["set", self.jailName, "delaction", "Doesn't exist"])[0],1) + + def testNOK(self): + self.assertEqual(self.__transm.proceed(["INVALID", "COMMAND"])[0],1) + + def testSetNOK(self): + self.assertEqual( + self.__transm.proceed(["set", "INVALID", "COMMAND"])[0],1) + + def testGetNOK(self): + self.assertEqual( + self.__transm.proceed(["get", "INVALID", "COMMAND"])[0],1) + + def testStatusNOK(self): + self.assertEqual( + self.__transm.proceed(["status", "INVALID", "COMMAND"])[0],1)