[1602] do include root label

also disallow split to zero length, add isAbsolute(), and update tests

src/lib/dns/labelsequence.cc

@@ -22,9 +22,9 @@ namespace dns {
 LabelSequence::LabelSequence(const Name& name) : name_(name),
                              first_label_(0), offsets_(name.getLabelCount()) {
     offsets_[0] = 0;
-    last_label_ = name.getLabelCount() - 1;
+    last_label_ = name.getLabelCount();
     // Walk through the wire format data and store all offsets
-    for (size_t i = 1; i < offsets_.size(); ++i) {
+    for (size_t i = 1; i < last_label_; ++i) {
         // Each offset is the previous offset plus the length of the
         // label plus 1 (for the label length octet)
         offsets_[i] = offsets_[i - 1] + name.at(offsets_[i - 1]) + 1;
@@ -33,7 +33,16 @@ LabelSequence::LabelSequence(const Name& name) : name_(name),
 
 const char*
 LabelSequence::getData(size_t *len) const {
-    *len = offsets_[last_label_] - offsets_[first_label_];
+    // If the labelsequence is absolute, the current last_label_ falls
+    // out of the vector (since it points to the 'label' after the
+    // root label, which doesn't exist; in that case, return
+    // the length for the 'previous' label (the root label) plus
+    // one (for the root label zero octet)
+    if (isAbsolute()) {
+        *len = offsets_[last_label_ - 1] - offsets_[first_label_] + 1;
+    } else {
+        *len = offsets_[last_label_] - offsets_[first_label_];
+    }
     return (name_.at_p(offsets_[first_label_]));
 }
 
@@ -55,9 +64,9 @@ LabelSequence::equals(const LabelSequence& other, bool case_sensitive) const {
 
 void
 LabelSequence::split(int i) {
-    if (abs(i) > getLabelCount()) {
-        isc_throw(OutOfRange, "Label " << i << " out of range (" <<
-                              getLabelCount() << ")");
+    if (abs(i) >= getLabelCount()) {
+        isc_throw(OutOfRange, "Cannot split to zero or less labels; " << i <<
+                              " (labelcount: " << getLabelCount() << ")");
     }
     if (i > 0) {
         first_label_ += i;
@@ -66,6 +75,10 @@ LabelSequence::split(int i) {
     }
 }
 
+bool
+LabelSequence::isAbsolute() const {
+    return last_label_ == offsets_.size();
+}
+
 } // end namespace dns
 } // end namespace isc
-

src/lib/dns/labelsequence.h

@@ -35,13 +35,6 @@ namespace dns {
 /// the LabelSequence, no changes in the Name's data occur, but the
 /// internal pointers of the LabelSequence are modified.
 ///
-/// \note For consistency reasons, when talking about labels and label
-/// counts, LabelSequence objects never include the root label in their
-/// calculations or return values. Wireformat data resulting from
-/// getData() is never absolute, and the result of a labelCount(),
-/// even if split() has never been called on the LabelSequence, is
-/// always smaller than the labelCount of the original Name object.
-///
 /// 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).
@@ -64,16 +57,11 @@ public:
     /// data of the original Name object, and the given len value is
     /// set to the number of octets that match this labelsequence.
     ///
-    /// \note The data pointed to here is never absolute (i.e. it does
-    /// not include the root label), so if this data is used anywhere
-    /// you probably need to add an empty label (one octet with value
-    /// zero).
-    ///
     /// \note The data pointed to is only valid if the original Name
     /// object is still in scope
     ///
     /// \param len Pointer to a size_t where the length of the data
-    ///        is stored
+    ///        will be stored (in number of octets)
     /// \return Pointer to the wire-format data of this label sequence
     const char* getData(size_t* len) const;
 
@@ -95,18 +83,16 @@ public:
     /// \note No actual memory is changed, this operation merely updates the
     /// internal pointers based on the offsets at creation time.
     ///
-    /// \exeption OutOfRange if abs(i) is greater than the number of labels
-    ///           currently pointed to by this LabelSequence
+    /// \exeption OutOfRange if abs(i) is greater than or equal to the
+    ///           number of labels currently pointed to by this LabelSequence
     ///
     /// \param i When positive, removes i labels from the front of the
     ///        LabelSequence. When negative, removes i labels from the
-    ///        end of it.
+    ///        end of it. When zero, this is a no-op.
     void split(int i);
 
     /// \brief Returns the current number of labels for this LabelSequence
     ///
-    /// \note This count does NOT include the root label
-    ///
     /// \return The number of labels
     size_t getLabelCount() const { return last_label_ - first_label_; }
 
@@ -121,6 +107,11 @@ public:
     /// \return Reference to the original Name object
     const Name& getName() const { return name_; }
 
+    /// \brief Checks whether the label sequence is absolute
+    ///
+    /// \return true if the last label is the root label
+    bool isAbsolute() const;
+
 private:
     const Name& name_;
     size_t first_label_;

src/lib/dns/tests/labelsequence_unittest.cc

@@ -134,39 +134,45 @@ getDataCheck(const char* expected_data, size_t expected_len,
 }
 
 TEST_F(LabelSequenceTest, getData) {
-    getDataCheck("\007example\003org", 12, ls1);
-    getDataCheck("\007example\003com", 12, ls2);
-    getDataCheck("\007example\003org", 12, ls3);
-    getDataCheck("\003foo\003bar\004test\007example", 21, ls4);
-    getDataCheck("\007example\003ORG", 12, ls5);
-    getDataCheck("\007ExAmPlE\003org", 12, ls6);
-    getDataCheck("", 0, ls7);
+    getDataCheck("\007example\003org\000", 13, ls1);
+    getDataCheck("\007example\003com\000", 13, ls2);
+    getDataCheck("\007example\003org\000", 13, ls3);
+    getDataCheck("\003foo\003bar\004test\007example\000", 22, ls4);
+    getDataCheck("\007example\003ORG\000", 13, ls5);
+    getDataCheck("\007ExAmPlE\003org\000", 13, ls6);
+    getDataCheck("\000", 1, ls7);
 };
 
 TEST_F(LabelSequenceTest, split_pos) {
+    EXPECT_TRUE(ls1.equals(ls3));
     ls1.split(0);
-    getDataCheck("\007example\003org", 12, ls1);
+    getDataCheck("\007example\003org\000", 13, ls1);
+    EXPECT_TRUE(ls1.equals(ls3));
     ls1.split(1);
-    getDataCheck("\003org", 4, ls1);
+    getDataCheck("\003org\000", 5, ls1);
+    EXPECT_FALSE(ls1.equals(ls3));
     ls1.split(1);
-    getDataCheck("", 0, ls1);
+    getDataCheck("\000", 1, ls1);
     EXPECT_TRUE(ls1.equals(ls7));
 
     ls2.split(2);
-    getDataCheck("", 0, ls2);
+    getDataCheck("\000", 1, ls2);
     EXPECT_TRUE(ls2.equals(ls7));
 }
 
 TEST_F(LabelSequenceTest, split_neg) {
-    ls1.split(0);
+    EXPECT_TRUE(ls1.equals(ls3));
+    ls1.split(-1);
     getDataCheck("\007example\003org", 12, ls1);
+    EXPECT_FALSE(ls1.equals(ls3));
     ls1.split(-1);
     getDataCheck("\007example", 8, ls1);
-    ls1.split(-1);
-    EXPECT_TRUE(ls1.equals(ls7));
+    EXPECT_FALSE(ls1.equals(ls3));
 
+    ASSERT_FALSE(ls1.equals(ls2));
     ls2.split(-2);
-    EXPECT_TRUE(ls2.equals(ls7));
+    getDataCheck("\007example", 8, ls2);
+    EXPECT_TRUE(ls1.equals(ls2));
 }
 
 TEST_F(LabelSequenceTest, split_oor) {
@@ -174,44 +180,74 @@ TEST_F(LabelSequenceTest, split_oor) {
     EXPECT_THROW(ls1.split(5), isc::OutOfRange);
     EXPECT_THROW(ls1.split(4), isc::OutOfRange);
     EXPECT_THROW(ls1.split(3), isc::OutOfRange);
-    getDataCheck("\007example\003org", 12, ls1);
+    getDataCheck("\007example\003org\000", 13, ls1);
 
     EXPECT_THROW(ls1.split(-100), isc::OutOfRange);
     EXPECT_THROW(ls1.split(-5), isc::OutOfRange);
     EXPECT_THROW(ls1.split(-4), isc::OutOfRange);
     EXPECT_THROW(ls1.split(-3), isc::OutOfRange);
-    getDataCheck("\007example\003org", 12, ls1);
+    getDataCheck("\007example\003org\000", 13, ls1);
 }
 
 TEST_F(LabelSequenceTest, getLabelCount) {
-    EXPECT_EQ(2, ls1.getLabelCount());
+    EXPECT_EQ(3, ls1.getLabelCount());
     ls1.split(0);
+    EXPECT_EQ(3, ls1.getLabelCount());
+    ls1.split(1);
     EXPECT_EQ(2, ls1.getLabelCount());
     ls1.split(1);
     EXPECT_EQ(1, ls1.getLabelCount());
-    ls1.split(1);
-    EXPECT_EQ(0, ls1.getLabelCount());
 
+    EXPECT_EQ(3, ls2.getLabelCount());
+    ls2.split(-1);
     EXPECT_EQ(2, ls2.getLabelCount());
-    ls2.split(2);
-    EXPECT_EQ(0, ls2.getLabelCount());
+    ls2.split(-1);
+    EXPECT_EQ(1, ls2.getLabelCount());
 
-    EXPECT_EQ(2, ls3.getLabelCount());
-    ls3.split(-1);
+    EXPECT_EQ(3, ls3.getLabelCount());
+    ls3.split(-2);
     EXPECT_EQ(1, ls3.getLabelCount());
-    ls3.split(-1);
-    EXPECT_EQ(0, ls3.getLabelCount());
 
-    EXPECT_EQ(4, ls4.getLabelCount());
+    EXPECT_EQ(5, ls4.getLabelCount());
     ls4.split(-3);
-    EXPECT_EQ(1, ls4.getLabelCount());
+    EXPECT_EQ(2, ls4.getLabelCount());
+
+    EXPECT_EQ(3, ls5.getLabelCount());
+    ls5.split(2);
+    EXPECT_EQ(1, ls5.getLabelCount());
 }
 
 TEST_F(LabelSequenceTest, comparePart) {
+    EXPECT_FALSE(ls1.equals(ls8));
+
+    // Split root label from example.org.
+    ls1.split(-1);
+    // Split foo from foo.example.org.bar.
+    ls8.split(1);
+    // Split bar. (i.e. bar and root) too
+    ls8.split(-2);
+
+    EXPECT_TRUE(ls1.equals(ls8));
+
+    // Data comparison
     size_t len;
     const char* data = ls1.getData(&len);
-    ls8.split(1);
-    ls8.split(-1);
     getDataCheck(data, len, ls8);
 }
 
+TEST_F(LabelSequenceTest, isAbsolute) {
+    ASSERT_TRUE(ls1.isAbsolute());
+
+    ls1.split(1);
+    ASSERT_TRUE(ls1.isAbsolute());
+    ls1.split(-1);
+    ASSERT_FALSE(ls1.isAbsolute());
+
+    ASSERT_TRUE(ls2.isAbsolute());
+    ls2.split(-1);
+    ASSERT_FALSE(ls2.isAbsolute());
+
+    ASSERT_TRUE(ls3.isAbsolute());
+    ls3.split(2);
+    ASSERT_TRUE(ls3.isAbsolute());
+}