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