code review: parse options properly (order independently), same logic for `unban` command, etc

fail2ban/protocol.py

@@ -54,7 +54,7 @@ protocol = [
 ["reload [--restart] [--unban] [--if-exists] <JAIL>", "reloads the jail <JAIL>, or restarts it (if option '--restart' specified)"],
 ["stop", "stops all jails and terminate the server"], 
 ["unban --all", "unbans all IP addresses (in all jails and database)"],
-["unban <IP> ... <IP>", "unbans <IP> (in all jails and database)"],
+["unban [--expr] [--] <IP> ... <IP>", "unbans <IP> (in all jails and database)"],
 ["banned", "return jails with banned IPs as dictionary"],
 ["banned <IP> ... <IP>]", "return list(s) of jails where given IP(s) are banned"],
 ["status", "gets the current status of the server"], 
@@ -105,7 +105,7 @@ protocol = [
 ["set <JAIL> usedns <VALUE>", "sets the usedns mode for <JAIL>"],
 ["set <JAIL> attempt <IP> [<failure1> ... <failureN>]", "manually notify about <IP> failure"], 
 ["set <JAIL> banip <IP> ... <IP>", "manually Ban <IP> for <JAIL>"], 
-["set <JAIL> unbanip [--report-absent] [--expr] <IP> ... <IP>", "manually Unban <IP> in <JAIL>"], 
+["set <JAIL> unbanip [--report-absent] [--expr] [--] <IP> ... <IP>", "manually Unban <IP> in <JAIL>"], 
 ["set <JAIL> maxretry <RETRY>", "sets the number of failures <RETRY> before banning the host for <JAIL>"], 
 ["set <JAIL> maxmatches <INT>", "sets the max number of matches stored in memory per ticket in <JAIL>"], 
 ["set <JAIL> maxlines <LINES>", "sets the number of <LINES> to buffer for regex search for <JAIL>"], 

fail2ban/server/server.py

@@ -24,14 +24,14 @@ __author__ = "Cyril Jaquier"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier"
 __license__ = "GPL"
 
-import threading
+from ast import literal_eval
-from threading import Lock, RLock
 import logging
 import os
 import signal
 import stat
 import sys
-from ast import literal_eval
+import threading
+from threading import Lock, RLock
 
 from .observer import Observers, ObserverThread
 from .jails import Jails
@@ -529,7 +529,7 @@ class Server:
 	def setBanIP(self, name, value):
 		return self.__jails[name].actions.addBannedIP(value)
 
-	def setUnbanIP(self, name=None, values=None, ifexists=True, ifexpr=False):
+	def setUnbanIP(self, name=None, values=None, ifexists=True, isexpr=False):
 		def parseExpr(v):
 			try:
 				return literal_eval(v)
@@ -542,7 +542,7 @@ class Server:
 			# in all jails:
 			jails = list(self.__jails.values())
 		# parse values if it contains an expression
-		if values and ifexpr:
+		if values and isexpr:
 			values = map(parseExpr, values)
 		# unban given or all (if values is None):
 		cnt = 0

fail2ban/server/transmitter.py

@@ -24,6 +24,7 @@ __author__ = "Cyril Jaquier"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier"
 __license__ = "GPL"
 
+import getopt
 import time
 import json
 
@@ -113,11 +114,12 @@ class Transmitter:
 			return 'OK'
 		elif name == "unban" and len(command) >= 2:
 			# unban in all jails:
-			value = command[1:]
+			opts, value = getopt.getopt(command[1:], "", ["expr", "all"])
+			opts = dict(opts)
 			# if all ips:
-			if len(value) == 1 and value[0] == "--all":
+			if "--all" in opts:
 				return self.__server.setUnbanIP()
-			return self.__server.setUnbanIP(None, value)
+			return self.__server.setUnbanIP(None, value, isexpr=("--expr" in opts))
 		elif name == "banned":
 			# check IP is banned in all jails:
 			return self.__server.banned(None, command[1:])
@@ -363,17 +365,10 @@ class Transmitter:
 			value = command[2:]
 			return self.__server.setBanIP(name,value)
 		elif command[1] == "unbanip":
-			ifexpr = False
+			opts, value = getopt.getopt(command[2:], "", ["expr", "report-absent"])
-			ifexists = True
+			opts = dict(opts)
-			offset = 2
+			return self.__server.setUnbanIP(name, value,
-			if "--report-absent" in command:
+				ifexists=("--report-absent" not in opts), isexpr=("--expr" in opts))
-				ifexists = False
-				offset += 1
-			if "--expr" in command:
-				ifexpr = True
-				offset += 1
-			value = command[offset:]
-			return self.__server.setUnbanIP(name, value, ifexists=ifexists, ifexpr=ifexpr)
 		elif command[1] == "addaction":
 			args = [command[2]]
 			if len(command) > 3: