BIND 10 #2267: new generateRRsetFromIterator doesn't handle RRSIGs correctly

[BIND 10 Development]

#2267: new generateRRsetFromIterator doesn't handle RRSIGs correctly
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |                       Status:  new
            Priority:  medium        |                    Milestone:  Next-
           Component:  Unclassified  |  Sprint-Proposed
           Sensitive:  0             |                     Keywords:
         Sub-Project:  DNS           |              Defect Severity:  N/A
Estimated Difficulty:  0             |  Feature Depending on Ticket:
         Total Hours:  0             |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
 The new (memory-efficient) version of in memory loader from another
 data source has a bug.  It cannot load RRSIGs correctly if a single
 name has multiple types of RRsets and their RRSIGs.

 It can be confirmed by applying the following patch to the unit test.
 load() will fail with an exception.

 {{{#!diff
  const char* rrset_data[] = {
      "example.org. 3600 IN SOA   ns1.example.org. bugs.x.w.example.org. 68
 3600 300 3600000 3600",
      "a.example.org.                     3600 IN A      192.168.0.1",
 +    "a.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959
 20051021000000 "
 +    "40430 example.org. FAKEFAKEFAKE",
      "a.example.org.                     3600 IN MX     10
 mail.example.org.",
      NULL
  };
 }}}

 Even ignoring this bug, the current implementation doesn't seem to be
 very understandable due to the last_rrset_ member variable.  It's
 quite difficult to keep track of this thing.

 My suggested fix is this:

 - simplify generateRRsetFromIterator() so it now just passes each
   RRset in the returned order.
 - implement the buffer logic in `InMemoryClientImpl::addFromLoad()`.
   it ensures a matched pair of RRset and (if signed) its RRSIG RRset
   is generated.  (it can assume if the different names of RRsets aren't
   interleaved.)  To implement this, it's probably better to introduce
   a helper `Loader` class.
 - revise `InMemoryClientImpl::addFromLoad()` so it takes RRset and its
   RRSIG.  Then we won't need the `last_rrset_` magic.
 - `InMemoryClient::add()` (and maybe some tests) will need to be updated.

 Also, we should merge the older loadFromIterator test.

 I'd also deprecate the impl class.  The in-memory client is now almost
 private itself, so the advantages of pimpl aren't that big.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2267>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development