[master] Merge branch 'trac2086'

src/lib/dns/labelsequence.cc

@@ -23,10 +23,38 @@
 namespace isc {
 namespace dns {
 
+LabelSequence::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_)
+{
+    if (data == NULL || offsets == NULL) {
+        isc_throw(BadValue, "Null pointer passed to LabelSequence constructor");
+    }
+    if (offsets_size == 0) {
+        isc_throw(BadValue, "Zero offsets to LabelSequence constructor");
+    }
+    if (offsets_size > Name::MAX_LABELS) {
+        isc_throw(BadValue, "MAX_LABELS exceeded");
+    }
+    for (size_t cur_offset = 0; cur_offset < offsets_size; ++cur_offset) {
+        if (offsets[cur_offset] > Name::MAX_LABELLEN) {
+            isc_throw(BadValue, "MAX_LABEL_LEN exceeded");
+        }
+        if (cur_offset > 0 && offsets[cur_offset] <= offsets[cur_offset - 1]) {
+            isc_throw(BadValue, "Offset smaller than previous offset");
+        }
+    }
+}
+
+
 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 +65,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 +106,83 @@ 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.
+
+    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 +205,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 +228,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:

src/lib/dns/labelsequence.h

@@ -21,24 +21,25 @@
 namespace isc {
 namespace dns {
 
-/// \brief Light-weight Accessor to Name object
+/// \brief Light-weight Accessor to data of Name object
 ///
 /// The purpose of this class is to easily match Names and parts of Names,
 /// without needing to copy the underlying data on each label strip.
 ///
-/// It can only work on existing Name objects, and the Name object MUST
+/// It can only work on existing Name objects, or data as provided by the
+/// Name object or another LabelSequence, and the data or Name MUST
 /// remain in scope during the entire lifetime of its associated
 /// LabelSequence(s).
 ///
 /// Upon creation of a LabelSequence, it records the offsets of the
 /// labels in the wireformat data of the Name. When stripLeft() or
 /// stripRight() is called on the LabelSequence, no changes in the
-/// Name's data occur, but the internal pointers of the
+/// original data occur, but the internal pointers of the
 /// LabelSequence are modified.
 ///
 /// LabelSequences can be compared to other LabelSequences, and their
 /// data can be requested (which then points to part of the original
-/// data of the associated Name object).
+/// data of the original Name object).
 ///
 class LabelSequence {
     // Name calls the private toText(bool) method of LabelSequence.
@@ -53,19 +54,42 @@ 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):
+                                     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.
+    ///
+    /// \exception isc::BadValue if basic checks for the input data, or
+    ///            offsets fails.
+    ///
+    /// \param data The raw data for the domain name, in wire format
+    /// \param offsets The offsets of the labels in the domain name data,
+    ///        as given by a 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);
+
     /// \brief Return the wire-format data for this LabelSequence
     ///
-    /// The data, is returned as a pointer to the original wireformat
-    /// data of the original Name object, and the given len value is
+    /// The data is returned as a pointer to (the part of) the original
+    /// wireformat data, from either the original Name object, or the
+    /// raw data given in the constructor, and the given len value is
     /// set to the number of octets that match this labelsequence.
     ///
     /// \note The data pointed to is only valid if the original Name
-    /// object is still in scope
+    /// object or data is still in scope
     ///
     /// \param len Pointer to a size_t where the length of the data
     ///        will be stored (in number of octets)
@@ -83,7 +107,7 @@ public:
     /// versa.
     ///
     /// \note The data pointed to is only valid if the original Name
-    /// object is still in scope
+    /// object or data is still in scope
     ///
     /// \return The length of the data of the label sequence in octets.
     size_t getDataLength() const;
@@ -125,7 +149,7 @@ public:
     /// \brief Remove labels from the end of this LabelSequence
     ///
     /// \note No actual memory is changed, this operation merely updates the
-    /// internal pointers based on the offsets in the Name object.
+    /// internal pointers based on the offsets originally provided.
     ///
     /// \exception OutOfRange if i is greater than or equal to the number
     ///           of labels currently pointed to by this LabelSequence
@@ -144,13 +168,13 @@ public:
     /// LabelSequence as a string.  The returned string ends with a dot
     /// '.' if the label sequence is absolute.
     ///
-    /// This function assumes the underlying name is in proper
+    /// This function assumes the underlying data is in proper
     /// uncompressed wire format.  If it finds an unexpected label
     /// character including compression pointer, an exception of class
     /// \c BadLabelType will be thrown.  In addition, if resource
     /// allocation for the result string fails, a corresponding standard
     /// exception will be thrown.
-    //
+    ///
     /// \return a string representation of the <code>LabelSequence</code>.
     std::string toText() const;
 
@@ -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_;
 };
@@ -218,7 +244,7 @@ private:
 ///
 /// \param os A \c std::ostream object on which the insertion operation is
 /// performed.
-/// \param name The \c LabelSequence object output by the operation.
+/// \param label_sequence The \c LabelSequence object output by the operation.
 /// \return A reference to the same \c std::ostream object referenced by
 /// parameter \c os after the insertion operation.
 std::ostream&

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);
 }
