[2440] make sure old rdata don't contain duplicate; add doc and tests on this.

diff --git a/src/lib/datasrc/memory/rdata_serialization.cc b/src/lib/datasrc/memory/rdata_serialization.cc
index 01ddd7f65ebc8a1b6d4ff63bd6243fc945789c1d..a56880d0765ec2ea5958ea9640019d0ad4234b6a 100644 (file)
--- a/src/lib/datasrc/memory/rdata_serialization.cc
+++ b/src/lib/datasrc/memory/rdata_serialization.cc
@@ -505,7 +505,9 @@ RdataEncoder::start(RRClass rrclass, RRType rrtype, const void* old_data,
     // keep them in rdatas_ and rrsigs_ so we can detect and ignore duplicate
     // data with the existing one later.  We'll also figure out the lengths
     // of the RDATA and RRSIG part of the data by iterating over the data
-    // fields.
+    // fields.  Note that the given old_data shouldn't contain duplicate
+    // Rdata or RRSIG as they should have been generated by this own class,
+    // which ensures that condition; if this assumption doesn't hold, we throw.
     size_t total_len = 0;
     RdataReader reader(rrclass, rrtype, old_data, old_rdata_count,
                        old_sig_count,
@@ -516,8 +518,11 @@ RdataEncoder::start(RRClass rrclass, RRType rrtype, const void* old_data,
     while (reader.iterateRdata()) {
         util::InputBuffer ibuffer(impl_->olddata_buffer_.getData(),
                                   impl_->olddata_buffer_.getLength());
-        impl_->rdatas_.insert(createRdata(rrtype, rrclass, ibuffer,
-                                          impl_->olddata_buffer_.getLength()));
+        if (!impl_->rdatas_.insert(
+                createRdata(rrtype, rrclass, ibuffer,
+                            impl_->olddata_buffer_.getLength())).second) {
+            isc_throw(Unexpected, "duplicate RDATA found in merging RdataSet");
+        }
         impl_->olddata_buffer_.clear();
     }
     impl_->old_data_len_ = total_len;
@@ -526,8 +531,11 @@ RdataEncoder::start(RRClass rrclass, RRType rrtype, const void* old_data,
     while (reader.iterateSingleSig()) {
         util::InputBuffer ibuffer(impl_->olddata_buffer_.getData(),
                                   impl_->olddata_buffer_.getLength());
-        impl_->rrsigs_.insert(createRdata(RRType::RRSIG(), rrclass, ibuffer,
-                                          impl_->olddata_buffer_.getLength()));
+        if (!impl_->rrsigs_.insert(
+                createRdata(RRType::RRSIG(), rrclass, ibuffer,
+                            impl_->olddata_buffer_.getLength())).second) {
+            isc_throw(Unexpected, "duplicate RRSIG found in merging RdataSet");
+        }
         impl_->olddata_buffer_.clear();
     }
     impl_->old_sig_len_ = total_len;

diff --git a/src/lib/datasrc/memory/rdata_serialization.h b/src/lib/datasrc/memory/rdata_serialization.h
index 8edf47e882f7b02c55efae0ebc31753bc1fea47a..e46786cf44804535b87f06affc947d26047e0ac1 100644 (file)
--- a/src/lib/datasrc/memory/rdata_serialization.h
+++ b/src/lib/datasrc/memory/rdata_serialization.h
@@ -175,6 +175,17 @@ public:
     /// for another attempt of encoding, which is unlikely), the caller can
     /// modify or destroy the old data.
     ///
+    /// The caller must also ensure that \c old_data don't contain any
+    /// duplicate Rdata or RRSIG.  Normally the caller doesn't have to do
+    /// anything special to meet this requirement, though, as the data
+    /// should have been generated by an \c RdataEncoder object before,
+    /// which guarantees that condition.  But this method checks the
+    /// assumption in case it was crafted or otherwise broken data, and
+    /// throws an exception if that is the case.
+    ///
+    /// \throw Unexpected Given encoded data contain duplicate Rdata or RRSIG
+    /// (normally shouldn't happen, see the description).
+    ///
     /// \param rrclass The RR class of RDATA to be encoded in the session.
     /// \param rrtype The RR type of RDATA to be encoded in the session.
     /// \param old_data Point to previously encoded data for the same RR

diff --git a/src/lib/datasrc/tests/memory/rdata_serialization_unittest.cc b/src/lib/datasrc/tests/memory/rdata_serialization_unittest.cc
index b23e07fdd60955a759aa8b83b046841fc355a532..fb01dd6f6455cc9194d613d0bd609d117dcef089 100644 (file)
--- a/src/lib/datasrc/tests/memory/rdata_serialization_unittest.cc
+++ b/src/lib/datasrc/tests/memory/rdata_serialization_unittest.cc
@@ -1003,6 +1003,33 @@ TYPED_TEST(RdataEncodeDecodeTest, mergeRdata) {
     this->mergeRdataCommon(old_rrsigs, rrsigs);
 }
 
+TEST_F(RdataSerializationTest, mergeRdataFromDuplicate) {
+    // create encoded data containing duplicate Rdata and try to start a
+    // new encoding session in the merge mode with that data.  It breaks the
+    // assumption and should result in an exception.
+    const uint8_t data[] = { 192, 0, 2, 1, 192, 0, 2, 1 };
+    EXPECT_THROW(encoder_.start(RRClass::IN(), RRType::A(), data, 2, 0),
+                 isc::Unexpected);
+
+    // Same for duplicate RRSIG
+    isc::util::OutputBuffer buffer(0);
+    vector<uint8_t> sigdata;
+    rrsig_rdata_->toWire(buffer);
+    const uint16_t sig_len = buffer.getLength();
+    // Encode lengths of RRSIGs, 2 bytes each, in host byte order
+    const uint8_t* const lp = static_cast<const uint8_t*>(
+        static_cast<const void*>(&sig_len));
+    sigdata.insert(sigdata.end(), lp, lp + sizeof(sig_len));
+    sigdata.insert(sigdata.end(), lp, lp + sizeof(sig_len));
+    // then the RRSIG data
+    const uint8_t* const dp = static_cast<const uint8_t*>(buffer.getData());
+    sigdata.insert(sigdata.end(), dp, dp + sig_len);
+    sigdata.insert(sigdata.end(), dp, dp + sig_len);
+
+    EXPECT_THROW(encoder_.start(RRClass::IN(), RRType::A(), &sigdata[0], 0, 2),
+                 isc::Unexpected);
+}
+
 void
 checkSigData(const ConstRdataPtr& decoded, bool* called, const void* encoded,
              size_t length)