BIND 10 trac2148, updated. a47ad9895154bc82744dc4830e17b519e4b7bb3b [2148] Create test fixture for common names and buf
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jul 25 16:40:55 UTC 2012
The branch, trac2148 has been updated
via a47ad9895154bc82744dc4830e17b519e4b7bb3b (commit)
via 39e4107227eb596a78d03094245ba09a1a66250b (commit)
via 3a5fcdca4afbd234efd5c7bad28b3ac1dc306a43 (commit)
from 6aa8ff8e9517f58b6c768cddad1823257e288f99 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit a47ad9895154bc82744dc4830e17b519e4b7bb3b
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Jul 25 18:39:55 2012 +0200
[2148] Create test fixture for common names and buf
(not for actual labelsequences, IMO it looks better if they are defined locally)
commit 39e4107227eb596a78d03094245ba09a1a66250b
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Jul 25 18:15:51 2012 +0200
[2148] cleanup and documentation
commit 3a5fcdca4afbd234efd5c7bad28b3ac1dc306a43
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Jul 25 18:05:02 2012 +0200
[2148] extend() improvements and more tests
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/labelsequence.cc | 60 +++++++----
src/lib/dns/labelsequence.h | 13 ++-
src/lib/dns/tests/labelsequence_unittest.cc | 156 ++++++++++++++++++---------
3 files changed, 156 insertions(+), 73 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/labelsequence.cc b/src/lib/dns/labelsequence.cc
index 55c4de4..13cbaf3 100644
--- a/src/lib/dns/labelsequence.cc
+++ b/src/lib/dns/labelsequence.cc
@@ -341,39 +341,57 @@ void
LabelSequence::extend(const LabelSequence& labels,
uint8_t buf[MAX_SERIALIZED_LENGTH])
{
- // check whether this labelsequence appears to have anything to do with
- // the given buf at all
+ // collect data to perform steps before anything is changed
+ bool absolute = isAbsolute();
+ size_t label_count = last_label_ + 1;
+ // Since we may have been stripped, do not use getDataLength(), but
+ // calculate actual data size this labelsequence currently uses
+ size_t data_pos = offsets_[last_label_] + data_[offsets_[last_label_]] + 1;
+
+ // If this labelsequence is absolute, virtually strip the root label.
+ if (absolute) {
+ data_pos--;
+ label_count--;
+ }
+ size_t append_label_count = labels.getLabelCount();
+ size_t data_len;
+ const uint8_t *data = labels.getData(&data_len);
+
+ // Sanity checks
if (data_ != buf || offsets_ != &buf[Name::MAX_WIRE]) {
isc_throw(BadValue,
"extend() called with unrelated buffer");
}
-
- // check name does not become too long
- size_t orig_len = getDataLength() - 1;
- if (orig_len + labels.getDataLength() > Name::MAX_WIRE) {
+ if (data_pos + data_len > Name::MAX_WIRE) {
isc_throw(BadValue,
"extend() would exceed maximum wire length");
}
-
- // check offsets data does not become too long
- if (getLabelCount() + labels.getLabelCount() > Name::MAX_LABELS) {
+ if (label_count + append_label_count > Name::MAX_LABELS) {
isc_throw(BadValue,
"extend() would exceed maximum number of labels");
}
- // append second to first labelsequence
- size_t data_len;
- const uint8_t *data = labels.getData(&data_len);
- memcpy(buf + orig_len, data, data_len);
-
- // append offsets
- for (size_t i = 0; i < labels.getLabelCount(); ++i) {
- buf[Name::MAX_WIRE + last_label_ + i] =
- offsets_[last_label_] +
- labels.offsets_[i + labels.first_label_] -
- labels.offsets_[labels.first_label_];
+ // All seems to be reasonably ok, let's proceed.
+
+ // Note: In theory this could be done with a straightforward memcpy.
+ // However, one can extend a labelsequence with itself, in which
+ // case we'd be copying overlapping data (overwriting the current last
+ // label if this LabelSequence is absolute). Therefore we do this
+ // manually, and more importantly, backwards.
+ // (note2: obviously this destroys data_len, don't use below,
+ // or reset it)
+ while (--data_len) {
+ buf[data_pos + data_len] = data[data_len];
+ }
+ buf[data_pos + data_len] = data[data_len];
+
+ for (size_t i = 0; i < append_label_count; ++i) {
+ buf[Name::MAX_WIRE + label_count + i] =
+ offsets_[label_count] +
+ labels.offsets_[i + labels.first_label_] -
+ labels.offsets_[labels.first_label_];
}
- last_label_ += labels.last_label_ - labels.first_label_;
+ last_label_ = label_count + append_label_count - 1;
}
std::ostream&
diff --git a/src/lib/dns/labelsequence.h b/src/lib/dns/labelsequence.h
index 3802557..06eb9e0 100644
--- a/src/lib/dns/labelsequence.h
+++ b/src/lib/dns/labelsequence.h
@@ -268,12 +268,20 @@ public:
/// \brief Extend this LabelSequence with the given labelsequence
///
- /// The given labels are added to the name data, and internal data
- /// is updated accordingly.
+ /// The given labels are added to the name data, and internal offset
+ /// data is updated accordingly.
+ ///
/// The data from the given LabelSequence is copied into the buffer
/// associated with this LabelSequence; the appended LabelSequence
/// can be released if it is not needed for other operations anymore.
///
+ /// If this LabelSequence is absolute, its root label will be stripped
+ /// before the given LabelSequence is appended; after extend(),
+ /// this LabelSequence will be absolute if, and only if, the appended
+ /// LabelSequence was. A side-effect of this property is that adding
+ /// the root label to an absolute LabelSequence has no effect (the
+ /// root label is stripped, then added again).
+ ///
/// Some minimal checking is done on the data, but internal integrity
/// is not assumed. Do NOT modify the given buffer except through calls
/// to this method, and do NOT call this method if the buffer is
@@ -335,6 +343,7 @@ public:
/// \return true if the last label is the root label
bool isAbsolute() const;
+ void dump() const;
private:
const uint8_t* data_; // wire-format name data
const uint8_t* offsets_; // an array of offsets in data_ for the labels
diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc
index f619a1c..492efe6 100644
--- a/src/lib/dns/tests/labelsequence_unittest.cc
+++ b/src/lib/dns/tests/labelsequence_unittest.cc
@@ -860,15 +860,40 @@ void stripRightCheck(LabelSequence ls1, LabelSequence ls2, LabelSequence ls3) {
} // end anonymous namespace
+class ExtendableLabelSequenceTest : public ::testing::Test {
+public:
+ ExtendableLabelSequenceTest() : bar("bar."),
+ example_org("example.org"),
+ foo("foo."),
+ foo_bar("foo.bar."),
+ foo_bar_example_org("foo.bar.example.org."),
+ foo_bar_foo_bar("foo.bar.foo.bar."),
+ foo_example("foo.example."),
+ org("org")
+ {
+ // explicitely set to non-zero data, to make sure
+ // we don't try to use data we don't set
+ memset(buf, 0xff, LabelSequence::MAX_SERIALIZED_LENGTH);
+ }
+
+ Name bar;
+ Name example_org;
+ Name foo;
+ Name foo_bar;
+ Name foo_bar_example_org;
+ Name foo_bar_foo_bar;
+ Name foo_example;
+ Name org;
+
+ uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+};
+
// Test that 'extendable' labelsequences behave correctly when using
// stripLeft() and stripRight()
-TEST(LabelSequence, extendableLabelSequence) {
- Name n1("example.org.");
- LabelSequence ls1(n1);
- LabelSequence ls2(n1);
+TEST_F(ExtendableLabelSequenceTest, extendableLabelSequence) {
+ LabelSequence ls1(example_org);
+ LabelSequence ls2(example_org);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls1, buf);
ASSERT_EQ(ls1.getDataLength(), els.getDataLength());
@@ -878,17 +903,14 @@ TEST(LabelSequence, extendableLabelSequence) {
// Test that 'extendable' LabelSequences behave correctly when initialized
// with a stripped source LabelSequence
-TEST(LabelSequence, extendableLabelSequenceStrippedSource) {
- Name n1("foo.bar.example.org.");
- LabelSequence ls1(n1);
- LabelSequence ls2(n1);
+TEST_F(ExtendableLabelSequenceTest, extendableLabelSequenceStrippedSource) {
+ LabelSequence ls1(foo_bar_example_org);
+ LabelSequence ls2(foo_bar_example_org);
while (ls1.getLabelCount() > 2) {
ls1.stripLeft(1);
ls2.stripLeft(1);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls1, buf);
ASSERT_EQ(ls1.getDataLength(), els.getDataLength());
@@ -897,17 +919,14 @@ TEST(LabelSequence, extendableLabelSequenceStrippedSource) {
}
}
-TEST(LabelSequence, extendableLabelSequenceRightStrippedSource) {
- Name n1("foo.bar.example.org.");
- LabelSequence ls1(n1);
- LabelSequence ls2(n1);
+TEST_F(ExtendableLabelSequenceTest, extendableLabelSequenceRightStrippedSource) {
+ LabelSequence ls1(foo_bar_example_org);
+ LabelSequence ls2(foo_bar_example_org);
while (ls1.getLabelCount() > 2) {
ls1.stripRight(1);
ls2.stripRight(1);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls1, buf);
ASSERT_EQ(ls1.getDataLength(), els.getDataLength());
@@ -917,67 +936,104 @@ TEST(LabelSequence, extendableLabelSequenceRightStrippedSource) {
}
// Check some basic 'extend' functionality
-TEST(LabelSequence, extend) {
- Name n1("foo.bar.");
- Name n2("foo");
- Name n3("bar");
- LabelSequence ls1(n1);
- LabelSequence ls2(n2);
- LabelSequence ls3(n3);
- LabelSequence ls4(n1);
+TEST_F(ExtendableLabelSequenceTest, extend) {
+ LabelSequence ls1(foo_bar);
+ LabelSequence ls2(foo);
+ LabelSequence ls3(bar);
+ LabelSequence ls4(foo_bar);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls2, buf);
check_compare(ls1, els, isc::dns::NameComparisonResult::COMMONANCESTOR, 1);
els.extend(ls3, buf);
- check_compare(ls1, els, isc::dns::NameComparisonResult::EQUAL, 3);
+ check_equal(ls1, els);
stripLeftCheck(ls1, els, ls4);
stripRightCheck(ls1, els, ls4);
+ // strip, then extend again
+ els.stripRight(2); // (2, 1 for root label, 1 for last label)
+ els.extend(ls3, buf);
+ check_equal(ls1, els);
+
+ // Extending again should make it different
els.extend(ls3, buf);
check_compare(ls1, ls2, isc::dns::NameComparisonResult::COMMONANCESTOR, 1);
+
+ // Extending with a non-absolute name should make it non-absolute as well
+ ls3.stripRight(1);
+ els.extend(ls3, buf);
+ EXPECT_FALSE(els.isAbsolute());
+
+ // And try extending when both are not absolute
+ els.stripRight(3);
+ ls1.stripRight(1);
+ EXPECT_FALSE(els.isAbsolute());
+ els.extend(ls3, buf);
+ check_equal(ls1, els);
+}
+
+TEST_F(ExtendableLabelSequenceTest, extendLeftStripped) {
+ LabelSequence ls1(foo_example);
+ LabelSequence ls2(example_org);
+ LabelSequence ls3(org);
+
+ LabelSequence els(ls1, buf);
+
+ els.stripLeft(1);
+ els.extend(ls3, buf);
+ check_equal(ls2, els);
}
// Check that when extending with itself, it does not cause horrible failures
-TEST(LabelSequence, extendWithItself) {
- Name n1("foo.bar.");
- Name n2("foo.bar.foo.bar.");
- LabelSequence ls1(n1);
- LabelSequence ls2(n2);
+TEST_F(ExtendableLabelSequenceTest, extendWithItself) {
+ LabelSequence ls1(foo_bar);
+ LabelSequence ls2(foo_bar_foo_bar);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls1, buf);
- std::cout << "[XX] " << els.toText() << std::endl;
- // some men just want to watch the world burn.
els.extend(els, buf);
- std::cout << "[XX] " << els.toText() << std::endl;
+ check_equal(ls2, els);
+
+ // Also try for non-absolute names
+ ls2.stripRight(1);
+ els = LabelSequence(ls1, buf);
+ els.stripRight(1);
+ els.extend(els, buf);
+ check_equal(ls2, els);
+
+ // Once more, now start out with non-absolue
+ ls1.stripRight(1);
+ els = LabelSequence(ls1, buf);
+ els.extend(els, buf);
check_equal(ls2, els);
}
-// Test that 'extending' with just a root label is a no-op
-TEST(LabelSequence, extendWithRoot) {
- Name n1("example.org");
- LabelSequence ls1(n1);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+// Test that 'extending' with just a root label is a no-op, iff the original
+// was already absolute
+TEST_F(ExtendableLabelSequenceTest, extendWithRoot) {
+ LabelSequence ls1(example_org);
LabelSequence els(LabelSequence(ls1, buf));
check_equal(ls1, els);
els.extend(LabelSequence(Name(".")), buf);
check_equal(ls1, els);
+
+ // but not if the original was not absolute (it will be equal to
+ // the original labelsequence used above, but not the one it was based
+ // on).
+ LabelSequence ls2(example_org);
+ ls2.stripRight(1);
+ els = LabelSequence(ls2, buf);
+ els.extend(LabelSequence(Name(".")), buf);
+ check_equal(ls1, els);
+ check_compare(ls2, els, isc::dns::NameComparisonResult::NONE, 0);
}
// Check possible failure modes of extend()
-TEST(LabelSequence, extendBadData) {
- Name n1("example.org.");
- LabelSequence ls1(n1);
+TEST_F(ExtendableLabelSequenceTest, extendBadData) {
+ LabelSequence ls1(example_org);
- uint8_t buf[LabelSequence::MAX_SERIALIZED_LENGTH];
- memset(buf, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
LabelSequence els(ls1, buf);
// try use with unrelated labelsequence
@@ -1001,7 +1057,7 @@ TEST(LabelSequence, extendBadData) {
Name shortname("1.");
LabelSequence short_ls(shortname);
els = LabelSequence(short_ls, buf);
- for (size_t i=0; i < 125; ++i) {
+ for (size_t i=0; i < 126; ++i) {
els.extend(short_ls, buf);
}
EXPECT_THROW(els.extend(short_ls, buf), isc::BadValue);
More information about the bind10-changes
mailing list