BIND 10 #1808: support NSEC for normal NXRRSET in in-memory

[BIND 10 Development]

#1808: support NSEC for normal NXRRSET in in-memory
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  jinmei                             |                Status:  closed
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120515
  medium                             |            Resolution:  complete
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  3
            Defect Severity:  N/A    |           Total Hours:  2.5
Feature Depending on Ticket:  in-    |
  memory NSEC                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:20 kevin_tes]:

 > > > > ?? It doesn't make sense to me.  The original findNSECSigned test
 had
 > > > > other cases unrelated to NXRRSET.
 > > > Until now I do not get the 'lost test cases' for removing
 findNSECSigned. If we keep this test, I think rewrite work should be done.
 > >
 > > In this branch, you did:

 > > How could the new NSEC4NXRRSET be a replacement of all of this
 > > function previously tested?
 >
 > Ok，here we have
 > {{{
 > TEST_F(InMemoryZoneFinderTest, find) {
 >         findCheck();
 > }
 > }}}
 > That's to say
 > {{{
 > > InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags
 expected_flags) {
 > >     // Fill some data inside
 > >     // Now put all the data we have there. It should throw nothing
 > >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_)));
 > >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_a_)));
 > >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS,
 zone_finder_.add(rr_ns_aaaa_)));
 > >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_a_)));
 > >     if ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0) {
 > >         EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec3_));
 > >     }
 > >
 > >     // These two should be successful
 > >     findTest(origin_, RRType::NS(), ZoneFinder::SUCCESS, true,
 rr_ns_);
 > >     findTest(rr_ns_a_->getName(), RRType::A(), ZoneFinder::SUCCESS,
 true,
 > >              rr_ns_a_);
 >
 > ...
 > >     // These domains don't exist (and one is out of the zone)
 > >     findTest(Name("nothere.example.org"), RRType::A(),
 ZoneFinder::NXDOMAIN,
 > >              true, ConstRRsetPtr(), expected_flags);
 >
 > "expected_flags" can be test by the new unittest NSEC4NXRRSET .
 >
 > >     EXPECT_THROW(zone_finder_.find(Name("example.net"), RRType::A()),
 > >                  OutOfZone);
 > }}}
 > Can be test by this unittest.

 Do you mean this set of tests are covered by
 InMemoryZoneFinderTest::find?  If so, I don't think that's correct; at
 the very least we still need to check "expected_flags" are correct in
 findTest when the zone is NSEC signed.  Also, it should be obvious
 we'll need to check the NSEC for the NXDOMAIN case.  Besides, the
 whole point of sharing findCheck() for various cases is to perform
 comprehensive checks slightly varying conditions (with or without
 NSEC/NSEC3, with or without FIND_DNSSEC).  Adding random specific test
 cases for each combination (as NSEC4NXRRSET does) doesn't scale and
 will very likely cause uncovered cases.

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