From: Michal 'vorner' Vaner Date: Mon, 3 Oct 2011 09:40:46 +0000 (+0200) Subject: [1259] Check the thing is not used after exception X-Git-Tag: perftcpdns_before_epoll~37^2~30^2^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c6babcd3e44bc42fdb090d3a4837848d8c7c149c;p=thirdparty%2Fkea.git [1259] Check the thing is not used after exception As it would not be safe anyway. --- diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py index 082bdb38a4..4291883f0d 100644 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@ -73,12 +73,13 @@ class Diff: def __check_commited(self): """ - This checks if the diff is already commited. If it is, it raises - ValueError. This check is for methods that need to work only on + This checks if the diff is already commited or broken. If it is, it + raises ValueError. This check is for methods that need to work only on yet uncommited diffs. """ if self.__updater is None: - raise ValueError("The diff is already commited, you come late") + raise ValueError("The diff is already commited or it has raised " + + "an exception, you come late") def __data_common(self, rr, operation): """ @@ -172,15 +173,21 @@ class Diff: self.__check_commited() # First, compact the data self.compact() - # Then pass the data inside the data source - for (operation, rrset) in self.__buffer: - if operation == 'add': - self.__updater.add_rrset(rrset) - elif operation == 'remove': - self.__updater.remove_rrset(rrset) - else: - raise ValueError('Unknown operation ' + operation) - # As everything is already in, drop the buffer + try: + # Then pass the data inside the data source + for (operation, rrset) in self.__buffer: + if operation == 'add': + self.__updater.add_rrset(rrset) + elif operation == 'remove': + self.__updater.remove_rrset(rrset) + else: + raise ValueError('Unknown operation ' + operation) + # As everything is already in, drop the buffer + except: + # If there's a problem, we can't continue. + self.__updater = None + raise + self.__buffer = [] def commit(self): @@ -195,10 +202,15 @@ class Diff: # Push the data inside the data source self.apply() # Make sure they are visible. - self.__updater.commit() - # Remove the updater. That will free some resources for one, but - # mark this object as already commited, so we can check - self.__updater = None + try: + self.__updater.commit() + finally: + # Remove the updater. That will free some resources for one, but + # mark this object as already commited, so we can check + + # We remove it even in case the commit failed, as that makes us + # unusable. + self.__updater = None def get_buffer(self): """ diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index dc1aca966e..0152d0718b 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -18,6 +18,13 @@ import unittest from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata from isc.xfrin.diff import Diff, NoSuchZone +class TestError(Exception): + """ + Just to have something to be raised during the tests. + Not used outside. + """ + pass + class DiffTest(unittest.TestCase): """ Tests for the isc.xfrin.diff.Diff class. @@ -37,6 +44,7 @@ class DiffTest(unittest.TestCase): self.__data_operations = [] self.__apply_called = False self.__commit_called = False + self.__broken_called = False # Some common values self.__rrclass = RRClass.IN() self.__type = RRType.A() @@ -73,6 +81,15 @@ class DiffTest(unittest.TestCase): """ self.__apply_called = True + def __broken_operation(self, *args): + """ + This can be used whenever an operation should fail. It raises TestError. + It should take whatever amount of parameters needed, so it can be put + quite anywhere. + """ + self.__broken_called = True + raise TestError("Test error") + def commit(self): """ This is part of pretending to be a zone updater. This notes the commit @@ -340,6 +357,45 @@ class DiffTest(unittest.TestCase): self.assertRaises(ValueError, diff.add_data, rrset) self.assertRaises(ValueError, diff.remove_data, rrset) + def __do_raise_test(self): + """ + Do a raise test. Expects that one of the operations is exchanged for + broken version. + """ + diff = Diff(self, Name('example.org.')) + diff.add_data(self.__rrset1) + diff.remove_data(self.__rrset2) + self.assertRaises(TestError, diff.commit) + self.assertTrue(self.__broken_called) + self.assertRaises(ValueError, diff.add_data, self.__rrset1) + self.assertRaises(ValueError, diff.remove_data, self.__rrset2) + self.assertRaises(ValueError, diff.commit) + self.assertRaises(ValueError, diff.apply) + + def test_raise_add(self): + """ + Test the exception from add_rrset is propagated and the diff can't be + used afterwards. + """ + self.add_rrset = self.__broken_operation + self.__do_raise_test() + + def test_raise_remove(self): + """ + Test the exception from remove_rrset is propagated and the diff can't be + used afterwards. + """ + self.remove_rrset = self.__broken_operation + self.__do_raise_test() + + def test_raise_commit(self): + """ + Test the exception from updater's commit gets propagated and it can't be + used afterwards. + """ + self.commit = self.__broken_operation + self.__do_raise_test() + if __name__ == "__main__": isc.log.init("bind10") unittest.main()