+
 }
 }

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,
@@ -405,35 +405,6 @@ public:
     /// comparison result.
     NameComparisonResult compare(const Name& other) const;
 
-private:
-    /// \brief Partially compare two <code>Name</code>s.
-    ///
-    /// This method performs a partial comparison of the
-    /// <code>Name</code> and <code>other</code> and returns the result
-    /// in the form of a <code>NameComparisonResult</code> object.
-    ///
-    /// This method can throw the BadValue exception if bad label
-    /// indices are passed.
-    ///
-    /// \param other the right-hand operand to compare against.
-    /// \param first_label the left-most label of <code>Name</code> to
-    /// begin comparing from.
-    /// \param first_label_other the left-most label of
-    /// <code>other</code> to begin comparing from.
-    /// \param last_label the right-most label of <code>Name</code> to
-    /// end comparing at.
-    /// \param last_label_other the right-most label of
-    /// <code>other</code> to end comparing at.
-    /// \param case_sensitive If true, comparison is case-insensitive
-    /// \return a <code>NameComparisonResult</code> 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 +522,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 <code>n</code>
-    /// labels including and following the <code>first</code> label.  
+    /// labels including and following the <code>first</code> label.
     Name split(unsigned int first, unsigned int n) const;
 
     /// \brief Extract a specified super domain name of Name.
@@ -623,7 +594,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 +714,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:

src/lib/dns/tests/labelsequence_unittest.cc

@@ -674,4 +674,79 @@ 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());
+    EXPECT_EQ(2, result.getCommonLabels());
+
+    s1.stripRight(1);
+    s3.stripRight(1);
+
+    result = s1.compare(s3);
+    EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
+              result.getRelation());
+    EXPECT_EQ(1, result.getCommonLabels());
+
+    data[9] = 'f';
+    result = s1.compare(s3);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
+              result.getRelation());
+    EXPECT_EQ(0, result.getCommonLabels());
+}
+
+// Test with some data that exceeds limits (MAX_LABELS and MAX_LABEL_LEN)
+TEST(LabelSequence, badRawConstruction) {
+    uint8_t data[1] = { 0 };
+    uint8_t offsets[1] = { 0 };
+
+    EXPECT_THROW(LabelSequence(NULL, offsets, 1), isc::BadValue);
+    EXPECT_THROW(LabelSequence(data, NULL, 1), isc::BadValue);
+    EXPECT_THROW(LabelSequence(data, offsets, 0), isc::BadValue);
+
+    // exceed MAX_LABELS
+    EXPECT_THROW(LabelSequence(data, offsets, 127), isc::BadValue);
+
+    // exceed MAX_LABEL_LEN
+    uint8_t offsets_toolonglabel[1] = { 64 };
+    EXPECT_THROW(LabelSequence(data, offsets_toolonglabel, 1), isc::BadValue);
+
+    // Add an offset that is lower than the previous offset
+    uint8_t offsets_lower[3] = { 0, 8, 4 };
+    EXPECT_THROW(LabelSequence(data, offsets_lower, 3), isc::BadValue);
+
+    // Add an offset that is equal to the previous offset
+    uint8_t offsets_noincrease[3] = { 0, 8, 8 };
+    EXPECT_THROW(LabelSequence(data, offsets_noincrease, 3), isc::BadValue);
+}
+
 }