BIND 10 #1462: check SOA serial in xfrout

[BIND 10 Development]

#1462: check SOA serial in xfrout
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  kevin_tes
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120110
                  Component:         |            Resolution:
  xfrout                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:  AXFR-  |           Total Hours:  0
  out                                |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by kevin_tes):

 Replying to [comment:13 jinmei]:
 > Replying to [comment:10 kevin_tes]:
 >
 > Shane already reassigned the ticket back to me (as expected), but in
 > case you didn't know it rather than forgetting it: in general, if you
 > have addressed review comments the next step is to give the ticket
 > back to the original reviewer.  If you change it to "unassigned", the
 > ticket could be dangling.
 Hi jinmei, i am not so sure about it, thanks for the information.
 >
 > > > - before doing that, add a test case that fails due to the integer
 > > >   comparison (e.g. 0xffffffff "<" 1).  see, again, xfrin.  Then make
 > > >   the above change, and then confirm it fixes the problem.
 > > I think this case for a while,and i think it does not to add this
 test.
 > > The first reason:The CURRENT_SOA_SERIAL=2011112001,SOA=2011119000 can
 test this case,and isc::dns::Serial has such test too.
 > > If add a test case that fails due to the integer comparison (e.g.
 0xffffffff "<" 1), i think it needs to change the  CURRENT_SOA_SERIAL,I
 think it does not need to change this exited codes too.
 >
 > I personally don't think it (skipping the explicit test) a good idea.
 > First, if I understand it correctly, the test case with
 > CURRENT_SOA_SERIAL=2011112001, SOA=2011119000 cannot detect an error
 > that integer comparison is incorrectly used instead of comparison by
 > Serial objects.  Second, the main purpose of using a test case like
 > 0xffffffff "<" 1 is not for testing the Serial object works correctly
 > but to check we really use Serial objects instead of integers and
 > really compare serials as Serial objects not as integers.  It will
 > also detect if and when introduce a regression we accidentally
 > reintroduce the same type of bug in future.  So the fact that it's
 > tested in the Serial class implementation cannot be a reason for
 > skipping it here.
 >
 > Also, I didn't suggest changing the definition of CURRENT_SOA_SERIAL,
 > etc.  For the test like 0xffffffff "<" 1 I expected to introduce
 > another constant (or hardcoded values if they are only used in that
 > specific tests).
 >
 I added some codes to test these case.
 > > > - ixfr-to-axfr test cases shouldn't be removed (we need to _SERIAL
 the
 > > >   test parameters and check the fallback behavior, not just remove
 it).
 > > > - I believe we need a changelog for it.
 > > I have recovered this test and changed the test parameters.
 >
 > I don't see the recover of the ixfr-to-axfr (and axfr-style of ixfr)
 > test in the latest revision (e99849d).  Perhaps you forgot pushing
 please see:
 1082     def test_ixfr_to_axfr(self):
 1083         self.xfrsess._request_data = \
 1084              self.create_request_data(ixfr=2000000000)
 1085         XfroutSession._handle(self.xfrsess)
 1086         response = self.sock.read_msg(Message.PRESERVE_ORDER);
 1087         self.assertEqual(Rcode.NOERROR(), response.get_rcode())
 1088         # The SOA serial is greater than that of requested one.
 1089         # So only the SOA was send.
 1090         self.assertEqual(RRType.IXFR(),
 response.get_question()[0].get_type())
 1091         self.check_axfr_stream(response);
 > your local change?
 >
 > Please also show proposed changelog entry.
   [func]        kevin
   Add SOA serial check in xfrout. When an IXFR query with the newer
 version
   number than that of the server is received, it is replied to with a
 single
   SOA record of the server's current version.
   (Trac #1462, git d394e64f4c44f16027b1e62b4ac34e054b49221d)

 Thanks again. It's ready now.

-- 
Ticket URL: <https://bind10.isc.org/ticket/1462#comment:15>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development