BIND 10 trac1261, updated. 3898b36a132fe44e51cc99674104d9e1f0d35d36 [1261] made clear XfrinState().handle_rr() shouldn't be called directly via an exception. added a test for that.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Oct 5 23:55:30 UTC 2011


The branch, trac1261 has been updated
       via  3898b36a132fe44e51cc99674104d9e1f0d35d36 (commit)
       via  ed7eecab42af0064d261d9c9dafd701250bbc1d3 (commit)
       via  d6616e7ef66b3904e2d585e7b4946900f67d3b70 (commit)
       via  c4344fadc93b62af473a8e05fc3a453256e4ce13 (commit)
      from  cf136247fad510f55ba230f746558274fada1de6 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 3898b36a132fe44e51cc99674104d9e1f0d35d36
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Oct 5 16:53:58 2011 -0700

    [1261] made clear XfrinState().handle_rr() shouldn't be called directly
    via an exception.   added a test for that.

commit ed7eecab42af0064d261d9c9dafd701250bbc1d3
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Oct 5 16:43:46 2011 -0700

    [1261] cleanup: removed an unused variable

commit d6616e7ef66b3904e2d585e7b4946900f67d3b70
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Oct 5 16:10:04 2011 -0700

    [1261] added more detailed documentation about SOA mismatch case
    for non incremental update

commit c4344fadc93b62af473a8e05fc3a453256e4ce13
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Oct 5 15:28:03 2011 -0700

    [1261] a small wording fix as suggested in review

-----------------------------------------------------------------------

Summary of changes:
 src/bin/xfrin/tests/xfrin_test.py |   17 +++++++++----
 src/bin/xfrin/xfrin.py.in         |   46 ++++++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index 39a8d55..9fcdf44 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -267,6 +267,15 @@ class TestXfrinState(unittest.TestCase):
         self.conn._datasrc_client = MockDataSourceClient()
         self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME)
 
+class TestXfrinStateBase(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+
+    def test_handle_rr_on_base(self):
+        # The base version of handle_rr() isn't supposed to be called
+        # directly (the argument doesn't matter in this test)
+        self.assertRaises(XfrinException, XfrinState().handle_rr, None)
+
 class TestXfrinInitialSOA(TestXfrinState):
     def setUp(self):
         super().setUp()
@@ -314,11 +323,9 @@ class TestXfrinFirstData(TestXfrinState):
         self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate()))
 
     def test_handle_ixfr_to_axfr_by_different_soa(self):
-        # Response contains two consecutive SOA but the serial of the second
-        # does not match the requested one.  The only possible interpretation
-        # at this point is that it's an AXFR-compatible IXFR that only
-        # consists of the SOA RR.  It will result in broken zone and should
-        # be rejected anyway, but at this point we should switch to AXFR.
+        # An unusual case: Response contains two consecutive SOA but the
+        # serial of the second does not match the requested one.  See
+        # the documentation for XfrinFirstData.handle_rr().
         self.assertFalse(self.state.handle_rr(self.conn, soa_rrset))
         self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate()))
 
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index 5eb0e3f..1ffd841 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -204,7 +204,9 @@ class XfrinState:
         sequence or completing the session.
 
         All subclass has their specific behaviors for this method, so
-        there is no default definition.
+        there is no default definition.  If the base class version
+        is called, it's a bug of the caller, and it's notified via
+        an XfrinException exception.
 
         This method returns a boolean value: True if the given RR was
         fully handled and the caller should go to the next RR; False
@@ -212,7 +214,8 @@ class XfrinState:
         state for the same RR again.
 
         '''
-        pass
+        raise XfrinException("Internal bug: " +
+                             "XfrinState.handle_rr() called directly")
 
     def finish_message(self, conn):
         '''Perform any final processing after handling all RRs of a response.
@@ -229,7 +232,7 @@ class XfrinInitialSOA(XfrinState):
     def handle_rr(self, conn, rr):
         if rr.get_type() != RRType.SOA():
             raise XfrinProtocolError('First RR in zone transfer must be SOA ('
-                                     + rr.get_type().to_text() + ' given)')
+                                     + rr.get_type().to_text() + ' received)')
         conn._end_serial = get_soa_serial(rr.get_rdata()[0])
 
         # FIXME: we need to check the serial is actually greater than ours.
@@ -243,9 +246,37 @@ class XfrinInitialSOA(XfrinState):
 
 class XfrinFirstData(XfrinState):
     def handle_rr(self, conn, rr):
-        # If the transfer begins with one SOA record, it is an AXFR,
-        # if it begins with two SOAs (the serial of the second one being
-        # equal to our serial), it is an IXFR.
+        '''Handle the first RR after initial SOA in an XFR session.
+
+        This state happens exactly once in an XFR session, where
+        we decide whether it's incremental update ("real" IXFR) or
+        non incremental update (AXFR or AXFR-style IXFR).
+        If we initiated IXFR and the transfer begins with two SOAs
+        (the serial of the second one being equal to our serial),
+        it's incremental; otherwise it's non incremental.
+
+        This method always return False (unlike many other handle_rr()
+        methods) because this first RR must be examined again in the
+        determined update context.
+
+        Note that in the non incremental case the RR should normally be
+        something other SOA, but it's still possible it's an SOA with a
+        different serial than ours.  The only possible interpretation at
+        this point is that it's non incremental update that only consists
+        of the SOA RR.  It will result in broken zone (for example, it
+        wouldn't even contain an apex NS) and should be rejected at post
+        XFR processing, but in terms of the XFR session processing we
+        accept it and move forward.
+
+        Note further that, in the half-broken SOA-only transfer case,
+        these two SOAs are supposed to be the same as stated in Section 2.2
+        of RFC 5936.  We don't check that condition here, either; we'll
+        leave whether and how to deal with that situation to the end of
+        the processing of non incremental update.  See also a related
+        discussion at the IETF dnsext wg:
+        http://www.ietf.org/mail-archive/web/dnsext/current/msg07908.html
+
+        '''
         if conn._request_type == RRType.IXFR() and \
                 rr.get_type() == RRType.SOA() and \
                 conn._request_serial == get_soa_serial(rr.get_rdata()[0]):
@@ -256,7 +287,7 @@ class XfrinFirstData(XfrinState):
             logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP,
                  conn.zone_str())
             self.set_xfrstate(conn, XfrinAXFR())
-        return False    # need to revisit this RR in an update context
+        return False
 
 class XfrinIXFRDeleteSOA(XfrinState):
     def handle_rr(self, conn, rr):
@@ -547,7 +578,6 @@ class XfrinConnection(asyncore.dispatcher):
             # to hardcode here.
             request_str = 'IXFR' if request_type == RRType.IXFR() else 'AXFR'
             if check_soa:
-                logstr = 'SOA check for \'%s\' ' % self.zone_str()
                 ret =  self._check_soa_serial()
 
             if ret == XFRIN_OK:




More information about the bind10-changes mailing list