amend to a54b401ee2 (0.11-fix-get-current-bans):

- differentiate between unknown ban-time after upgrade (-2) and persistent ban-time (-1);
- better handling to correct current ban-time, uses `bantime.maxtime` (if increment allowed) or `bantime` of jail;
- extended test-cases in order to cover the change of `bantime.maxtime`.

fail2ban/server/database.py

@@ -378,9 +378,9 @@ class Fail2BanDb(object):
 							"COMMIT;" % Fail2BanDb._CREATE_TABS['logs'])
 
 			if version < 3 and self._tableExists(cur, "bans"):
-				# set ban-time to -1 (note it means rather unknown, as persistent, will be fixed by restore):
+				# set ban-time to -2 (note it means rather unknown, as persistent, will be fixed by restore):
 				cur.executescript("BEGIN TRANSACTION;"
-							"CREATE TEMPORARY TABLE bans_temp AS SELECT jail, ip, timeofban, -1 as bantime, 1 as bancount, data FROM bans;"
+							"CREATE TEMPORARY TABLE bans_temp AS SELECT jail, ip, timeofban, -2 as bantime, 1 as bancount, data FROM bans;"
 							"DROP TABLE bans;"
 							"%s;\n"
 							"INSERT INTO bans SELECT * from bans_temp;"
@@ -747,7 +747,7 @@ class Fail2BanDb(object):
 		if ip is not None:
 			query += " AND ip=?"
 			queryArgs.append(ip)
-		query += " AND (timeofban + bantime > ? OR bantime = -1)"
+		query += " AND (timeofban + bantime > ? OR bantime <= -1)"
 		queryArgs.append(fromtime)
 		if forbantime not in (None, -1): # not specified or persistent (all)
 			query += " AND timeofban > ?"
@@ -773,7 +773,9 @@ class Fail2BanDb(object):
 		tickets = []
 		ticket = None
 		if correctBanTime is True:
-			correctBanTime = jail.actions.getBanTime() if jail is not None else None
+			correctBanTime = jail.getMaxBanTime() if jail is not None else None
+			# don't change if persistent allowed:
+			if correctBanTime == -1: correctBanTime = None
 
 		for ticket in self._getCurrentBans(cur, jail=jail, ip=ip, 
 			forbantime=forbantime, fromtime=fromtime
@@ -781,12 +783,17 @@ class Fail2BanDb(object):
 			# can produce unpack error (database may return sporadical wrong-empty row):
 			try:
 				banip, timeofban, bantime, bancount, data = ticket
-				# if persistent ban (or still unknown after upgrade), use current bantime of the jail:
-				if correctBanTime and (bantime == -1 or bantime > correctBanTime):
-					bantime = correctBanTime
 				# additionally check for empty values:
 				if banip is None or banip == "": # pragma: no cover
 					raise ValueError('unexpected value %r' % (banip,))
+				# if bantime unknown (after upgrade-db from earlier version), just use min known ban-time:
+				if bantime == -2: # todo: remove it in future version
+					bantime = jail.actions.getBanTime() if jail is not None else (
+						correctBanTime if correctBanTime else 600)
+				elif correctBanTime:
+					# if persistent ban (or greater as max), use current max-bantime of the jail:
+					if bantime == -1 or bantime > correctBanTime:
+						bantime = correctBanTime
 			except ValueError as e: # pragma: no cover
 				logSys.debug("get current bans: ignore row %r - %s", ticket, e)
 				continue

fail2ban/server/jail.py

@@ -263,14 +263,23 @@ class Jail(object):
 			return self._banExtra.get(opt, None)
 		return self._banExtra
 
+	def getMaxBanTime(self):
+		"""Returns max possible ban-time of jail.
+		"""
+		return self._banExtra.get("maxtime", -1) \
+			if self._banExtra.get('increment') else self.actions.getBanTime()
+
 	def restoreCurrentBans(self, correctBanTime=True):
 		"""Restore any previous valid bans from the database.
 		"""
 		try:
 			if self.database is not None:
-				forbantime = None;
-				# use ban time as search time if we have not enabled a increasing:
-				if not self.getBanTimeExtra('increment'):
+				if self._banExtra.get('increment'):
+					forbantime = None;
+					if correctBanTime:
+						correctBanTime = self.getMaxBanTime()
+				else:
+					# use ban time as search time if we have not enabled a increasing:
 					forbantime = self.actions.getBanTime()
 				for ticket in self.database.getCurrentBans(jail=self, forbantime=forbantime,
 					correctBanTime=correctBanTime

fail2ban/tests/databasetestcase.py

@@ -170,9 +170,9 @@ class DatabaseTest(LogCaptureTestCase):
 			self.assertEqual(self.db.updateDb(Fail2BanDb.__version__), Fail2BanDb.__version__)
 			self.assertRaises(NotImplementedError, self.db.updateDb, Fail2BanDb.__version__ + 1)
 			# check current bans (should find exactly 1 ticket after upgrade):
-			tickets = self.db.getCurrentBans(fromtime=1388009242, correctBanTime=False)
+			tickets = self.db.getCurrentBans(fromtime=1388009242, correctBanTime=123456)
 			self.assertEqual(len(tickets), 1)
-			self.assertEqual(tickets[0].getBanTime(), -1); # ban-time still unknown (normally updated from jail)
+			self.assertEqual(tickets[0].getBanTime(), 123456); # ban-time was unknown (normally updated from jail)
 		finally:
 			if self.db and self.db._dbFilename != ":memory:":
 				os.remove(self.db._dbBackupFilename)

fail2ban/tests/observertestcase.py

@@ -386,6 +386,7 @@ class BanTimeIncrDB(unittest.TestCase):
 
 		# two separate jails :
 		jail1 = DummyJail(backend='polling')
+		jail1.setBanTimeExtra('increment', 'true')
 		jail1.database = self.db
 		self.db.addJail(jail1)
 		jail2 = DummyJail(name='DummyJail-2', backend='polling')
@@ -433,7 +434,8 @@ class BanTimeIncrDB(unittest.TestCase):
 		# jail2 does not restore any bans (because all ban tickets should be already expired: stime-6000):
 		jail2.restoreCurrentBans(correctBanTime=False)
 		self.assertEqual(jail2.getFailTicket(), False)
-		# test again, but now normally (with maximum ban-time of restored ticket allowed):
+		# test again, but now normally (with maximum ban-time of restored ticket = allowed 10m = 600):
+		jail1.setBanTimeExtra('maxtime', '10m')
 		jail1.restoreCurrentBans()
 		ticket = jail1.getFailTicket()
 		self.assertTrue(ticket.restored)