From: Jelte Jansen Date: Thu, 5 Jul 2012 15:53:43 +0000 (+0200) Subject: [2086] use raw Name data in LabelSequence X-Git-Tag: trac2351_base~188^2~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=50dc1afa945214cf133502ea600c48bfeb363a7d;p=thirdparty%2Fkea.git [2086] use raw Name data in LabelSequence instead of reference to Name - moved actual name comparison code to labelsequence - removed Name reference - replaced by data, offsets, and offset_size values - added 'raw' constructor for LabelSequence --- diff --git a/src/lib/dns/labelsequence.cc b/src/lib/dns/labelsequence.cc index c95c5b0cbe..351d86b45a 100644 --- a/src/lib/dns/labelsequence.cc +++ b/src/lib/dns/labelsequence.cc @@ -26,7 +26,7 @@ namespace dns { const uint8_t* LabelSequence::getData(size_t *len) const { *len = getDataLength(); - return (&name_.ndata_[name_.offsets_[first_label_]]); + return &(data_[offsets_[first_label_]]); } size_t @@ -37,10 +37,10 @@ LabelSequence::getDataLength() const { // the length for the 'previous' label (the root label) plus // one (for the root label zero octet) if (isAbsolute()) { - return (name_.offsets_[last_label_ - 1] - - name_.offsets_[first_label_] + 1); + return (offsets_[last_label_ - 1] - + offsets_[first_label_] + 1); } else { - return (name_.offsets_[last_label_] - name_.offsets_[first_label_]); + return (offsets_[last_label_] - offsets_[first_label_]); } } @@ -78,12 +78,87 @@ LabelSequence::compare(const LabelSequence& other, return (NameComparisonResult(0, 0, NameComparisonResult::NONE)); } - return (name_.compare(other.name_, - first_label_, - other.first_label_, - last_label_, - other.last_label_, - case_sensitive)); + // Determine the relative ordering under the DNSSEC order relation of + // 'this' and 'other', and also determine the hierarchical relationship + // of the names. + + if ((first_label_ > last_label_) || + (other.first_label_ > other.last_label_)) { + isc_throw(BadValue, "Bad label index ranges were passed"); + } + + unsigned int nlabels = 0; + int l1 = last_label_ - first_label_; + int l2 = other.last_label_ - other.first_label_; + int ldiff = (int)l1 - (int)l2; + unsigned int l = (ldiff < 0) ? l1 : l2; + + while (l > 0) { + --l; + --l1; + --l2; + size_t pos1 = offsets_[l1 + first_label_]; + size_t pos2 = other.offsets_[l2 + other.first_label_]; + unsigned int count1 = data_[pos1++]; + unsigned int count2 = other.data_[pos2++]; + + // We don't support any extended label types including now-obsolete + // bitstring labels. + assert(count1 <= Name::MAX_LABELLEN && count2 <= Name::MAX_LABELLEN); + + int cdiff = (int)count1 - (int)count2; + unsigned int count = (cdiff < 0) ? count1 : count2; + + while (count > 0) { + uint8_t label1 = data_[pos1]; + uint8_t label2 = other.data_[pos2]; + int chdiff; + + if (case_sensitive) { + chdiff = (int)label1 - (int)label2; + } else { + chdiff = (int)isc::dns::name::internal::maptolower[label1] - (int)isc::dns::name::internal::maptolower[label2]; + } + + if (chdiff != 0) { + if ((nlabels == 0) && + (!isAbsolute() || + ((last_label_ < getLabelCount()) || + (other.last_label_ < other.getLabelCount())))) { + return (NameComparisonResult(0, 0, + NameComparisonResult::NONE)); + } else { + return (NameComparisonResult(chdiff, nlabels, + NameComparisonResult::COMMONANCESTOR)); + } + } + --count; + ++pos1; + ++pos2; + } + if (cdiff != 0) { + if ((nlabels == 0) && + ((last_label_ < getLabelCount()) || + (other.last_label_ < other.getLabelCount()))) { + return (NameComparisonResult(0, 0, + NameComparisonResult::NONE)); + } else { + return (NameComparisonResult(cdiff, nlabels, + NameComparisonResult::COMMONANCESTOR)); + } + } + ++nlabels; + } + + if (ldiff < 0) { + return (NameComparisonResult(ldiff, nlabels, + NameComparisonResult::SUPERDOMAIN)); + } else if (ldiff > 0) { + return (NameComparisonResult(ldiff, nlabels, + NameComparisonResult::SUBDOMAIN)); + } + + return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL)); } void @@ -106,7 +181,7 @@ LabelSequence::stripRight(size_t i) { bool LabelSequence::isAbsolute() const { - return (last_label_ == name_.offsets_.size()); + return (last_label_ == offsets_size_); } size_t @@ -129,9 +204,9 @@ LabelSequence::getHash(bool case_sensitive) const { std::string LabelSequence::toText(bool omit_final_dot) const { - Name::NameString::const_iterator np = name_.ndata_.begin() + - name_.offsets_[first_label_]; - const Name::NameString::const_iterator np_end = np + getDataLength(); + const uint8_t* np = &data_[offsets_[first_label_]]; + const uint8_t* np_end = np + getDataLength(); + // use for integrity check unsigned int labels = last_label_ - first_label_; // init with an impossible value to catch error cases in the end: diff --git a/src/lib/dns/labelsequence.h b/src/lib/dns/labelsequence.h index 25adc711a2..f2b3c112ef 100644 --- a/src/lib/dns/labelsequence.h +++ b/src/lib/dns/labelsequence.h @@ -53,11 +53,35 @@ public: /// to the labels in the Name object). /// /// \param name The Name to construct a LabelSequence for - LabelSequence(const Name& name): name_(name), + explicit LabelSequence(const Name& name): /*name_(name),*/ + data_(&name.ndata_[0]), + offsets_(&name.offsets_[0]), + offsets_size_(name.offsets_.size()), first_label_(0), last_label_(name.getLabelCount()) {} + /// \brief Constructs a LabelSequence for the given data + /// + /// \note The associated data MUST remain in scope during the lifetime + /// of this LabelSequence, since only the pointers are copied. + /// + /// \note No validation is done on the given data upon construction; + /// use with care. + /// + /// \param data The Name data, in wire format + /// \param offsets The offsets of the labels in the Name, as given + /// by the Name object or another LabelSequence + /// \param offsets_size The size of the offsets data + LabelSequence(const uint8_t* data, + const uint8_t* offsets, + size_t offsets_size) : data_(data), + offsets_(offsets), + offsets_size_(offsets_size), + first_label_(0), + last_label_(offsets_size_) + {} + /// \brief Return the wire-format data for this LabelSequence /// /// The data, is returned as a pointer to the original wireformat @@ -201,7 +225,9 @@ public: bool isAbsolute() const; private: - const Name& name_; + const uint8_t* data_; + const uint8_t* offsets_; + size_t offsets_size_; size_t first_label_; size_t last_label_; }; diff --git a/src/lib/dns/name.cc b/src/lib/dns/name.cc index d7df7bd088..c6f0c23782 100644 --- a/src/lib/dns/name.cc +++ b/src/lib/dns/name.cc @@ -127,7 +127,7 @@ typedef enum { // Unused at this moment. We'll revisit this when we support master file // parser where @ is used to mean an origin name. - ft_at + ft_at } ft_state; } @@ -435,101 +435,7 @@ Name::toText(bool omit_final_dot) const { NameComparisonResult Name::compare(const Name& other) const { - return (compare(other, 0, 0, labelcount_, other.labelcount_)); -} - -NameComparisonResult -Name::compare(const Name& other, - unsigned int first_label, - unsigned int first_label_other, - unsigned int last_label, - unsigned int last_label_other, - bool case_sensitive) const { - // Determine the relative ordering under the DNSSEC order relation of - // 'this' and 'other', and also determine the hierarchical relationship - // of the names. - - if ((first_label > last_label) || - (first_label_other > last_label_other)) { - isc_throw(BadValue, "Bad label index ranges were passed"); - } - - if ((first_label > labelcount_) || - (first_label_other > other.labelcount_)) { - isc_throw(BadValue, "Bad first label indices were passed"); - } - - unsigned int nlabels = 0; - int l1 = last_label - first_label; - int l2 = last_label_other - first_label_other; - int ldiff = (int)l1 - (int)l2; - unsigned int l = (ldiff < 0) ? l1 : l2; - - while (l > 0) { - --l; - --l1; - --l2; - size_t pos1 = offsets_[l1 + first_label]; - size_t pos2 = other.offsets_[l2 + first_label_other]; - unsigned int count1 = ndata_[pos1++]; - unsigned int count2 = other.ndata_[pos2++]; - - // We don't support any extended label types including now-obsolete - // bitstring labels. - assert(count1 <= MAX_LABELLEN && count2 <= MAX_LABELLEN); - - int cdiff = (int)count1 - (int)count2; - unsigned int count = (cdiff < 0) ? count1 : count2; - - while (count > 0) { - uint8_t label1 = ndata_[pos1]; - uint8_t label2 = other.ndata_[pos2]; - int chdiff; - - if (case_sensitive) { - chdiff = (int)label1 - (int)label2; - } else { - chdiff = (int)maptolower[label1] - (int)maptolower[label2]; - } - - if (chdiff != 0) { - if ((nlabels == 0) && - ((last_label < labelcount_) || - (last_label_other < other.labelcount_))) { - return (NameComparisonResult(0, 0, - NameComparisonResult::NONE)); - } else { - return (NameComparisonResult(chdiff, nlabels, - NameComparisonResult::COMMONANCESTOR)); - } - } - --count; - ++pos1; - ++pos2; - } - if (cdiff != 0) { - if ((nlabels == 0) && - ((last_label < labelcount_) || - (last_label_other < other.labelcount_))) { - return (NameComparisonResult(0, 0, - NameComparisonResult::NONE)); - } else { - return (NameComparisonResult(cdiff, nlabels, - NameComparisonResult::COMMONANCESTOR)); - } - } - ++nlabels; - } - - if (ldiff < 0) { - return (NameComparisonResult(ldiff, nlabels, - NameComparisonResult::SUPERDOMAIN)); - } else if (ldiff > 0) { - return (NameComparisonResult(ldiff, nlabels, - NameComparisonResult::SUBDOMAIN)); - } - - return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL)); + return LabelSequence(*this).compare(LabelSequence(other)); } bool @@ -581,7 +487,7 @@ Name::gthan(const Name& other) const { bool Name::isWildcard() const { - return (length_ >= 2 && ndata_[0] == 1 && ndata_[1] == '*'); + return (length_ >= 2 && ndata_[0] == 1 && ndata_[1] == '*'); } Name @@ -729,5 +635,6 @@ operator<<(std::ostream& os, const Name& name) { os << name.toText(); return (os); } + } } diff --git a/src/lib/dns/name.h b/src/lib/dns/name.h index 6cfa5460be..92c9c5fd59 100644 --- a/src/lib/dns/name.h +++ b/src/lib/dns/name.h @@ -137,7 +137,7 @@ public: /// want to distinguish "com" and "com.", and the current definition would /// be more compatible for that purpose. /// If, on the other hand, we finally decide we really don't need that - /// notion, we'll probably reconsider the design here, too. + /// notion, we'll probably reconsider the design here, too. enum NameRelation { SUPERDOMAIN = 0, SUBDOMAIN = 1, @@ -427,12 +427,14 @@ private: /// \param case_sensitive If true, comparison is case-insensitive /// \return a NameComparisonResult object representing the /// comparison result. +/* NameComparisonResult compare(const Name& other, unsigned int first_label, unsigned int first_label_other, unsigned int last_label, unsigned int last_label_other, bool case_sensitive = false) const; +*/ public: /// \brief Return true iff two names are equal. @@ -551,7 +553,7 @@ public: /// \param first The start position (in labels) of the extracted name /// \param n Number of labels of the extracted name /// \return A new Name object based on the Name containing n - /// labels including and following the first label. + /// labels including and following the first label. Name split(unsigned int first, unsigned int n) const; /// \brief Extract a specified super domain name of Name. @@ -623,7 +625,7 @@ public: /// \brief Reverse the labels of a name /// /// This method reverses the labels of a name. For example, if - /// \c this is "www.example.com.", this method will return + /// \c this is "www.example.com.", this method will return /// "com.example.www." (This is useful because DNSSEC sort order /// is equivalent to a lexical sort of label-reversed names.) Name reverse() const; @@ -743,10 +745,11 @@ Name::ROOT_NAME() { /// parameter \c os after the insertion operation. std::ostream& operator<<(std::ostream& os, const Name& name); + } } #endif // __NAME_H -// Local Variables: +// Local Variables: // mode: c++ -// End: +// End: diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc index 3ba1d73359..99810dbe2d 100644 --- a/src/lib/dns/tests/labelsequence_unittest.cc +++ b/src/lib/dns/tests/labelsequence_unittest.cc @@ -674,4 +674,46 @@ TEST_F(LabelSequenceTest, LeftShiftOperator) { oss << ls1; EXPECT_EQ(ls1.toText(), oss.str()); } + +// Test different ways of construction, and see if they compare +TEST(LabelSequence, rawConstruction) { + Name n("example.org"); + + uint8_t data[] = { 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', + 0x03, 'o', 'r', 'g', + 0x00 }; + uint8_t offsets[] = { 0, 8, 12 }; + size_t offsets_size = 3; + + LabelSequence s1(n); + LabelSequence s2(s1); + LabelSequence s3(data, offsets, offsets_size); + + // Assuming equality is transitive, so only comparing 1 to 2 and 1 to 3 + NameComparisonResult result = s1.compare(s2); + EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL, + result.getRelation()); + EXPECT_EQ(0, result.getOrder()); + EXPECT_EQ(3, result.getCommonLabels()); + + result = s1.compare(s3); + EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL, + result.getRelation()); + EXPECT_EQ(0, result.getOrder()); + EXPECT_EQ(3, result.getCommonLabels()); + + // Modify the data and make sure it's not equal anymore + data[2] = 'f'; + result = s1.compare(s3); + EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR, + result.getRelation()); + + s1.stripRight(1); + s2.stripRight(1); + data[9] = 'f'; + result = s1.compare(s3); + EXPECT_EQ(isc::dns::NameComparisonResult::NONE, + result.getRelation()); +} + }