[master] Merge branch 'trac2095' with fixing conflicts.

doc/Doxyfile

@@ -573,7 +573,8 @@ WARN_LOGFILE           =
 # with spaces.
 
 INPUT                  = ../src/lib/exceptions ../src/lib/cc \
-    ../src/lib/config ../src/lib/cryptolink ../src/lib/dns ../src/lib/datasrc \
+    ../src/lib/config ../src/lib/cryptolink ../src/lib/dns \
+    ../src/lib/datasrc ../src/lib/datasrc/memory \
     ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log \
     ../src/lib/log/compiler ../src/lib/asiolink/ ../src/lib/nsas \
     ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \

src/lib/datasrc/memory/rdata_encoder.cc

@@ -12,20 +12,23 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
+#include <util/buffer.h>
+
 #include <dns/name.h>
 #include <dns/labelsequence.h>
+#include <dns/messagerenderer.h>
 #include <dns/rdata.h>
-#include <dns/rdataclass.h>     // for a test function
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 
-#include <util/buffer.h>        // for test functions
-
 #include "rdata_encoder.h"
 
 #include <boost/static_assert.hpp>
 
 #include <cassert>
+#include <cstring>
 #include <vector>
 
 #include <stdint.h>
@@ -58,6 +61,23 @@ struct RdataFieldSpec {
 };
 
 /// Specification of RDATA in terms of internal encoding.
+///
+/// The fields must be a sequence of:
+/// <0 or 1 fixed/var-len data field>,
+/// <1 or more domain name fields>,
+/// <1 fixed/var-len data field>,
+/// <1 or more domain name fields>,
+/// <1 fixed/var-len data field>,
+/// ...and so on.
+/// There must not be more than one consecutive data fields (i.e., without
+/// interleaved by a domain name); it would just be inefficient in terms of
+/// memory footprint and iterating over the fields, and it would break
+/// some assumption within the encoder implementation.  For consecutive
+/// data fields in the DNS protocol, if all fields have fixed lengths, they
+/// should be combined into a single fixed-length field (like the last 20
+/// bytes of SOA RDATA).  If there's a variable length field, they should be
+/// combined into a single variable-length field (such as DNSKEY, which has
+/// 3 fixed-length fields followed by one variable-length field).
 struct RdataEncodeSpec {
     const uint16_t field_count; // total number of fields (# of fields member)
     const uint16_t name_count;  // number of domain name fields
@@ -243,72 +263,272 @@ getRdataEncodeSpec(RRClass rrclass, RRType rrtype) {
     return (generic_data_spec);
 }
 
-// A temporary helper of temporary encodeRdata(): it calculates the length
-// of the data portion of a NAPTR RDATA (i.e., the RDATA fields before the
-// "replacement" name).
+// This class is a helper for RdataEncoder to divide the content of RDATA
+// fields for encoding by "abusing" the  message rendering logic.
+// The idea is to identify domain name fields in the writeName() method,
+// while keeping track of the size and position of other types of data
+// around the names.
+//
+// Technically, this use of inheritance may be considered a violation of
+// Liskov Substitution Principle in that it doesn't actually compress domain
+// names, and some of the methods are not expected to be used.
+// In fact, skip() or trim() may not be make much sense in this context.
+// Nevertheless we keep this idea at the moment.  Since the usage is limited
+// (it's only used within this file, and only used with \c Rdata variants),
+// it's hopefully an acceptable practice.
+class RdataFieldComposer : public AbstractMessageRenderer {
+public:
+    RdataFieldComposer() : last_data_pos_(0), encode_spec_(NULL),
+                           current_field_(0)
+    {}
+    virtual ~RdataFieldComposer() {}
+    virtual bool isTruncated() const { return (false); }
+    virtual size_t getLengthLimit() const { return (65535); }
+    virtual CompressMode getCompressMode() const { return (CASE_INSENSITIVE); }
+    virtual void setTruncated() {}
+    virtual void setLengthLimit(size_t) {}
+    virtual void setCompressMode(CompressMode) {}
+
+    // Called for each domain name in the RDATA, from the RDATA's toWire()
+    // implementation.
+    virtual void writeName(const Name& name, bool compress) {
+        // First, see if we have other data already stored in the renderer's
+        // buffer, and handle it appropriately.
+        updateOtherData();
+
+        // Then, we should still have a field in the spec, and it must be a
+        // domain name field.
+        if (current_field_ >= encode_spec_->field_count) {
+            isc_throw(BadValue,
+                      "RDATA encoder encounters an unexpected name data: " <<
+                      name);
+        }
+        const RdataFieldSpec& field =
+            encode_spec_->fields[current_field_++];
+        // Since we know we've passed any prior data field, the next field
+        // must be a domain name as long as it exists; otherwise it's a bug
+        // in the spec (not a bogus input).  So we assert() that condition.
+        assert(field.type == RdataFieldSpec::DOMAIN_NAME);
+
+        // It would be compressed iff the field has that attribute.
+        if (compress !=
+            ((field.name_attributes & NAMEATTR_COMPRESSIBLE) != 0)) {
+            isc_throw(BadValue, "RDATA encoder error, inconsistent name "
+                      "compression policy: " << name);
+        }
+
+        const LabelSequence labels(name);
+        labels.serialize(labels_placeholder_, sizeof(labels_placeholder_));
+        writeData(labels_placeholder_, labels.getSerializedLength());
+
+        last_data_pos_ += labels.getSerializedLength();
+    }
+    // Clear all internal states and resources for a new set of RDATA.
+    void clearLocal(const RdataEncodeSpec* encode_spec) {
+        AbstractMessageRenderer::clear();
+        encode_spec_ = encode_spec;
+        data_lengths_.clear();
+        last_data_pos_ = 0;
+    }
+    // Called at the beginning of an RDATA.
+    void startRdata() {
+        current_field_ = 0;
+    }
+    // Called at the end of an RDATA.
+    void endRdata() {
+        // Handle any remaining data (there should be no more name).  Then
+        // we should reach the end of the fields.
+        updateOtherData();
+        if (current_field_ != encode_spec_->field_count) {
+            isc_throw(BadValue,
+                      "RDATA encoder didn't find all expected fields");
+        }
+    }
+
+    // Hold the lengths of variable length fields, in the order of their
+    // appearance.  For convenience, allow the encoder to refer to it
+    // directly.
+    vector<uint16_t> data_lengths_;
+
+private:
+    // We use generict write* methods, with the exception of writeName.
+    // So new data can arrive without us knowing it, this considers all new
+    // data to be just data, checking consistency with the field spec, and
+    // if it contains variable-length field, record its length.
+    size_t last_data_pos_;
+    void updateOtherData() {
+        // If we've reached the end of the fields or we are expecting a
+        // domain name, there's nothing to do here.
+        if (current_field_ >= encode_spec_->field_count ||
+            encode_spec_->fields[current_field_].type ==
+            RdataFieldSpec::DOMAIN_NAME) {
+            return;
+        }
+
+        const size_t cur_pos = getLength();
+        const size_t data_len = cur_pos - last_data_pos_;
+
+        const RdataFieldSpec& field = encode_spec_->fields[current_field_];
+        if (field.type == RdataFieldSpec::FIXEDLEN_DATA) {
+            // The data length of a fixed length field must be the one
+            // specified in the field spec.
+            if (data_len != field.fixeddata_len) {
+                isc_throw(BadValue,
+                          "RDATA encoding: available data too short for the "
+                          "type");
+            }
+        } else {
+            // For encoding purposes, a variable-length data field is
+            // a single field covering all data, even if it may
+            // consist of multiple fields as DNS RDATA (e.g. TXT).
+            if (data_len > 0xffff) {
+                isc_throw(RdataEncodingError, "RDATA field is too large: "
+                          << data_len << " bytes");
+            }
+            data_lengths_.push_back(data_len);
+        }
+
+        ++current_field_;
+        last_data_pos_ = cur_pos;
+    }
+
+    // The RDATA field spec of the current session.  Set at the beginning of
+    // each session.
+    const RdataEncodeSpec* encode_spec_;
+    // the RDATA field (for encoding) currently handled.  Reset to 0 for
+    // each RDATA of the session.
+    size_t current_field_;
+    // Placeholder to convert a name object to a label sequence.
+    uint8_t labels_placeholder_[LabelSequence::MAX_SERIALIZED_LENGTH];
+};
+} // end of unnamed namespace
+
+struct RdataEncoder::RdataEncoderImpl {
+    RdataEncoderImpl() : encode_spec_(NULL), rrsig_buffer_(0),
+                         rdata_count_(0)
+    {}
+
+    const RdataEncodeSpec* encode_spec_; // encode spec of current RDATA set
+    RdataFieldComposer field_composer_;
+    util::OutputBuffer rrsig_buffer_;
+    size_t rdata_count_;
+    vector<uint16_t> rrsig_lengths_;
+};
+
+RdataEncoder::RdataEncoder() :
+    impl_(new RdataEncoderImpl)
+{}
+
+RdataEncoder::~RdataEncoder() {
+    delete impl_;
+}
+
+void
+RdataEncoder::start(RRClass rrclass, RRType rrtype) {
+    if (rrtype == RRType::RRSIG()) {
+        isc_throw(BadValue, "RRSIG cannot be encoded as main RDATA type");
+    }
+
+    impl_->encode_spec_ = &getRdataEncodeSpec(rrclass, rrtype);
+    impl_->field_composer_.clearLocal(impl_->encode_spec_);
+    impl_->rrsig_buffer_.clear();
+    impl_->rdata_count_ = 0;
+    impl_->rrsig_lengths_.clear();
+}
+
+void
+RdataEncoder::addRdata(const rdata::Rdata& rdata) {
+    if (impl_->encode_spec_ == NULL) {
+        isc_throw(InvalidOperation,
+                  "RdataEncoder::addRdata performed before start");
+    }
+
+    impl_->field_composer_.startRdata();
+    rdata.toWire(impl_->field_composer_);
+    impl_->field_composer_.endRdata();
+    ++impl_->rdata_count_;
+}
+
+void
+RdataEncoder::addSIGRdata(const rdata::Rdata& sig_rdata) {
+    if (impl_->encode_spec_ == NULL) {
+        isc_throw(InvalidOperation,
+                  "RdataEncoder::addSIGRdata performed before start");
+    }
+    const size_t cur_pos = impl_->rrsig_buffer_.getLength();
+    sig_rdata.toWire(impl_->rrsig_buffer_);
+    const size_t rrsig_datalen = impl_->rrsig_buffer_.getLength() - cur_pos;
+    if (rrsig_datalen > 0xffff) {
+        isc_throw(RdataEncodingError, "RRSIG is too large: "
+                  << rrsig_datalen << " bytes");
+    }
+    impl_->rrsig_lengths_.push_back(rrsig_datalen);
+}
+
 size_t
