RF/ENH: 1st wave of IPAddr pythonization - properties, logical statements, etc

# Conflicts:
#	fail2ban/server/ipdns.py

5 files changed, 124 insertions, 166 deletions

diff --git a/MANIFEST b/MANIFEST
index 3a5b4477..11f635c3 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -201,6 +201,7 @@ fail2ban/tests/actionstestcase.py
 fail2ban/tests/actiontestcase.py
 fail2ban/tests/banmanagertestcase.py
 fail2ban/tests/clientreadertestcase.py
+fail2ban/tests/clientbeautifiertestcase.py
 fail2ban/tests/config/action.d/brokenaction.conf
 fail2ban/tests/config/fail2ban.conf
 fail2ban/tests/config/filter.d/simple.conf
diff --git a/fail2ban/server/banmanager.py b/fail2ban/server/banmanager.py
index 67ae1b71..afa70685 100644
--- a/fail2ban/server/banmanager.py
+++ b/fail2ban/server/banmanager.py
@@ -152,8 +152,9 @@ class BanManager:
 			for banData in self.__banList:
 				ip = banData.getIP()
 				# Reference: http://www.team-cymru.org/Services/ip-to-asn.html#dns
-				question = ip.getPTR("origin.asn.cymru.com" if ip.isIPv4()
-						else "origin6.asn.cymru.com"
+				question = ip.getPTR(
+					"origin.asn.cymru.com" if ip.isIPv4
+					else "origin6.asn.cymru.com"
 				)
 				try:
 					answers = dns.resolver.query(question, "TXT")
diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py
index 49aba907..4bbf7996 100644
--- a/fail2ban/server/filter.py
+++ b/fail2ban/server/filter.py
@@ -383,7 +383,7 @@ class Filter(JailThread):
 		for net in self.__ignoreIpList:
 			# check if the IP is covered by ignore IP
 			if ip.isInNet(net):
-				self.logIgnoreIp(ip, log_ignore, ignore_source=("ip" if net.isValidIP() else "dns"))
+				self.logIgnoreIp(ip, log_ignore, ignore_source=("ip" if net.isValid else "dns"))
 				return True
 
 		if self.__ignoreCommand:
diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py
index b36b5cb1..744a8a73 100644
--- a/fail2ban/server/ipdns.py
+++ b/fail2ban/server/ipdns.py
@@ -17,13 +17,13 @@
 # along with Fail2Ban; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
-__author__ = "Fail2Ban Developers, Alexander Koeppe, Serg G. Brester"
+__author__ = "Fail2Ban Developers, Alexander Koeppe, Serg G. Brester, Yaroslav Halchenko"
 __copyright__ = "Copyright (c) 2004-2016 Fail2ban Developers"
 __license__ = "GPL"
 
-import re
 import socket
 import struct
+import re
 
 from .utils import Utils
 from ..helpers import getLogger
@@ -38,9 +38,7 @@ logSys = getLogger(__name__)
 #
 def asip(ip):
 	"""A little helper to guarantee ip being an IPAddr instance"""
-	if isinstance(ip, IPAddr):
-		return ip
-	return IPAddr(ip)
+	return ip if isinstance(ip, IPAddr) or ip is None else IPAddr(ip)
 
 
 ##
@@ -68,7 +66,7 @@ class DNSUtils:
 			ips = list()
 			for result in socket.getaddrinfo(dns, None, 0, 0, socket.IPPROTO_TCP):
 				ip = IPAddr(result[4][0])
-				if ip.isValidIP():
+				if ip.isValid:
 					ips.append(ip)
 		except socket.error, e:
 			# todo: make configurable the expired time of cache entry:
@@ -88,7 +86,7 @@ class DNSUtils:
 			if not isinstance(ip, IPAddr):
 				v = socket.gethostbyaddr(ip)[0]
 			else:
-				v = socket.gethostbyaddr(ip.ntoa())[0]
+				v = socket.gethostbyaddr(ip.ntoa)[0]
 		except socket.error, e:
 			logSys.debug("Unable to find a name for the IP %s: %s", ip, e)
 			v = None
@@ -104,7 +102,7 @@ class DNSUtils:
 		plainIP = IPAddr.searchIP(text)
 		if plainIP is not None:
 			ip = IPAddr(plainIP.group(0))
-			if ip.isValidIP():
+			if ip.isValid:
 				ipList.append(ip)
 
 		# If we are allowed to resolve -- give it a try if nothing was found
@@ -123,134 +121,132 @@ class DNSUtils:
 # Class for IP address handling.
 #
 # This class contains methods for handling IPv4 and IPv6 addresses.
-class IPAddr(object):
-	""" provide functions to handle IPv4 and IPv6 addresses 
+#
+class IPAddr:
+	"""Encapsulate functionality for IPv4 and IPv6 addresses
 	"""
 
 	IP_CRE = re.compile("^(?:\d{1,3}\.){3}\d{1,3}$")
 	IP6_CRE = re.compile("^[0-9a-fA-F]{4}[0-9a-fA-F:]+:[0-9a-fA-F]{1,4}|::1$")
 
 	# object attributes
-	addr = 0
-	family = socket.AF_UNSPEC
-	plen = 0
-	valid = False
-	raw = ""
-
-	# todo: make configurable the expired time and max count of cache entries:
-	CACHE_OBJ = Utils.Cache(maxCount=1000, maxTime=5*60)
-
-	def __new__(cls, ipstring, cidr=-1):
-		# already correct IPAddr
-		args = (ipstring, cidr)
-		ip = IPAddr.CACHE_OBJ.get(args)
-		if ip is not None:
-			return ip
-		ip = super(IPAddr, cls).__new__(cls)
-		ip.__init(ipstring, cidr)
-		IPAddr.CACHE_OBJ.set(args, ip)
-		return ip
+	_addr = 0
+	_family = socket.AF_UNSPEC
+	_plen = 0
+	_isValid = False
+	_raw = ""
 
 	# object methods
-	def __init(self, ipstring, cidr=-1):
+	def __init__(self, ipstring, cidr=-1):
 		""" initialize IP object by converting IP address string
 			to binary to integer
 		"""
 		for family in [socket.AF_INET, socket.AF_INET6]:
 			try:
 				binary = socket.inet_pton(family, ipstring)
-				self.valid = True
-				break
 			except socket.error:
 				continue
+			else: 
+				self._isValid = True
+				break
 
-		if self.valid and family == socket.AF_INET:
+		if self.isValid and family == socket.AF_INET:
 			# convert host to network byte order
-			self.addr, = struct.unpack("!L", binary)
-			self.family = family
-			self.plen = 32
+			self._addr, = struct.unpack("!L", binary)
+			self._family = family
+			self._plen = 32
 
 			# mask out host portion if prefix length is supplied
-			if cidr != None and cidr >= 0:
+			if cidr is not None and cidr >= 0:
 				mask = ~(0xFFFFFFFFL >> cidr)
-				self.addr = self.addr & mask
-				self.plen = cidr
+				self._addr &= mask
+				self._plen = cidr
 
-		elif self.valid and family == socket.AF_INET6:
+		elif self.isValid and family == socket.AF_INET6:
 			# convert host to network byte order
 			hi, lo = struct.unpack("!QQ", binary)
-			self.addr = (hi << 64) | lo
-			self.family = family
-			self.plen = 128
+			self._addr = (hi << 64) | lo
+			self._family = family
+			self._plen = 128
 
 			# mask out host portion if prefix length is supplied
-			if cidr != None and cidr >= 0:
+			if cidr is not None and cidr >= 0:
 				mask = ~(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFL >> cidr)
-				self.addr = self.addr & mask
-				self.plen = cidr
+				self._addr &= mask
+				self._plen = cidr
 
 			# if IPv6 address is a IPv4-compatible, make instance a IPv4
-			elif self.isInNet(IPAddr("::ffff:0:0", 96)):
-				self.addr = lo & 0xFFFFFFFFL
-				self.family = socket.AF_INET
-				self.plen = 32
+			elif self.isInNet(_IPv6_v4COMPAT):
+				self._addr = lo & 0xFFFFFFFFL
+				self._family = socket.AF_INET
+				self._plen = 32
 		else:
 			# string couldn't be converted neither to a IPv4 nor
 			# to a IPv6 address - retain raw input for later use
 			# (e.g. DNS resolution)
-			self.raw = ipstring
+			self._raw = ipstring
 
 	def __repr__(self):
-		return self.ntoa()
+		return self.ntoa
 
 	def __str__(self):
-		return self.ntoa()
+		return self.ntoa
+	
+	@property
+	def addr(self):
+		return self._addr
+
+	@property
+	def family(self):
+		return self._family
+
+	@property
+	def plen(self):
+		return self._plen
+
+	@property
+	def raw(self):
+		"""The raw address
+
+		Should only be set to a non-empty string if prior address
+		conversion wasn't possible
+		"""
+		return self._raw
+
+	@property
+	def isValid(self):
+		"""Either the object corresponds to a valid IP address
+		"""
+		return self._isValid
 
 	def __eq__(self, other):
-		if not isinstance(other, IPAddr):
-			if other is None: return False
-			other = IPAddr(other)
-		if not self.valid and not other.valid: return self.raw == other.raw
-		if not self.valid or not other.valid: return False
-		if self.addr != other.addr: return False
-		if self.family != other.family: return False
-		if self.plen != other.plen: return False
-		return True
+		if not (self.isValid or other.isValid):
+			return self.raw == other.raw
+		return (
+			(self.isValid and other.isValid) and
+			(self.addr == other.addr) and
+			(self.family == other.family) and
+			(self.plen == other.plen)
+		)
 
 	def __ne__(self, other):
-		if not isinstance(other, IPAddr):
-			if other is None: return True
-			other = IPAddr(other)
-		if not self.valid and not other.valid: return self.raw != other.raw
-		if self.addr != other.addr: return True
-		if self.family != other.family: return True
-		if self.plen != other.plen: return True
-		return False
+		return not (self == other)
 
 	def __lt__(self, other):
-		if not isinstance(other, IPAddr):
-			if other is None: return False
-			other = IPAddr(other)
 		return self.family < other.family or self.addr < other.addr
 
 	def __add__(self, other):
-		if not isinstance(other, IPAddr):
-			other = IPAddr(other)
 		return "%s%s" % (self, other)
 
 	def __radd__(self, other):
-		if not isinstance(other, IPAddr):
-			other = IPAddr(other)
 		return "%s%s" % (other, self)
 
 	def __hash__(self):
-		# should be the same as by string (because of possible compare with string):
-		return hash(self.ntoa())
-		#return hash(self.addr)^hash((self.plen<<16)|self.family)
+		return hash(self.addr) ^ hash((self.plen << 16) | self.family)
 
+	@property
 	def hexdump(self):
-		""" dump the ip address in as a hex sequence in
-			network byte order - for debug purpose
+		"""Hex representation of the IP address (for debug purposes)
 		"""
 		if self.family == socket.AF_INET:
 			return "%08x" % self.addr
@@ -258,132 +254,94 @@ class IPAddr(object):
 			return "%032x" % self.addr
 		else:
 			return ""
-	
+
+	# TODO: could be lazily evaluated
+	@property
 	def ntoa(self):
-		""" represent IP object as text like the depricated 
-			C pendant inet_ntoa() but address family independent
+		""" represent IP object as text like the deprecated
+			C pendant inet.ntoa but address family independent
 		"""
-		if self.family == socket.AF_INET:
+		if self.isIPv4:
 			# convert network to host byte order
-			binary = struct.pack("!L", self.addr)
-		elif self.family == socket.AF_INET6:
+			binary = struct.pack("!L", self._addr)
+		elif self.isIPv6:
 			# convert network to host byte order
 			hi = self.addr >> 64
 			lo = self.addr & 0xFFFFFFFFFFFFFFFFL
 			binary = struct.pack("!QQ", hi, lo)
 		else:
-			return self.getRaw()
+			return self.raw
 
 		return socket.inet_ntop(self.family, binary)
 
 	def getPTR(self, suffix=""):
-		""" generates the DNS PTR string of the provided IP address object
-			if "suffix" is provided it will be appended as the second and top
+		""" return the DNS PTR string of the provided IP address object
+
+			If "suffix" is provided it will be appended as the second and top
 			level reverse domain.
-			if omitted it is implicitely set to the second and top level reverse
+			If omitted it is implicitly set to the second and top level reverse
 			domain of the according IP address family
 		"""
-		if self.family == socket.AF_INET:
-			reversed_ip = ".".join(reversed(self.ntoa().split(".")))
+		if self.isIPv4:
+			exploded_ip = self.ntoa.split(".")
 			if not suffix:
 				suffix = "in-addr.arpa."
-
-			return "%s.%s" % (reversed_ip, suffix)
-
-		elif self.family == socket.AF_INET6:
-			reversed_ip = ".".join(reversed(self.hexdump()))
+		elif self.isIPv6:
+			exploded_ip = self.hexdump()
 			if not suffix:
-				suffix =  "ip6.arpa."
-
-			return "%s.%s" % (reversed_ip, suffix)
-			
+				suffix = "ip6.arpa."
 		else:
 			return ""
 
+		return "%s.%s" % (".".join(reversed(exploded_ip)), suffix)
+
+	@property
 	def isIPv4(self):
-		""" return true if the IP object is of address family AF_INET
+		"""Either the IP object is of address family AF_INET
 		"""
 		return self.family == socket.AF_INET
 
+	@property
 	def isIPv6(self):
-		""" return true if the IP object is of address family AF_INET6
+		"""Either the IP object is of address family AF_INET6
 		"""
 		return self.family == socket.AF_INET6
 
-	def getRaw(self):
-		""" returns the raw attribute - should only be set
-			to a non-empty string if prior address conversion
-			wasn't possible
-		"""
-		return self.raw
-
-	def isValidIP(self):
-		""" returns true if the IP object has been created
-			from a valid IP address or false if not
-		"""
-		return self.valid
-
-	
 	def isInNet(self, net):
-		""" returns true if the IP object is in the provided
-			network (object)
+		"""Return either the IP object is in the provided network
 		"""
-		# if it isn't a valid IP address, try DNS resolution
-		if not net.isValidIP() and net.getRaw() != "":
-			# Check if IP in DNS
-			return self in DNSUtils.dnsToIp(net.getRaw())
-
 		if self.family != net.family:
 			return False
-
-		if self.family == socket.AF_INET:
+		if self.isIPv4:
 			mask = ~(0xFFFFFFFFL >> net.plen)
-
-		elif self.family == socket.AF_INET6:
+		elif self.isIPv6:
 			mask = ~(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFL >> net.plen)
 		else:
 			return False
 		
-		if self.addr & mask == net.addr:
-			return True
+		return self.addr & mask == net.addr
 
-		return False
+	@staticmethod
+	def masktoplen(mask):
+		"""Convert mask string to prefix length
 
-	@property
-	def maskplen(self):
+		To be used only for IPv4 masks
+		"""
+		mask = mask.addr  # to avoid side-effect within original mask
 		plen = 0
-		if (hasattr(self, '_maskplen')):
-			return self._plen
-		maddr = self.addr
-		while maddr:
-			if not (maddr & 0x80000000):
-				raise ValueError("invalid mask %r, no plen representation" % (self.ntoa(),))
-			maddr = (maddr << 1) & 0xFFFFFFFFL
+		while mask:
+			mask = (mask << 1) & 0xFFFFFFFFL
 			plen += 1
-		self._maskplen = plen
 		return plen
 
-		
-	@staticmethod
-	def masktoplen(maskstr):
-		""" converts mask string to prefix length
-			only used for IPv4 masks
-		"""
-		return IPAddr(maskstr).maskplen
-
-
 	@staticmethod
 	def searchIP(text):
-		""" Search if an IP address if directly available and return
-			it.
+		"""Search if text is an IP address, and return it if so, else None
 		"""
 		match = IPAddr.IP_CRE.match(text)
-		if match:
-			return match
-		else:
+		if not match:
 			match = IPAddr.IP6_CRE.match(text)
-			if match:
-				return match
-			else:
-				return None
+		return match if match else None
 
+# An IPv4 compatible IPv6 to be reused
+_IPv6_v4COMPAT = IPAddr("::ffff:0:0", 96)
diff --git a/fail2ban/tests/clientbeautifiertestcase.py b/fail2ban/tests/clientbeautifiertestcase.py
index d14030ea..704aedd0 100644
--- a/fail2ban/tests/clientbeautifiertestcase.py
+++ b/fail2ban/tests/clientbeautifiertestcase.py
@@ -107,5 +107,3 @@ class BeautifierTest(unittest.TestCase):
 
 		self.assertEqual(self.b.beautify(response), output)
 
-
-