[2440] extended RdataEncoder so it can begin with existing ecoded data.

some test cases are hardcoded, not all cases are covered yet.

src/lib/datasrc/memory/rdata_serialization.cc

@@ -384,10 +384,38 @@ RdataLess(const ConstRdataPtr& rdata1, const ConstRdataPtr& rdata2) {
 
 struct RdataEncoder::RdataEncoderImpl {
     RdataEncoderImpl() : encode_spec_(NULL), rrsig_buffer_(0),
+                         old_rdata_count_(0), old_sig_count_(0),
+                         old_data_len_(0), old_sig_len_(0),
+                         old_length_fields_(NULL), old_data_(NULL),
+                         old_sig_data_(NULL),
                          rdatas_(boost::bind(RdataLess, _1, _2)),
                          rrsigs_(boost::bind(RdataLess, _1, _2))
     {}
 
+    // Common initialization for RdataEncoder::start().
+    void start(RRClass rrclass, RRType rrtype) {
+        if (rrtype == RRType::RRSIG()) {
+            isc_throw(BadValue, "RRSIG cannot be encoded as main RDATA type");
+        }
+
+        encode_spec_ = &getRdataEncodeSpec(rrclass, rrtype);
+        current_class_ = rrclass;
+        current_type_ = rrtype;
+        field_composer_.clearLocal(encode_spec_);
+        rrsig_buffer_.clear();
+        rrsig_lengths_.clear();
+        old_rdata_count_ = 0;
+        old_sig_count_ = 0;
+        old_data_len_ = 0;
+        old_sig_len_ = 0;
+        old_length_fields_ = NULL;
+        old_data_ = NULL;
+        old_sig_data_ = NULL;
+
+        rdatas_.clear();
+        rrsigs_.clear();
+    }
+
     const RdataEncodeSpec* encode_spec_; // encode spec of current RDATA set
     RdataFieldComposer field_composer_;
     util::OutputBuffer rrsig_buffer_;
@@ -396,6 +424,14 @@ struct RdataEncoder::RdataEncoderImpl {
     boost::optional<RRClass> current_class_;
     boost::optional<RRType> current_type_;
 
+    size_t old_rdata_count_;
+    size_t old_sig_count_;
+    size_t old_data_len_;
+    size_t old_sig_len_;
+    const void* old_length_fields_;
+    const void* old_data_;
+    const void* old_sig_data_;
+
     // Temporary storage of Rdata and RRSIGs to be encoded.  They are used
     // to detect and ignore duplicate data.
     typedef boost::function<bool(const ConstRdataPtr&, const ConstRdataPtr&)>
@@ -416,18 +452,29 @@ RdataEncoder::~RdataEncoder() {
 
 void
 RdataEncoder::start(RRClass rrclass, RRType rrtype) {
-    if (rrtype == RRType::RRSIG()) {
-        isc_throw(BadValue, "RRSIG cannot be encoded as main RDATA type");
-    }
+    impl_->start(rrclass, rrtype);
+}
 
-    impl_->encode_spec_ = &getRdataEncodeSpec(rrclass, rrtype);
-    impl_->current_class_ = rrclass;
-    impl_->current_type_ = rrtype;
-    impl_->field_composer_.clearLocal(impl_->encode_spec_);
-    impl_->rrsig_buffer_.clear();
-    impl_->rrsig_lengths_.clear();
-    impl_->rdatas_.clear();
-    impl_->rrsigs_.clear();
+void
+RdataEncoder::start(RRClass rrclass, RRType rrtype, const void* old_data,
+                    size_t old_rdata_count, size_t old_sig_count)
+{
+    impl_->start(rrclass, rrtype);
+
+    // hardcode for initial test
+    assert(old_rdata_count == 1);
+    assert(old_sig_count == 0);
+    if (rrtype == RRType::A()) {
+        impl_->old_data_ = old_data;
+        impl_->old_data_len_ = 4;
+    } else {
+        const uint8_t* cp = static_cast<const uint8_t*>(old_data);
+        impl_->old_rdata_count_ = 1;
+        impl_->old_length_fields_ = old_data;
+        impl_->old_data_ = cp +
+            (old_rdata_count + old_sig_count) * sizeof(uint16_t);
+        impl_->old_data_len_ = 12;
+    }
 }
 
 void
@@ -483,8 +530,11 @@ RdataEncoder::getStorageLength() const {
                   "RdataEncoder::getStorageLength performed before start");
     }
 
-    return (sizeof(uint16_t) * impl_->field_composer_.data_lengths_.size() +
-            sizeof(uint16_t) * impl_->rrsig_lengths_.size() +
+    return (sizeof(uint16_t) * (impl_->old_rdata_count_ +
+                                impl_->old_sig_count_ +
+                                impl_->field_composer_.data_lengths_.size() +
+                                impl_->rrsig_lengths_.size()) +
+            impl_->old_data_len_ + impl_->old_sig_len_ +
             impl_->rrsig_buffer_.getLength() +
             impl_->field_composer_.getLength());
 }
@@ -507,6 +557,12 @@ RdataEncoder::encode(void* buf, size_t buf_len) const {
     uint8_t* dp = dp_beg;
     uint16_t* lenp = reinterpret_cast<uint16_t*>(buf);
 
+    // Encode list of lengths for variable length fields for old data (if any)
+    const size_t old_varlen_fields_len =
+        impl_->old_rdata_count_ * sizeof(uint16_t);
+    std::memcpy(lenp, impl_->old_length_fields_, old_varlen_fields_len);
+    lenp += impl_->old_rdata_count_;
+    dp += old_varlen_fields_len;
     // Encode list of lengths for variable length fields (if any)
     if (!impl_->field_composer_.data_lengths_.empty()) {
         const size_t varlen_fields_len =
@@ -516,6 +572,9 @@ RdataEncoder::encode(void* buf, size_t buf_len) const {
         lenp += impl_->field_composer_.data_lengths_.size();
         dp += varlen_fields_len;
     }
+    // Encode list of lengths for old RRSIGs (if any)
+    // TBD
+    //
     // Encode list of lengths for RRSIGs (if any)
     if (!impl_->rrsig_lengths_.empty()) {
         const size_t rrsigs_len =
@@ -523,10 +582,15 @@ RdataEncoder::encode(void* buf, size_t buf_len) const {
         std::memcpy(lenp, &impl_->rrsig_lengths_[0], rrsigs_len);
         dp += rrsigs_len;
     }
+    // Encode main old RDATA, if any
+    std::memcpy(dp, impl_->old_data_, impl_->old_data_len_);
+    dp += impl_->old_data_len_;
     // Encode main RDATA
     std::memcpy(dp, impl_->field_composer_.getData(),
                 impl_->field_composer_.getLength());
     dp += impl_->field_composer_.getLength();
+    // Encode old RRSIGs, if any
+    // TBD
     // Encode RRSIGs, if any
     std::memcpy(dp, impl_->rrsig_buffer_.getData(),
                 impl_->rrsig_buffer_.getLength());
@@ -547,7 +611,7 @@ RdataReader::RdataReader(const RRClass& rrclass, const RRType& rrtype,
     var_count_total_(spec_.varlen_count * rdata_count),
     sig_count_(sig_count),
     spec_count_(spec_.field_count * rdata_count),
-    // The lenghts are stored first
+    // The lengths are stored first
     lengths_(reinterpret_cast<const uint16_t*>(data)),
     // And the data just after all the lengths
     data_(reinterpret_cast<const uint8_t*>(data) +

src/lib/datasrc/memory/rdata_serialization.h

@@ -155,6 +155,9 @@ public:
     /// \param rrtype The RR type of RDATA to be encoded in the session.
     void start(dns::RRClass rrclass, dns::RRType rrtype);
 
+    void start(dns::RRClass rrclass, dns::RRType rrtype,
+               const void* old_data, size_t rdata_count, size_t sig_count);
+
     /// \brief Add an RDATA for encoding.
     ///
     /// This method updates internal state of the \c RdataEncoder() with the

src/lib/datasrc/tests/memory/rdata_serialization_unittest.cc

@@ -133,7 +133,7 @@ protected:
                                           "20120715220826 12345 com. FAKE"))
     {}
 
-    // A wraper for RdataEncoder::encode() with buffer overrun check.
+    // A wrapper for RdataEncoder::encode() with buffer overrun check.
     void encodeWrapper(size_t data_len);
 
     // Some commonly used RDATA
@@ -161,15 +161,28 @@ public:
     // in the wire format.  It then compares the wire data with the one
     // generated by the normal libdns++ interface to see the encoding/decoding
     // works as intended.
+    // By default it encodes the given RDATAs from the scratch; if old_data
+    // is non NULL, the test case assumes it points to previously encoded data
+    // and the given RDATAs are to be merged with it.  old_rdata/rrsig_count
+    // will be set to the number of RDATAs and RRSIGs encoded in old_data.
+    // These "count" variables must not be set to non 0 unless old_data is
+    // non NULL, but it's not checked in this methods; it's the caller's
+    // responsibility to ensure that.  rdata_list and rrsig_list should contain
+    // all RDATAs and RRSIGs included those stored in old_data.
     void checkEncode(RRClass rrclass, RRType rrtype,
                      const vector<ConstRdataPtr>& rdata_list,
                      size_t expected_varlen_fields,
                      const vector<ConstRdataPtr>& rrsig_list =
-                     vector<ConstRdataPtr>());
+                     vector<ConstRdataPtr>(),
+                     const void* old_data = NULL,
+                     size_t old_rdata_count = 0,
+                     size_t old_rrsig_count = 0);
 
     void addRdataCommon(const vector<ConstRdataPtr>& rrsigs);
     void addRdataMultiCommon(const vector<ConstRdataPtr>& rrsigs,
                              bool duplicate = false);
+    void mergeRdataCommon(const vector<ConstRdataPtr>& old_rrsigs,
+                          const vector<ConstRdataPtr>& rrsigs);
 };
 
 // Used across more classes and scopes. But it's just uninteresting
@@ -275,11 +288,12 @@ public:
                        size_t rdata_count,
                        size_t rrsig_count,
                        size_t expected_varlen_fields,
-                       // Warning: this test actualy might change the
-                       // encoded_data !
-                       vector<uint8_t>& encoded_data, size_t,
+                       const vector<uint8_t>& encoded_data_orig, size_t,
                        MessageRenderer& renderer)
     {
+        // Make a manual copy, which we're going to modify.
+        vector<uint8_t> encoded_data = encoded_data_orig;
+
         // If this type of RDATA is expected to contain variable-length fields,
         // we brute force the encoded data, exploiting our knowledge of actual
         // encoding, then adjust the encoded data excluding the list of length
@@ -546,7 +560,9 @@ RdataEncodeDecodeTest<DecoderStyle>::
 checkEncode(RRClass rrclass, RRType rrtype,
             const vector<ConstRdataPtr>& rdata_list,
             size_t expected_varlen_fields,
-            const vector<ConstRdataPtr>& rrsig_list)
+            const vector<ConstRdataPtr>& rrsig_list,
+            const void* old_data, size_t old_rdata_count,
+            size_t old_rrsig_count)
 {
     // These two names will be rendered before and after the test RDATA,
     // to check in case the RDATA contain a domain name whether it's
@@ -586,13 +602,25 @@ checkEncode(RRClass rrclass, RRType rrtype,
     // 1st dummy name
     actual_renderer_.writeName(dummy_name);
 
-    // Create encoded data
-    encoder_.start(rrclass, rrtype);
+    // Create encoded data.  If old_xxx_count > 0, that part should be in
+    // old_data, so should be excluded from addRdata/addSIGRdata.
+    if (old_data) {
+        encoder_.start(rrclass, rrtype, old_data, old_rdata_count,
+                       old_rrsig_count);
+    } else {
+        encoder_.start(rrclass, rrtype);
+    }
+    size_t count = 0;
     BOOST_FOREACH(const ConstRdataPtr& rdata, rdata_list) {
-        encoder_.addRdata(*rdata);
+        if (++count > old_rdata_count) {
+            encoder_.addRdata(*rdata);
+        }
     }
+    count = 0;
     BOOST_FOREACH(const ConstRdataPtr& rdata, rrsig_list) {
-        encoder_.addSIGRdata(*rdata);
+        if (++count > old_rrsig_count) {
+            encoder_.addSIGRdata(*rdata);
+        }
     }
     const size_t storage_len = encoder_.getStorageLength();
     encodeWrapper(storage_len);
@@ -824,6 +852,64 @@ TEST_F(RdataSerializationTest, badAddRdata) {
                  isc::BadValue);
 }
 
+template<class DecoderStyle>
+void
+RdataEncodeDecodeTest<DecoderStyle>::
+mergeRdataCommon(const vector<ConstRdataPtr>& old_rrsigs,
+                 const vector<ConstRdataPtr>& rrsigs)
+{
+    // Test with fixed-length old RDATA
+    rdata_list_.clear();
+    rdata_list_.push_back(a_rdata_);
+    checkEncode(RRClass::IN(), RRType::A(), rdata_list_, 0, old_rrsigs);
+    vector<uint8_t> old_encoded_data = encoded_data_;
+
+    ConstRdataPtr a_rdata2 = createRdata(RRType::A(), RRClass::IN(),
+                                         "192.0.2.54");
+    rdata_list_.push_back(a_rdata2);
+    vector<ConstRdataPtr> rrsigs_all = old_rrsigs;
+    rrsigs_all.insert(rrsigs_all.end(), rrsigs.begin(), rrsigs.end());
+    checkEncode(RRClass::IN(), RRType::A(), rdata_list_, 0, rrsigs_all,
+                &old_encoded_data[0], 1, old_rrsigs.size());
+
+    // Test with variable-length old RDATA
+    rdata_list_.clear();
+    rrsigs_all.clear();
+    ConstRdataPtr txt_rdata1 = createRdata(RRType::TXT(), RRClass::IN(),
+                                           "foo bar baz");
+    rdata_list_.push_back(txt_rdata1);
+    checkEncode(RRClass::IN(), RRType::TXT(), rdata_list_, 1, old_rrsigs);
+    old_encoded_data = encoded_data_;
+
+    ConstRdataPtr txt_rdata2 = createRdata(RRType::TXT(), RRClass::IN(),
+                                          "another text data");
+    rdata_list_.push_back(txt_rdata2);
+    rrsigs_all.insert(rrsigs_all.end(), rrsigs.begin(), rrsigs.end());
+    checkEncode(RRClass::IN(), RRType::TXT(), rdata_list_, 1, rrsigs_all,
+                &old_encoded_data[0], 1, old_rrsigs.size());
+}
+
+TYPED_TEST(RdataEncodeDecodeTest, mergeRdata) {
+    vector<ConstRdataPtr> old_rrsigs;
+    vector<ConstRdataPtr> rrsigs;
+
+    // Test without RRSIGs, either for old or new.
+    this->mergeRdataCommon(old_rrsigs, rrsigs);
+
+    // Test without RRSIG for old and with RRSIG for new.
+    rrsigs.push_back(this->rrsig_rdata_);
+    this->mergeRdataCommon(old_rrsigs, rrsigs);
+
+#if 0
+    // Tests with two RRSIGs
+    rrsigs.push_back(this->rrsig_rdata_);
+    rrsigs.push_back(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                 "A 5 2 3600 20120814220826 "
+                                 "20120715220826 54321 com. FAKE"));
+    this->addRdataMultiCommon(rrsigs);
+#endif
+}
+
 void
 checkSigData(const ConstRdataPtr& decoded, bool* called, const void* encoded,
              size_t length)