-getNAPTRDataLen(const rdata::Rdata& rdata) {
-    const rdata::generic::NAPTR& naptr_rdata =
-        dynamic_cast<const rdata::generic::NAPTR&>(rdata);
+RdataEncoder::getStorageLength() const {
+    if (impl_->encode_spec_ == NULL) {
+        isc_throw(InvalidOperation,
+                  "RdataEncoder::getStorageLength performed before start");
+    }
 
-    util::OutputBuffer buffer(0);
-    rdata.toWire(buffer);
-    return (buffer.getLength() - naptr_rdata.getReplacement().getLength());
+    return (sizeof(uint16_t) * impl_->field_composer_.data_lengths_.size() +
+            sizeof(uint16_t) * impl_->rrsig_lengths_.size() +
+            impl_->rrsig_buffer_.getLength() +
+            impl_->field_composer_.getLength());
 }
-} // end of unnamed namespace
 
-namespace testing {
 void
-encodeRdata(const rdata::Rdata& rdata, RRClass rrclass, RRType rrtype,
-            vector<uint8_t>& data_result, vector<uint16_t>& len_result)
-{
-    util::OutputBuffer buffer(0);
-    rdata.toWire(buffer);
-    util::InputBuffer ibuffer(buffer.getData(), buffer.getLength());
-    vector<uint8_t> tmp;        // used as temporary placeholder below
+RdataEncoder::encode(void* buf, size_t buf_len) const {
+    if (impl_->encode_spec_ == NULL) {
+        isc_throw(InvalidOperation,
+                  "RdataEncoder::encode performed before start");
+    }
+    if (buf == NULL) {
+        isc_throw(BadValue,
+                  "RdataEncoder::encode NULL buffer is given");
+    }
+    if (getStorageLength() > buf_len) {
+        isc_throw(BadValue, "RdataEncoder::encode short buffer given");
+    }
 
-    const RdataEncodeSpec& encode_spec = getRdataEncodeSpec(rrclass, rrtype);
-    for (size_t i = 0; i < encode_spec.field_count; ++i) {
-        const RdataFieldSpec& field_spec = encode_spec.fields[i];
-        switch (field_spec.type) {
-        case RdataFieldSpec::FIXEDLEN_DATA:
-            tmp.resize(field_spec.fixeddata_len);
-            ibuffer.readData(&tmp[0], tmp.size());
-            data_result.insert(data_result.end(), tmp.begin(), tmp.end());
-            break;
-        case RdataFieldSpec::VARLEN_DATA:
-        {
-            // In the vast majority cases of our supported RR types,
-            // variable-length data fields are placed at the end of RDATA,
-            // so the length of the field should be the remaining length
-            // of the output buffer.  The only exception is NAPTR, for which
-            // we use an ad hoc workaround (remember this function is for
-            // initial testing only, and will be cleaned up eventually).
-            const size_t pos = ibuffer.getPosition();
-            const size_t data_len = rrtype == RRType::NAPTR() ?
-                getNAPTRDataLen(rdata) : (ibuffer.getLength() - pos);
-            tmp.resize(data_len);
-            ibuffer.readData(&tmp[0], tmp.size());
-            data_result.insert(data_result.end(), tmp.begin(), tmp.end());
-            len_result.push_back(data_len);
-            break;
-        }
-        case RdataFieldSpec::DOMAIN_NAME:
-        {
-            const Name name(ibuffer);
-            const LabelSequence labels(name);
-            uint8_t labels_holder[LabelSequence::MAX_SERIALIZED_LENGTH];
-            labels.serialize(labels_holder, sizeof(labels_holder));
-            data_result.insert(data_result.end(), labels_holder,
-                               labels_holder + labels.getSerializedLength());
-            break;
-        }
-        }
+    uint8_t* const dp_beg = reinterpret_cast<uint8_t*>(buf);
+    uint8_t* dp = dp_beg;
+    uint16_t* lenp = reinterpret_cast<uint16_t*>(buf);
+
+    // Encode list of lengths for variable length fields (if any)
+    if (!impl_->field_composer_.data_lengths_.empty()) {
+        const size_t varlen_fields_len =
+            impl_->field_composer_.data_lengths_.size() * sizeof(uint16_t);
+        std::memcpy(lenp, &impl_->field_composer_.data_lengths_[0],
+                    varlen_fields_len);
+        lenp += impl_->field_composer_.data_lengths_.size();
+        dp += varlen_fields_len;
     }
+    // Encode list of lengths for RRSIGs (if any)
+    if (!impl_->rrsig_lengths_.empty()) {
+        const size_t rrsigs_len =
+            impl_->rrsig_lengths_.size() * sizeof(uint16_t);
+        std::memcpy(lenp, &impl_->rrsig_lengths_[0], rrsigs_len);
+        dp += rrsigs_len;
+    }
+    // Encode main RDATA
+    std::memcpy(dp, impl_->field_composer_.getData(),
+                impl_->field_composer_.getLength());
+    dp += impl_->field_composer_.getLength();
+    // Encode RRSIGs, if any
+    std::memcpy(dp, impl_->rrsig_buffer_.getData(),
+                impl_->rrsig_buffer_.getLength());
+    dp += impl_->rrsig_buffer_.getLength();
+
+    // The validation at the entrance must ensure this
+    assert(buf_len >= dp - dp_beg);
 }
 
+namespace testing {
 void
 foreachRdataField(RRClass rrclass, RRType rrtype,
+                  size_t rdata_count,
                   const vector<uint8_t>& encoded_data,
                   const vector<uint16_t>& varlen_list,
                   NameCallback name_callback, DataCallback data_callback)
@@ -318,40 +538,62 @@ foreachRdataField(RRClass rrclass, RRType rrtype,
     size_t off = 0;
     size_t varlen_count = 0;
     size_t name_count = 0;
-    for (size_t i = 0; i < encode_spec.field_count; ++i) {
-        const RdataFieldSpec& field_spec = encode_spec.fields[i];
-        switch (field_spec.type) {
-        case RdataFieldSpec::FIXEDLEN_DATA:
-            if (data_callback) {
-                data_callback(&encoded_data.at(off), field_spec.fixeddata_len);
+    for (size_t count = 0; count < rdata_count; ++count) {
+        for (size_t i = 0; i < encode_spec.field_count; ++i) {
+            const RdataFieldSpec& field_spec = encode_spec.fields[i];
+            switch (field_spec.type) {
+            case RdataFieldSpec::FIXEDLEN_DATA:
+                if (data_callback) {
+                    data_callback(&encoded_data.at(off),
+                                  field_spec.fixeddata_len);
+                }
+                off += field_spec.fixeddata_len;
+                break;
+            case RdataFieldSpec::VARLEN_DATA:
+            {
+                const size_t varlen = varlen_list.at(varlen_count);
+                if (data_callback && varlen > 0) {
+                    data_callback(&encoded_data.at(off), varlen);
+                }
+                off += varlen;
+                ++varlen_count;
+                break;
             }
-            off += field_spec.fixeddata_len;
-            break;
-        case RdataFieldSpec::VARLEN_DATA:
-        {
-            const size_t varlen = varlen_list.at(varlen_count);
-            if (data_callback && varlen > 0) {
-                data_callback(&encoded_data.at(off), varlen);
+            case RdataFieldSpec::DOMAIN_NAME:
+            {
+                ++name_count;
+                const LabelSequence labels(&encoded_data.at(off));
+                if (name_callback) {
+                    name_callback(labels, field_spec.name_attributes);
+                }
+                off += labels.getSerializedLength();
+                break;
             }
-            off += varlen;
-            ++varlen_count;
-            break;
-        }
-        case RdataFieldSpec::DOMAIN_NAME:
-        {
-            ++name_count;
-            const LabelSequence labels(&encoded_data.at(off));
-            if (name_callback) {
-                name_callback(labels, field_spec.name_attributes);
             }
-            off += labels.getSerializedLength();
-            break;
-        }
         }
     }
+    assert(name_count == encode_spec.name_count * rdata_count);
+    assert(varlen_count == encode_spec.varlen_count * rdata_count);
+}
+
+void
+foreachRRSig(const vector<uint8_t>& encoded_data,
+             const vector<uint16_t>& rrsiglen_list,
+             DataCallback data_callback)
+{
+    size_t rrsig_totallen = 0;
+    for (vector<uint16_t>::const_iterator it = rrsiglen_list.begin();
+         it != rrsiglen_list.end();
+         ++it) {
+        rrsig_totallen += *it;
+    }
+    assert(encoded_data.size() >= rrsig_totallen);
 
-    assert(name_count == encode_spec.name_count);
-    assert(varlen_count == encode_spec.varlen_count);
+    const uint8_t* dp = &encoded_data[encoded_data.size() - rrsig_totallen];
+    for (size_t i = 0; i < rrsiglen_list.size(); ++i) {
+        data_callback(dp, rrsiglen_list[i]);
+        dp += rrsiglen_list[i];
+    }
 }
 } // namespace testing
 

src/lib/datasrc/memory/rdata_encoder.h

@@ -15,19 +15,97 @@
 #ifndef DATASRC_MEMORY_RDATA_ENCODER_H
 #define DATASRC_MEMORY_RDATA_ENCODER_H 1
 
+#include <exceptions/exceptions.h>
+
 #include <dns/labelsequence.h>
 #include <dns/rdata.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 
 #include <boost/function.hpp>
+#include <boost/noncopyable.hpp>
 
 #include <vector>
 
+/// \file rdata_encoder.h
+/// \brief Set of utility classes for encoding RDATA in memory efficient way.
+///
+/// This file defines a set of interfaces (classes, types, constants) to
+/// manipulate a given set of RDATA of the same type (normally associated with
+/// an RRset) that may be accompanied with RRSIGs in a memory efficient way.
+///
+/// The entire set of RDATA is stored in a packed form in a contiguous
+/// memory region.  It's opaque data, without containing non trivial
+/// data structures, so it can be located anywhere in the memory or even
+/// dumped to a file.
+///
+/// Two main classes are provided: one is
+/// \c isc::datasrc::memory::RdataEncoder, which allows
+/// the application to create encoded data for a set of RDATA;
+/// the other (TBD) provides an interface to iterate over encoded set of
+/// RDATA for purposes such as data lookups or rendering the data into the
+/// wire format to create a DNS message.
+///
+/// The actual encoding detail is private information to the implementation,
+/// and the application shouldn't assume anything about that except that
+/// each RDATA is considered to consist of one or more generic fields,
+/// and each field is typed as either opaque data or a domain name.
+/// A domain name field has additional attributes
+/// (see \c isc::datasrc::memory::RdataNameAttributes)
+/// so the application can change how the name should be handled in terms
+/// of the DNS protocol (e.g., whether it's subject to name compression).
+///
+/// The following are the current implementation of internal encoding, shown
+/// only for reference.  Applications must not assume this particular form
+/// for the encoded data; in fact, it can change in a future version of the
+/// implementation.
+/// \verbatim
+// The encoded data begin with a series of 16-bit length fields (values are
+// stored in the host byte order).  The sequence may be empty.
+// uint16_t n1_1: size of 1st variable len field (if any) of 1st RDATA
+// uint16_t n1_2: size of 2nd variable len field of 1st RDATA
+// ...
+// uint16_t nN_M: size of last (Mth) variable len field of last (Nth) RDATA
+// uint16_t ns1: size of 1st RRSIG (if any) data
+// ...
+// uint16_t nsL: size of last (Lth) RRSIG data
+// A sequence of packed data fields follows:
+// uint8_t[]: data field value, length specified by nI_J (in case it's
+//            variable-length) or by the per type field spec (in case it's
+//            fixed-length).
+// or
+// opaque data, LabelSequence::getSerializedLength() bytes: data for a name
+// uint8_t[ns1]: 1st RRSIG data
+// ...
+// uint8_t[nsL]: last RRSIG data
+// \endverbatim
+///
+/// As described above, this implementation treats RRSIGs as opaque data
+/// that don't contain any domain names.  Technically, it has a "signer"
+/// domain name field in the sense of RFC4034.  In practice, however, this
+/// field is essentially mere data; it's not subject to name compression,
+/// and since it's very likely to be a subdomain of (or equal to) the
+/// owner name of the corresponding RR (or, if used in a DNS message,
+/// some domain name that already appears before this field), so it won't
+/// be a target of name compression either.  By treating the entire RRSIG
+/// as single-field data we can make the implementation simpler, and probably
+/// make it faster in rendering it into a DNS message.
+
 namespace isc {
 namespace datasrc {
 namespace memory {
 
+/// \brief General error in RDATA encoding.
+///
+/// This is thrown when \c RdataEncoder encounters a rare, unsupported
+/// situation. a method is called for a name or RRset which
+/// is not in or below the zone.
+class RdataEncodingError : public Exception {
+public:
+    RdataEncodingError(const char* file, size_t line, const char* what) :
+        Exception(file, line, what) {}
+};
+
 /// \brief Attributes of domain name fields of encoded RDATA.
 ///
 /// The enum values define special traits of the name that can affect how
@@ -40,42 +118,194 @@ enum RdataNameAttributes {
                                                       ///< handling
 };
 
-// We use the following quick-hack version of encoder and "foreach"
-// operator until we implement the complete versions.  The plan is to
+/// \brief RDATA encoder.
+///
+/// This class provides interfaces to encode a set of RDATA of a specific
+/// RR class and type, possibly with their RRSIG RDATAs, in a memory-efficient
+/// format.  In many cases these sets of RDATA come from a specific (signed
+/// or unsigned) RRset.
+///
+/// It is expected for a single \c RdataEncoder object to be used multiple
+/// times for different sets of RDATA, such as in loading an entire zone
+/// into memory.  Each encoding session begins with the \c start() method,
+/// which sets the context for the specific RR class and type to be encoded.
+/// Any number of calls to \c addRdata() or \c addSIGRdata() follow, each
+/// of which updates the internal state of the encoder with the encoding
+/// information for the given RDATA or RRSIG RDATA, respectively.
+/// The \c addRdata() is expected to be called with an
+/// \c isc::dns::rdata::Rdata object
+/// of the specified class and type, and \c addRdata() checks the consistency
+/// for the purpose of encoding (but it's not completely type safe; for
+/// example, it wouldn't distinguish TXT RDATA and HINFO RDATA.
+/// Likewise, an \c isc::dns::rdata::Rdata given to \c addSIGRdata() is
+/// expected to be of RRSIG, but the method does not check the assumption).
+///
+/// After passing the complete set of RDATA and their RRSIG, the application
+/// is expected to call \c getStorageLength() to know the size of storage
+/// that is sufficient to store all encoded data.  Normally the application
+/// would allocate a memory region of that size, and then call \c encode()
+/// with the prepared region.  The \c encode() method dumps encoded data
+/// to the given memory region.
+///
+/// The caller can reuse the \c RdataEncoder object for another set of RDATA
+/// by repeating the session from \c start().
+class RdataEncoder : boost::noncopyable {
+public:
+    /// \brief Default constructor.
+    RdataEncoder();
+
+    /// \brief The destrcutor.
+    ~RdataEncoder();
+
+    /// \brief Start the encoding session.
+    ///
+    /// It re-initializes the internal encoder state for a new encoding
+    /// session.  The \c rrclass and \c rrtype parameters specify the
+    /// type of RDATA to be encoded in the new session.  Note that if the
+    /// set of RDATA is signed, \c rrtype always specifies the "signed" type;
+    /// it must not be RRSIG.
+    ///
+    /// \throw BadValue RRSIG is specified for rrtype.
+    ///
+    /// \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.
+    void start(dns::RRClass rrclass, dns::RRType rrtype);
+
+    /// \brief Add an RDATA for encoding.
+    ///
+    /// This method updates internal state of the \c RdataEncoder() with the
+    /// given RDATA so it will be part of the encoded data in a subsequent
+    /// call to \c encode().
+    ///
+    /// The given \c rdata must be of the RR class and type specified at
+    /// the prior call to \c start().  This method checks the assumption
+    /// to some extent, but the check is not complete; this is generally
+    /// the responsibility of the caller.
+    ///
+    /// The caller can destroy \c rdata after this call is completed.
+    ///
+    /// \note This implementation does not support RDATA (or any subfield of
+    /// it) whose size exceeds 65535 bytes (max uint16_t value).  Such RDATA
+    /// may not necessarily be considered invalid in terms of protocol
+    /// specification, but in practice it's mostly useless because the
+    /// corresponding RR won't fit in any valid DNS message.
+    ///
+    /// As long as the \c rdata is of the correct type and its size is normal,
+    /// this method should normally be exception free.  If it throws, however,
+    /// it doesn't always provide the strong exception guarantee.  In general,
+    /// the caller needs to either destroy the encoder object or restart a
+    /// new session from \c start() should this method throws an exception.
+    ///
+    /// \throw InvalidOperation called before start().
+    /// \throw BadValue inconsistent data found.
+    /// \throw RdataEncodingError A very unusual case, such as over 64KB RDATA.
+    /// \throw std::bad_alloc Internal memory allocation failure.
+    ///
+    /// \param rdata An RDATA to be encoded in the session.
+    void addRdata(const dns::rdata::Rdata& rdata);
+
+    /// \brief Add an RRSIG RDATA for encoding.
+    ///
+    /// This method updates internal state of the \c RdataEncoder() with the
+    /// given RDATA, which is assumed to be of type RRSIG that covers the
+    /// type specified at the time of \c start() for the encoding session.
+    /// The corresponding data for the RRSIG RDATA will be encoded in a
+    /// subsequent call to \c encode().
+    ///
+    /// The passed \c sig_rdata is expected to be of type RRSIG and cover
+    /// the RR type specified at the call to \c start() to this encoding
+    /// session.  But this method does not check if it is the case at all;
+    /// it could even accept any type of RDATA as opaque data.  It's caller's
+    /// responsibility to ensure the assumption.
+    ///
+    /// The caller can destroy \c rdata after this call is completed.
+    ///
+    /// \note Like addRdata(), this implementation does not support
+    /// RRSIG RDATA whose size (in the form of wire format) exceeds 65535
+    /// bytes.
+    ///
+    /// The same note about exception safety as \c addRdata() applies.
+    ///
+    /// \throw InvalidOperation called before start().
+    /// \throw RdataEncodingError A very unusual case, such as over 64KB RDATA.
+    /// \throw std::bad_alloc Internal memory allocation failure.
+    ///
+    /// \param sig_rdata An RDATA to be encoded in the session.  Supposed to
+    /// be of type RRSIG.
+    void addSIGRdata(const dns::rdata::Rdata& sig_rdata);
+
+    /// \brief Return the length of space for encoding for the session.
+    ///
+    /// It returns the size of the encoded data that would be generated for
+    /// the set of RDATA (and RRSIGs) in the encoder at the call of this
+    /// method.  It's ensured that a buffer of that size can be safely passed
+    /// to \c encode() unless there's no other "add" method is called by then.
+    ///
+    /// As long as this method is called after start(), it never throws.
+    ///
+    /// \throw InvalidOperation called before start().
+    ///
+    /// \return The expected size of the encoded data at the time of the call.
+    size_t getStorageLength() const;
+
+    /// \brief Encode RDATAs of the session to a buffer.
+    ///
+    /// This method dumps encoded data for the stored set of RDATA and
+    /// their RRSIGs to a given buffer.  The buffer must have a size
+    /// at least as large as the return value of a prior call to
+    /// \c getStorageLength() (it may be larger than that).
+    ///
+    /// The given buffer must be aligned at the natural boundary for
+    /// 16-bit integers.  The method doesn't check this condition; it's
+    /// caller's responsibility to ensure that.  Note: the alignment
+    /// requirement may change in a future version of this implementation.
+    ///
+    /// As long as this method is called after start() and the buffer is
+    /// valid with a sufficient size, this method never throws.
+    ///
+    /// \throw InvalidOperation called before start().
+    /// \throw BadValue buffer is NULL or it's too short for the encoded data.
+    ///
+    /// \param buf A pointer to the buffer to which encoded data are to be
+    /// dumped.
+    /// \param buf_len The size of the buffer in bytes.
+    void encode(void* buf, size_t buf_len) const;
+
+private:
+    struct RdataEncoderImpl;
+    RdataEncoderImpl* impl_;
+};
+
+// We use the following quick-hack version of "foreach"
+// operators until we implement the complete versions.  The plan is to
 // update the test cases that use these functions with the complete
 // functions/classes, and then remove the entire namespace.
 namespace testing {
-// "Encode" given RDATA of given RR class and type.
-//
-// Fixed/variable-length data fields are encoded in their wire-format;
-// domain names are encoded in the form of:
-//  - nlen: name data length (1 byte)
-//  - olen: offset data length (1 byte)
-//  - name data (nlen bytes)
-//  - offset data (olen bytes)
-//
-// The encoded results are appended to data_result.
-// If the RDATA contain variable-length data fields, the lengths of the
-// these fields will be appended in len_result, in the order of appearance.
-void encodeRdata(const dns::rdata::Rdata& rdata, dns::RRClass rrclass,
-                 dns::RRType rrtype, std::vector<uint8_t>& data_result,
-                 std::vector<uint16_t>& len_result);
-
 // Callbacks used in foreachRdataField.
 typedef boost::function<void(const dns::LabelSequence&,
                              RdataNameAttributes)> NameCallback;
 typedef boost::function<void(const uint8_t*, size_t)> DataCallback;
 
-// Iterate over each RDATA field (in terms of the internal encoding) stored
-// in encoded_data, and call the given callback for each data (for
-// domain name fields, name_callback will be called; for normal data fields
-// data_callback will be called).  If the encoded data contain variable-length
-// data fields, varlen_list should store a sequence of their lengths, in the
-// of the appearance.
+// Iterate over each field (in terms of the internal encoding) of each
+// RDATA stored in encoded_data, and call the given callback for each
+// data (for domain name fields, name_callback will be called; for
+// normal data fields data_callback will be called).  rdata_count is
+// the number of RDATAs.  If the encoded data contain variable-length
+// data fields, varlen_list should store a sequence of their lengths,
+// in the order of the appearance.
 void foreachRdataField(dns::RRClass rrclass, dns::RRType rrtype,
+                       size_t rdata_count,
                        const std::vector<uint8_t>& encoded_data,
                        const std::vector<uint16_t>& varlen_list,
                        NameCallback name_callback, DataCallback data_callback);
+
+// Iterate over each RRSIG stored in encoded_data, and call the given
+// callback for each.  rrsiglen_list should store a sequence of their lengths,
+// in the order of the appearance.  Its size is the number of RRSIGs.
+// The list can be empty, in which case this function does nothing.
+void foreachRRSig(const std::vector<uint8_t>& encoded_data,
+                  const std::vector<uint16_t>& rrsiglen_list,
+                  DataCallback data_callback);
 }
 
 } // namespace memory

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

@@ -12,10 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
+#include <util/buffer.h>
+
 #include <dns/name.h>
 #include <dns/labelsequence.h>
 #include <dns/messagerenderer.h>
 #include <dns/rdata.h>
+#include <dns/rdataclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 
@@ -26,7 +31,9 @@
 #include <gtest/gtest.h>
 
 #include <boost/bind.hpp>
+#include <boost/foreach.hpp>
 
+#include <cstring>
 #include <set>
 #include <string>
 #include <vector>
@@ -49,10 +56,8 @@ struct TestRdata {
     const size_t n_varlen_fields; // expected # of variable-len fields
 };
 
-// This test data consist of all supported types of RDATA (+ some
-// unusual and corner cases).  We'll construct corresponding Rdata
-// object from this, and compare its wire format data both generated
-// by normal libdns++ interface and via encoding conversion.
+// This test data consist of (almost) all supported types of RDATA (+ some
+// unusual and corner cases).
 const TestRdata test_rdata_list[] = {
     {"IN", "A", "192.0.2.1", 0},
     {"IN", "NS", "ns.example.com", 0},
@@ -71,11 +76,7 @@ const TestRdata test_rdata_list[] = {
     {"IN", "DNAME", "dname.example.com", 0},
     {"IN", "DS", "12892 5 2 5F0EB5C777586DE18DA6B5", 1},
     {"IN", "SSHFP", "1 1 dd465c09cfa51fb45020cc83316fff", 1},
-    // Note: in our implementation RRSIG is treated as opaque data (including
-    // the signer name).  We use "com" for signer so it won't be a compress
-    // target in the test.
-    {"IN", "RRSIG", "SOA 5 2 3600 20120814220826 20120715220826 12345 "
-     "com. FAKEFAKEFAKE", 1},
+    // We handle RRSIG separately, so it's excluded from the list
     {"IN", "NSEC", "next.example.com. A AAAA NSEC RRSIG", 1},
     {"IN", "DNSKEY", "256 3 5 FAKEFAKE", 1},
     {"IN", "DHCID", "FAKEFAKE", 1},
@@ -110,7 +111,69 @@ renderDataField(MessageRenderer* renderer, const uint8_t* data,
     renderer->writeData(data, data_len);
 }
 
-TEST(RdataFieldSpec, checkData) {
+class RdataEncoderTest : public ::testing::Test {
+protected:
+    RdataEncoderTest() : a_rdata_(createRdata(RRType::A(), RRClass::IN(),
+                                              "192.0.2.53")),
+                         aaaa_rdata_(createRdata(RRType::AAAA(), RRClass::IN(),
+                                                 "2001:db8::53")),
+                         rrsig_rdata_(createRdata(
+                                          RRType::RRSIG(), RRClass::IN(),
+                                          "A 5 2 3600 20120814220826 "
+                                          "20120715220826 12345 com. FAKE"))
+    {}
+
+    // This helper test method constructs encodes the given list of RDATAs
+    // (in rdata_list), and then iterates over the data, rendering the fields
+    // 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.
+    void checkEncode(RRClass rrclass, RRType rrtype,
+                     const vector<ConstRdataPtr>& rdata_list,
+                     size_t expected_varlen_fields,
+                     const vector<ConstRdataPtr>& rrsig_list);
+    // A wraper for RdataEncoder::encode() with buffer overrun check.
+    void encodeWrapper(size_t data_len);
+
+    void addRdataCommon(const vector<ConstRdataPtr>& rrsigs);
+    void addRdataMultiCommon(const vector<ConstRdataPtr>& rrsigs);
+
+    // Some commonly used RDATA
+    const ConstRdataPtr a_rdata_;
+    const ConstRdataPtr aaaa_rdata_;
+    const ConstRdataPtr rrsig_rdata_;
+
+    RdataEncoder encoder_;
+    vector<uint8_t> encoded_data_;
+    MessageRenderer expected_renderer_;
+    MessageRenderer actual_renderer_;
+    vector<ConstRdataPtr> rdata_list_;
+};
+
+
+void
+RdataEncoderTest::encodeWrapper(size_t data_len) {
+    // make sure the data buffer is large enough for the canary
+    encoded_data_.resize(data_len + 2);
+    // set the canary data
+    encoded_data_.at(data_len) = 0xde;
+    encoded_data_.at(data_len + 1) = 0xad;
+    // encode, then check the canary is intact
+    encoder_.encode(&encoded_data_[0], data_len);
+    EXPECT_EQ(0xde, encoded_data_.at(data_len));
+    EXPECT_EQ(0xad, encoded_data_.at(data_len + 1));
+    // shrink the data buffer to the originally expected size (some tests
+    // expect that).  the actual encoded data should be intact.
+    encoded_data_.resize(data_len);
+}
+
+void
+RdataEncoderTest::checkEncode(RRClass rrclass, RRType rrtype,
+                              const vector<ConstRdataPtr>& rdata_list,
+                              size_t expected_varlen_fields,
+                              const vector<ConstRdataPtr>& rrsig_list =
+                              vector<ConstRdataPtr>())
+{
     // 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
     // compressed or not correctly.  The names in the RDATA should basically
@@ -120,10 +183,6 @@ TEST(RdataFieldSpec, checkData) {
     const Name dummy_name("com");
     const Name dummy_name2("example.com");
 
-    MessageRenderer expected_renderer, actual_renderer;
-    vector<uint8_t> encoded_data;
-    vector<uint16_t> varlen_list;
-
     // The set of RR types that require additional section processing.
     // We'll pass it to renderNameField to check the stored attribute matches
     // our expectation.
@@ -131,42 +190,292 @@ TEST(RdataFieldSpec, checkData) {
     need_additionals.insert(RRType::NS());
     need_additionals.insert(RRType::MX());
     need_additionals.insert(RRType::SRV());
+    expected_renderer_.clear();
+    actual_renderer_.clear();
+    encoded_data_.clear();
 
-    for (size_t i = 1; test_rdata_list[i].rrclass != NULL; ++i) {
-        SCOPED_TRACE(string(test_rdata_list[i].rrclass) + "/" +
-                     test_rdata_list[i].rrtype);
+    const bool additional_required =
+        (need_additionals.find(rrtype) != need_additionals.end());
 
-        expected_renderer.clear();
-        actual_renderer.clear();
-        expected_renderer.writeName(dummy_name);
-        actual_renderer.writeName(dummy_name);
-        encoded_data.clear();
-        varlen_list.clear();
+    // Build expected wire-format data
+    expected_renderer_.writeName(dummy_name);
+    BOOST_FOREACH(const ConstRdataPtr& rdata, rdata_list) {
+        rdata->toWire(expected_renderer_);
+    }
+    expected_renderer_.writeName(dummy_name2);
+    BOOST_FOREACH(const ConstRdataPtr& rdata, rrsig_list) {
+        rdata->toWire(expected_renderer_);
+    }
+
+    // Then build wire format data using the encoded data.
+    // 1st dummy name
+    actual_renderer_.writeName(dummy_name);
+
+    // Create encoded data
+    encoder_.start(rrclass, rrtype);
+    BOOST_FOREACH(const ConstRdataPtr& rdata, rdata_list) {
+        encoder_.addRdata(*rdata);
+    }
+    BOOST_FOREACH(const ConstRdataPtr& rdata, rrsig_list) {
+        encoder_.addSIGRdata(*rdata);
+    }
+    encodeWrapper(encoder_.getStorageLength());
+
+    // 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
+    // fields.  This is ugly, but we should be able to eliminate this hack
+    // at #2096.
+    vector<uint16_t> varlen_list;
+    if (expected_varlen_fields > 0) {
+        const size_t varlen_list_size =
+            rdata_list.size() * expected_varlen_fields * sizeof(uint16_t);
+        ASSERT_LE(varlen_list_size, encoded_data_.size());
+        varlen_list.resize(rdata_list.size() * expected_varlen_fields);
+        std::memcpy(&varlen_list[0], &encoded_data_[0], varlen_list_size);
+        encoded_data_.assign(encoded_data_.begin() + varlen_list_size,
+                             encoded_data_.end());
+    }
 
+    // If RRSIGs are given, we need to extract the list of the RRSIG lengths
+    // and adjust encoded_data_ further (this will be unnecessary at #2096,
+    // too).
+    vector<uint16_t> rrsiglen_list;
+    if (rrsig_list.size() > 0) {
+        const size_t rrsig_len_size = rrsig_list.size() * sizeof(uint16_t);
+        ASSERT_LE(rrsig_len_size, encoded_data_.size());
+        rrsiglen_list.resize(rrsig_list.size() * rrsig_len_size);
+        std::memcpy(&rrsiglen_list[0], &encoded_data_[0], rrsig_len_size);
+        encoded_data_.assign(encoded_data_.begin() + rrsig_len_size,
+                             encoded_data_.end());
+    }
+
+    // Create wire-format data from the encoded data
+    foreachRdataField(rrclass, rrtype, rdata_list.size(), encoded_data_,
+                      varlen_list,
+                      boost::bind(renderNameField, &actual_renderer_,
+                                  additional_required, _1, _2),
+                      boost::bind(renderDataField, &actual_renderer_, _1, _2));
+    // 2nd dummy name
+    actual_renderer_.writeName(dummy_name2);
+    // Finally, dump any RRSIGs in wire format.
+    foreachRRSig(encoded_data_, rrsiglen_list,
+                 boost::bind(renderDataField, &actual_renderer_, _1, _2));
+
+    // Two sets of wire-format data should be identical.
+    matchWireData(expected_renderer_.getData(), expected_renderer_.getLength(),
+                  actual_renderer_.getData(), actual_renderer_.getLength());
+}
+
+void
+RdataEncoderTest::addRdataCommon(const vector<ConstRdataPtr>& rrsigs) {
+    // Basic check on the encoded data for (most of) all supported RR types,
+    // in a comprehensive manner.
+    for (size_t i = 0; test_rdata_list[i].rrclass != NULL; ++i) {
+        SCOPED_TRACE(string(test_rdata_list[i].rrclass) + "/" +
+                     test_rdata_list[i].rrtype);
         const RRClass rrclass(test_rdata_list[i].rrclass);
         const RRType rrtype(test_rdata_list[i].rrtype);
         const ConstRdataPtr rdata = createRdata(rrtype, rrclass,
                                                 test_rdata_list[i].rdata);
-        rdata->toWire(expected_renderer);
-        expected_renderer.writeName(dummy_name2);
-
-        const bool additional_required =
-            (need_additionals.find(rrtype) != need_additionals.end());
-
-        encodeRdata(*rdata, rrclass, rrtype, encoded_data, varlen_list);
-        EXPECT_EQ(varlen_list.size(), test_rdata_list[i].n_varlen_fields);
-        foreachRdataField(rrclass, rrtype, encoded_data, varlen_list,
-                          boost::bind(renderNameField, &actual_renderer,
-                                      additional_required, _1, _2),
-                          boost::bind(renderDataField, &actual_renderer,
-                                      _1, _2));
-
-        actual_renderer.writeName(dummy_name2);
-        matchWireData(expected_renderer.getData(),
-                      expected_renderer.getLength(),
-                      actual_renderer.getData(),
-                      actual_renderer.getLength());
+        rdata_list_.clear();
+        rdata_list_.push_back(rdata);
+        checkEncode(rrclass, rrtype, rdata_list_,
+                    test_rdata_list[i].n_varlen_fields, rrsigs);
     }
 }
 
+TEST_F(RdataEncoderTest, addRdata) {
+    vector<ConstRdataPtr> rrsigs;
+    addRdataCommon(rrsigs);     // basic tests without RRSIGs (empty vector)
+
+    // Test with RRSIGs (covered type doesn't always match, but the encoder
+    // doesn't check that)
+    rrsigs.push_back(rrsig_rdata_);
+    addRdataCommon(rrsigs);
+}
+
+void
+RdataEncoderTest::addRdataMultiCommon(const vector<ConstRdataPtr>& rrsigs) {
+    // Similar to addRdata(), but test with multiple RDATAs.
+    // Four different cases are tested: a single fixed-len RDATA (A),
+    // fixed-len data + domain name (MX), variable-len data only (TXT),
+    // variable-len data + domain name (NAPTR).
+    ConstRdataPtr a_rdata2 = createRdata(RRType::A(), RRClass::IN(),
+                                         "192.0.2.54");
+    rdata_list_.clear();
+    rdata_list_.push_back(a_rdata_);
+    rdata_list_.push_back(a_rdata2);
+    checkEncode(RRClass::IN(), RRType::A(), rdata_list_, 0, rrsigs);
+
+    ConstRdataPtr mx_rdata1 = createRdata(RRType::MX(), RRClass::IN(),
+                                          "5 mx1.example.com");
+    ConstRdataPtr mx_rdata2 = createRdata(RRType::MX(), RRClass::IN(),
+                                          "10 mx2.example.com");
+    rdata_list_.clear();
+    rdata_list_.push_back(mx_rdata1);
+    rdata_list_.push_back(mx_rdata2);
+    checkEncode(RRClass::IN(), RRType::MX(), rdata_list_, 0, rrsigs);
+
+    ConstRdataPtr txt_rdata1 = createRdata(RRType::TXT(), RRClass::IN(),
+                                           "foo bar baz");
+    ConstRdataPtr txt_rdata2 = createRdata(RRType::TXT(), RRClass::IN(),
+                                          "another text data");
+    rdata_list_.clear();
+    rdata_list_.push_back(txt_rdata1);
+    rdata_list_.push_back(txt_rdata2);
+    checkEncode(RRClass::IN(), RRType::TXT(), rdata_list_, 1, rrsigs);
+
+    ConstRdataPtr naptr_rdata1 =
+        createRdata(RRType::NAPTR(), RRClass::IN(),
+                    "100 50 \"s\" \"http\" \"\" _http._tcp.example.com");
+    ConstRdataPtr naptr_rdata2 =
+        createRdata(RRType::NAPTR(), RRClass::IN(),
+                    "200 100 \"s\" \"http\" \"\" _http._tcp.example.com");
+    rdata_list_.clear();
+    rdata_list_.push_back(naptr_rdata1);
+    rdata_list_.push_back(naptr_rdata2);
+    checkEncode(RRClass::IN(), RRType::NAPTR(), rdata_list_, 1, rrsigs);
+}
+
+TEST_F(RdataEncoderTest, encodeLargeRdata) {
+    // There should be no reason for a large RDATA to fail in encoding,
+    // but we check such a case explicitly.
+
+    encoded_data_.resize(65535); // max unsigned 16-bit int
+    isc::util::InputBuffer buffer(&encoded_data_[0], encoded_data_.size());
+    const in::DHCID large_dhcid(buffer, encoded_data_.size());
+
+    encoder_.start(RRClass::IN(), RRType::DHCID());
+    encoder_.addRdata(large_dhcid);
+    encodeWrapper(encoder_.getStorageLength());
+
+    // The encoded data should be identical to the original one.
+    ASSERT_LT(sizeof(uint16_t), encoder_.getStorageLength());
+    isc::util::InputBuffer ib(&encoded_data_[2], encoded_data_.size() - 2);
+    const in::DHCID encoded_dhcid(ib, ib.getLength());
+    EXPECT_EQ(0, encoded_dhcid.compare(large_dhcid));
+}
+
+TEST_F(RdataEncoderTest, addRdataMulti) {
+    vector<ConstRdataPtr> rrsigs;
+    addRdataMultiCommon(rrsigs); // test without RRSIGs (empty vector)
+
+    // Tests with two RRSIGs
+    rrsigs.push_back(rrsig_rdata_);
+    rrsigs.push_back(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                 "A 5 2 3600 20120814220826 "
+                                 "20120715220826 54321 com. FAKE"));
+    addRdataMultiCommon(rrsigs);
+}
+
+TEST_F(RdataEncoderTest, badAddRdata) {
+    // Some operations must follow start().
+    EXPECT_THROW(encoder_.addRdata(*a_rdata_), isc::InvalidOperation);
+    EXPECT_THROW(encoder_.getStorageLength(), isc::InvalidOperation);
+    // will allocate space of some arbitrary size (256 bytes)
+    EXPECT_THROW(encodeWrapper(256), isc::InvalidOperation);
+
+    // Bad buffer for encode
+    encoder_.start(RRClass::IN(), RRType::A());
+    encoder_.addRdata(*a_rdata_);
+    const size_t buf_len = encoder_.getStorageLength();
+    // NULL buffer for encode
+    EXPECT_THROW(encoder_.encode(NULL, buf_len), isc::BadValue);
+    // buffer length is too short (we don't use the wrraper because we don't
+    // like to tweak the length arg to encode()).
+    encoded_data_.resize(buf_len - 1);
+    EXPECT_THROW(encoder_.encode(&encoded_data_[0], buf_len - 1),
+                 isc::BadValue);
+
+    // Type of RDATA and the specified RR type don't match.  addRdata() should
+    // detect this inconsistency.
+    encoder_.start(RRClass::IN(), RRType::AAAA());
+    EXPECT_THROW(encoder_.addRdata(*a_rdata_), isc::BadValue);
+
+    // Likewise.
+    encoder_.start(RRClass::IN(), RRType::A());
+    EXPECT_THROW(encoder_.addRdata(*aaaa_rdata_), isc::BadValue);
+
+    // Likewise.  The encoder expects the first name completes the data, and
+    // throws on the second due as an unexpected name field.
+    const ConstRdataPtr rp_rdata =
+        createRdata(RRType::RP(), RRClass::IN(), "a.example. b.example");
+    encoder_.start(RRClass::IN(), RRType::NS());
+    EXPECT_THROW(encoder_.addRdata(*rp_rdata), isc::BadValue);
+
+    // Likewise.  The encoder considers the name data a variable length data
+    // field, and throws on the first name.
+    encoder_.start(RRClass::IN(), RRType::DHCID());
+    EXPECT_THROW(encoder_.addRdata(*rp_rdata), isc::BadValue);
+
+    // Likewise.  The text RDATA (2 bytes) will be treated as MX preference,
+    // and the encoder will still expect to see a domain name.
+    const ConstRdataPtr txt_rdata = createRdata(RRType::TXT(), RRClass::IN(),
+                                                "a");
+    encoder_.start(RRClass::IN(), RRType::MX());
+    EXPECT_THROW(encoder_.addRdata(*txt_rdata), isc::BadValue);
+
+    // Similar to the previous one, but in this case there's no data field
+    // in the spec.
+    encoder_.start(RRClass::IN(), RRType::NS());
+    EXPECT_THROW(encoder_.addRdata(*txt_rdata), isc::BadValue);
+
+    // Likewise.  Inconsistent name compression policy.
+    const ConstRdataPtr ns_rdata =
+        createRdata(RRType::NS(), RRClass::IN(), "ns.example");
+    encoder_.start(RRClass::IN(), RRType::DNAME());
+    EXPECT_THROW(encoder_.addRdata(*ns_rdata), isc::BadValue);
+
+    // Same as the previous one, opposite inconsistency.
+    const ConstRdataPtr dname_rdata =
+        createRdata(RRType::DNAME(), RRClass::IN(), "dname.example");
+    encoder_.start(RRClass::IN(), RRType::NS());
+    EXPECT_THROW(encoder_.addRdata(*dname_rdata), isc::BadValue);
+
+    // RDATA len exceeds the 16-bit range.  Technically not invalid, but
+    // we don't support that (and it's practically useless anyway).
+    encoded_data_.resize(65536); // use encoded_data_ for placeholder
+    isc::util::InputBuffer buffer(&encoded_data_[0], encoded_data_.size());
+    encoder_.start(RRClass::IN(), RRType::DHCID());
+    EXPECT_THROW(encoder_.addRdata(in::DHCID(buffer, encoded_data_.size())),
+                                   RdataEncodingError);
+
+    // RRSIG cannot be used as the main RDATA type (can only be added as
+    // a signature for some other type of RDATAs).
+    EXPECT_THROW(encoder_.start(RRClass::IN(), RRType::RRSIG()),
+                 isc::BadValue);
+}
+
+TEST_F(RdataEncoderTest, addSIGRdataOnly) {
+    // Encoded data that only contain RRSIGs.  Mostly useless, but can happen
+    // (in a partially broken zone) and it's accepted.
+    encoder_.start(RRClass::IN(), RRType::A());
+    encoder_.addSIGRdata(*rrsig_rdata_);
+    encodeWrapper(encoder_.getStorageLength());
+    ASSERT_LT(sizeof(uint16_t), encoder_.getStorageLength());
+
+    // The encoded data should be identical to the given one.
+    isc::util::InputBuffer ib(&encoded_data_[2], encoded_data_.size() - 2);
+    const generic::RRSIG encoded_sig(ib, ib.getLength());
+    EXPECT_EQ(0, encoded_sig.compare(*rrsig_rdata_));
+}
+
+TEST_F(RdataEncoderTest, badAddSIGRdata) {
+    // try adding SIG before start
+    EXPECT_THROW(encoder_.addSIGRdata(*rrsig_rdata_), isc::InvalidOperation);
+
+    // Very big RRSIG.  This implementation rejects it.
+    isc::util::OutputBuffer ob(0);
+    rrsig_rdata_->toWire(ob);
+    // append dummy trailing signature to make it too big
+    vector<uint8_t> dummy_sig(65536 - ob.getLength());
+    ob.writeData(&dummy_sig[0], dummy_sig.size());
+    ASSERT_EQ(65536, ob.getLength());
+
+    isc::util::InputBuffer ib(ob.getData(), ob.getLength());
+    const generic::RRSIG big_sigrdata(ib, ob.getLength());
+    encoder_.start(RRClass::IN(), RRType::A());
+    EXPECT_THROW(encoder_.addSIGRdata(big_sigrdata), RdataEncodingError);
+}
 }