BIND 10 #1462: check SOA serial in xfrout

BIND 10 Development do-not-reply at isc.org
Wed Jan 4 03:12:06 UTC 2012


#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:17 jinmei]:
 > Replying to [comment:15 kevin_tes]:
 >
 > > Hi jinmei, i am not so sure about it, thanks for the information
 >
 > If you've not done it, maybe you want to take a look at
 > http://bind10.isc.org/wiki/CodeReviewProcedure
 > Some (most?) of it are outdated, but the general idea should still hold.

 Thanks again,jinmei. I have already read this page, I am just not sure
 'you' are the reviewer or who just makes some comments about this ticket
 not intent to take it. Now I am sure.
 >
 > The changes don't seem to address my concerns.  Rather than explaining
 > what's missing and running more develop-review cycle, I've directly
 > revised the branch with the changes I'd envision.
 >
 > First, see 0f43276 and 8c0f14c.  These are what I wanted to preserve
 > the original test scenarios.
 >
 > Second, see 0fa9421.  This kind of test confirms we really use serial
 > number arithmetic.  Your previous test didn't fail even if I reverted
 > get_soa_serial() to the integer version:
 > {{{#!python
 > def get_soa_serial(soa_rdata):
 >     return int(soa_rdata.to_text().split()[2])
 >     #return Serial(int(soa_rdata.to_text().split()[2]))
 > }}}
 > and didn't actually test what should have been tested.
 >
 > In general, if we add a test case for a bug fix, that test should fail
 > before applying the fix.  Please make this sure for subsequent
 > changes.
 Thanks,I'll do that.
 >
 > If you agree with the changes I made, I'm okay with the branch.
 > Please feel free to merge.
 >
 I have read all the 'news', thanks for the effort and the work.
 I agree with those.
 > As for the changelog entry:
 >
 > > > 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)
 >
 > I'd say it's a "bug" (fix).  This is similar to changelog entry 338
 > (Trac #1299).  Also, "add" is not entirely correct because we already
 > did some check (i.e, equality check).  And, the git hash should be the
 > merge commit of the corresponding branch, which is not known yet.  So
 > at the proposal phase we'd normally use a placeholder such as "TBD".
 > Considering these, I'd revise it to:
 > {{{
 > 347.? [bug]           kevin
 >       Corrected SOA serial check in xfrout.  It now compares the SOA
 >       serial of an IXFR query with that of the server based serial
 >       number arithmetic, and replies with a single SOA record of the
 >       server's current version if the former is equal to or newer than
 >       the latter.
 >       (Trac #1462, git TBD)
 > }}}
 >
 I agree with it.
 > (Don't forget updating "TBD" when you actually add this to the master)

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


More information about the bind10-tickets mailing list