BIND 10 #1310: auth NSEC support: Handle WILDCARD_NXRRSET case

[BIND 10 Development]

#1310: auth NSEC support: Handle WILDCARD_NXRRSET case
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  kevin_tes
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111122
                  Component:         |            Resolution:
  b10-auth                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:         |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by kevin_tes):

 Replying to [comment:13 stephen]:
 > '''src/bin/auth/query.cc'''[[BR]]
 > Indentation of addWildcardNxrrsetProof() uses tabs - it should use
 spaces.
 I set my vim editor that a tabs equals four spaces,,,is it ok? Otherwise i
 'll change it.

 >
 > The first block of comments in addWildcardNxrrsetProof() do not apply to
 the immediately following statements, but to the block of statements after
 that - the one starting:
 > {{{
 > const ZoneFinder::FindResult fresult =
 > }}}
 >
 > As far as I can determine, the first "if" block:
 > {{{
 > if (nsec->getRdataCount() == 0) {
 > }}}
 > is a check that the the first NSEC RR in 3.1.3.4 - the one that proves
 there are no RRsets of the STYPE requested at the name that matched
 <SNAME, SCLASS> via wildcard expansion - is present in the "nsec" RRset.
 The reason for this check should be commented.

 good advice, i'll add it.

 >
 > Adding the NSEC RRsets to the Authority section is done via the
 construct:
 > {{{
 > if names are equal {
 >    add fresult.rrset
 > } else {
 >    add nsec
 >    add fresult.rrset
 > }
 > }}}
 > The "add fresult rrset" is common to both branches of the "if" statement
 and so can be outside it.
 >
 > If the above change is made, then part of the processing in
 addWildcardNxrrsetProof() is identical to the processing in
 addWildcardProof().  It suggests that the two could be combined: add a
 second argument to addWildcardProof() that defaults to an empty
 ConstRRsetPtr.  The code would then:
 > * add the standard wildcard NSEC record
 > * if "nsec" is not null:
 >   * test if it contains any NSEC records; if not, thrown an exception
 >   * add to the Authority section if the name is different from the
 standard wildcard record
 >
 good advice, i'll changed it.
 >
 > '''src/bin/auth/query.h'''[[BR]]
 > The header for addWildcardNxrrsetProof() should include Doxygen tags
 describing the arguments.
 >
 > Although not really part of this ticket, while addressing the above
 comment the opportunity should be taken to add Doxygen tags to the headers
 of the other "addXxx()" methods.
 >
 > The headers of these methods should be expanded to do a better
 description as to how the methods are used.  It took quite a bit of
 searching (and reading [http://tools.ietf.org/hrml/rfc4035 RFC 4035]) to
 find out how the logic worked (i.e. that addWildcardNxrrsetProof() depends
 on the fact that ZoneFinder::find() in this case - Wildcard No Data -
 returns the NSEC that covers the interval where the empty non-terminal
 lives, but not the NSEC for the wildcard demanded by
 [http://tools.ietf.org/hrml/rfc4035 RFC 4035], thus requiring the addition
 of the wildcard NSEC).
 >
 good advice,i'll fix it.
 >
 > '''src/bin/auth/tests/query_unittest.cc'''[[BR]]
 > There should be two tests: one for the case that returns two NSEC
 records, and one that returns a single NSEC record (to prove that the
 duplicate NSEC is not included in the Authority section).
 >

 say my comment http://bind10.isc.org/ticket/1310?replyto=11#comment

 but i'll fixed it.

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