From: JINMEI Tatuya Date: Wed, 25 Jul 2012 22:17:17 +0000 (-0700) Subject: [2095] handled various error cases X-Git-Tag: trac2351_base~137^2~4^2~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d3ada48eec11ef1c25f776ab1f2598ea245a8367;p=thirdparty%2Fkea.git [2095] handled various error cases --- diff --git a/src/lib/datasrc/memory/rdata_encoder.cc b/src/lib/datasrc/memory/rdata_encoder.cc index 0ce42ffe52..ed7f099c51 100644 --- a/src/lib/datasrc/memory/rdata_encoder.cc +++ b/src/lib/datasrc/memory/rdata_encoder.cc @@ -286,14 +286,31 @@ public: virtual void setTruncated() {} virtual void setLengthLimit(size_t) {} virtual void setCompressMode(CompressMode) {} - virtual void writeName(const Name& name, bool /*compress*/) { - assert(current_field_ < encode_spec_->field_count); // TBD - + virtual void writeName(const Name& name, bool compress) { + if (current_field_ >= encode_spec_->field_count) { + isc_throw(BadValue, + "RDATA encoder encounters an unexpected name data: " << + name); + } extendData(); + if (current_field_ >= encode_spec_->field_count) { + isc_throw(BadValue, + "RDATA encoder encounters an unexpected name data: " << + name); + } + // At this point it's ensured we still have a field in the spec, + // and it's a domain name field. const RdataFieldSpec& field = encode_spec_->fields[current_field_++]; - assert(field.type == RdataFieldSpec::DOMAIN_NAME); // TBD + 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_)); @@ -314,10 +331,14 @@ public: current_field_ = 0; } void endRdata() { + // Handle any remaining data (there should be no more name). Then + // we should reach the end of the fields. if (current_field_ < encode_spec_->field_count) { extendData(); } - assert(current_field_ == encode_spec_->field_count); // TBD + if (current_field_ != encode_spec_->field_count) { + isc_throw(BadValue, "RDATA encoder finds missing field"); + } } vector > data_positions_; vector > names_; @@ -340,18 +361,32 @@ private: } ++current_field_; if (field.type == RdataFieldSpec::FIXEDLEN_DATA) { - // TBD: validation + if (data_len < field.fixeddata_len) { + isc_throw(BadValue, + "RDATA encoding: available data too short " + "for the type"); + } data_len -= field.fixeddata_len; continue; } - // We are looking at a variable-length data field. - // TBD: 16bit len check + // We are looking at a variable-length data field. For encoding + // purposes, it's 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); data_len = 0; break; } - // TBD: data_len must be 0; - assert(data_len == 0); + + // We've reached the end of the RDATA or just consumed a variable + // length data field. So we should have eaten all available data + // at this moment. + if (data_len != 0) { + isc_throw(BadValue, "redundant data for encoding in RDATA"); + } // We added this much data from last time data_positions_.push_back( diff --git a/src/lib/datasrc/memory/rdata_encoder.h b/src/lib/datasrc/memory/rdata_encoder.h index b63d9d38ce..74f1c4b9c8 100644 --- a/src/lib/datasrc/memory/rdata_encoder.h +++ b/src/lib/datasrc/memory/rdata_encoder.h @@ -15,6 +15,8 @@ #ifndef DATASRC_MEMORY_RDATA_ENCODER_H #define DATASRC_MEMORY_RDATA_ENCODER_H 1 +#include + #include #include #include @@ -29,6 +31,17 @@ 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 @@ -71,7 +84,16 @@ public: /// \brief TBD /// + /// 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 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. + /// /// \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. void addRdata(const dns::rdata::Rdata& rdata); /// \brief TBD diff --git a/src/lib/datasrc/memory/tests/rdata_encoder_unittest.cc b/src/lib/datasrc/memory/tests/rdata_encoder_unittest.cc index 34e0a78e7a..49a5b9f7c6 100644 --- a/src/lib/datasrc/memory/tests/rdata_encoder_unittest.cc +++ b/src/lib/datasrc/memory/tests/rdata_encoder_unittest.cc @@ -14,10 +14,13 @@ #include +#include + #include #include #include #include +#include #include #include @@ -111,7 +114,9 @@ renderDataField(MessageRenderer* renderer, const uint8_t* data, class RdataEncoderTest : public ::testing::Test { protected: RdataEncoderTest() : a_rdata_(createRdata(RRType::A(), RRClass::IN(), - "192.0.2.53")) + "192.0.2.53")), + aaaa_rdata_(createRdata(RRType::AAAA(), RRClass::IN(), + "2001:db8::53")) {} // This helper test method constructs encodes the given list of RDATAs @@ -124,6 +129,7 @@ protected: size_t expected_varlen_fields); const ConstRdataPtr a_rdata_; // commonly used RDATA + const ConstRdataPtr aaaa_rdata_; // commonly used RDATA RdataEncoder encoder_; vector encoded_data_; MessageRenderer expected_renderer_; @@ -279,9 +285,55 @@ TEST_F(RdataEncoderTest, badAddRdata) { isc::BadValue); encoded_data_.resize(buf_len + 1); encoder_.encode(&encoded_data_[1], buf_len); -} -// TODO: add before start + // 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); + + // 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); +} // 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