[2095] added some basic parameter validations.

src/lib/datasrc/memory/rdata_encoder.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include <dns/name.h>
 #include <dns/labelsequence.h>
 #include <dns/messagerenderer.h>
@@ -389,6 +391,11 @@ RdataEncoder::start(RRClass rrclass, RRType rrtype) {
 
 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();
@@ -397,20 +404,35 @@ RdataEncoder::addRdata(const rdata::Rdata& rdata) {
 
 size_t
 RdataEncoder::getStorageLength() const {
+    if (impl_->encode_spec_ == NULL) {
+        isc_throw(InvalidOperation,
+                  "RdataEncoder::getStorageLength performed before start");
+    }
+
     return (sizeof(uint16_t) * impl_->field_composer_.data_lengths_.size() +
             impl_->field_composer_.getLength());
 }
 
 void
 RdataEncoder::encode(void* buf, size_t buf_len) const {
-    // validation
+    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 uint8_t* const dp_beg = reinterpret_cast<uint8_t*>(buf);
-    uint8_t* dp = reinterpret_cast<uint8_t*>(buf);
+    uint8_t* const dp_beg = reinterpret_cast<uint8_t*>(buf);
+    uint8_t* dp = dp_beg;
     if (!impl_->field_composer_.data_lengths_.empty()) {
         const size_t varlen_fields_len =
             impl_->field_composer_.data_lengths_.size() * sizeof(uint16_t);
-        uint16_t* lenp = reinterpret_cast<uint16_t*>(buf);
+        uint16_t* const lenp = reinterpret_cast<uint16_t*>(buf);
         memcpy(lenp, &impl_->field_composer_.data_lengths_[0],
                varlen_fields_len);
         dp += varlen_fields_len;
@@ -419,6 +441,7 @@ RdataEncoder::encode(void* buf, size_t buf_len) const {
            impl_->field_composer_.getLength());
     dp += impl_->field_composer_.getLength();
 
+    // The validation at the entrance must ensure this
     assert(buf_len >= dp - dp_beg);
 }
 

src/lib/datasrc/memory/rdata_encoder.h

@@ -69,10 +69,20 @@ public:
 
     void start(dns::RRClass rrclass, dns::RRType rrtype);
 
+    /// \brief TBD
+    ///
+    /// \throw InvalidOperation called before start().
     void addRdata(const dns::rdata::Rdata& rdata);
 
+    /// \brief TBD
+    ///
+    /// \throw InvalidOperation called before start().
     size_t getStorageLength() const;
 
+    /// \brief TBD
+    ///
+    /// \throw InvalidOperation called before start().
+    /// \throw BadValue buffer is NULL or it's too short for the encoded data.
     void encode(void* buf, size_t buf_len) const;
 
 private:

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

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include <dns/name.h>
 #include <dns/labelsequence.h>
 #include <dns/messagerenderer.h>
@@ -108,7 +110,9 @@ renderDataField(MessageRenderer* renderer, const uint8_t* data,
 
 class RdataEncoderTest : public ::testing::Test {
 protected:
-    RdataEncoderTest() {}
+    RdataEncoderTest() : a_rdata_(createRdata(RRType::A(), RRClass::IN(),
+                                              "192.0.2.53"))
+    {}
 
     // This helper test method constructs encodes the given list of RDATAs
     // (in rdata_list), and then iterates over the data, rendering the fields
@@ -119,6 +123,7 @@ protected:
                      const vector<ConstRdataPtr>& rdata_list,
                      size_t expected_varlen_fields);
 
+    const ConstRdataPtr a_rdata_;     // commonly used RDATA
     RdataEncoder encoder_;
     vector<uint8_t> encoded_data_;
     MessageRenderer expected_renderer_;
@@ -217,12 +222,10 @@ TEST_F(RdataEncoderTest, addRdataMulti) {
     // 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_rdata1 = createRdata(RRType::A(), RRClass::IN(),
-                                        "192.0.2.53");
     ConstRdataPtr a_rdata2 = createRdata(RRType::A(), RRClass::IN(),
                                          "192.0.2.54");
     rdata_list_.clear();
-    rdata_list_.push_back(a_rdata1);
+    rdata_list_.push_back(a_rdata_);
     rdata_list_.push_back(a_rdata2);
     checkEncode(RRClass::IN(), RRType::A(), rdata_list_, 0);
 
@@ -256,6 +259,28 @@ TEST_F(RdataEncoderTest, addRdataMulti) {
     checkEncode(RRClass::IN(), RRType::NAPTR(), rdata_list_, 1);
 }
 
+TEST_F(RdataEncoderTest, badAddRdata) {
+    // Some operations must follow start().
+    EXPECT_THROW(encoder_.addRdata(*a_rdata_), isc::InvalidOperation);
+    EXPECT_THROW(encoder_.getStorageLength(), isc::InvalidOperation);
+    encoded_data_.resize(256); // allocate space of some arbitrary size
+    EXPECT_THROW(encoder_.encode(&encoded_data_[0], encoded_data_.size()),
+                 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
+    encoded_data_.resize(buf_len - 1);
+    EXPECT_THROW(encoder_.encode(&encoded_data_[0], buf_len - 1),
+                 isc::BadValue);
+    encoded_data_.resize(buf_len + 1);
+    encoder_.encode(&encoded_data_[1], buf_len);
+}
+
 // TODO: add before start
 
 // Note: in our implementation RRSIG is treated as opaque data (including