Merge #2383

Add a new constructor to name, one that takes an optional origin and completes
relative names with it.

Also clarify the '@' handling as a side effect.

src/lib/dns/name.cc

@@ -123,15 +123,18 @@ typedef enum {
     ft_ordinary,                // parsing an ordinary label
     ft_initialescape,           // just found '\'
     ft_escape,                  // begin of handling a '\'-escaped sequence
-    ft_escdecimal,              // parsing a '\DDD' octet.
-
-    // 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_escdecimal               // parsing a '\DDD' octet.
 } ft_state;
-}
 
-Name::Name(const std::string &namestring, bool downcase) {
+// The parser of name from a string. It is a template, because
+// some parameters are used with two different types, while others
+// are private type aliases.
+template<class Iterator, class Offsets, class Data>
+void
+stringParse(Iterator s, Iterator send, bool downcase, Offsets& offsets,
+            Data& ndata)
+{
+    const Iterator orig_s(s);
     //
     // Initialize things to make the compiler happy; they're not required.
     //
@@ -142,17 +145,14 @@ Name::Name(const std::string &namestring, bool downcase) {
     //
     // Set up the state machine.
     //
-    std::string::const_iterator s = namestring.begin();
-    std::string::const_iterator send = namestring.end();
     bool done = false;
     bool is_root = false;
+    const bool empty = s == send;
     ft_state state = ft_init;
 
-    NameOffsets offsets;
+    // Prepare the output buffers.
     offsets.reserve(Name::MAX_LABELS);
     offsets.push_back(0);
-
-    NameString ndata;
     ndata.reserve(Name::MAX_WIRE);
 
     // should we refactor this code using, e.g, the state pattern?  Probably
@@ -171,7 +171,8 @@ Name::Name(const std::string &namestring, bool downcase) {
             if (c == '.') {
                 if (s != send) {
                     isc_throw(EmptyLabel,
-                              "non terminating empty label in " << namestring);
+                              "non terminating empty label in " <<
+                              string(orig_s, send));
                 }
                 is_root = true;
             } else if (c == '@' && s == send) {
@@ -200,7 +201,7 @@ Name::Name(const std::string &namestring, bool downcase) {
             if (c == '.') {
                 if (count == 0) {
                     isc_throw(EmptyLabel,
-                              "duplicate period in " << namestring);
+                              "duplicate period in " << string(orig_s, send));
                 }
                 ndata.at(offsets.back()) = count;
                 offsets.push_back(ndata.size());
@@ -212,9 +213,9 @@ Name::Name(const std::string &namestring, bool downcase) {
             } else if (c == '\\') {
                 state = ft_escape;
             } else {
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
-                              "label is too long in " << namestring);
+                              "label is too long in " << string(orig_s, send));
                 }
                 ndata.push_back(downcase ? maptolower[c] : c);
             }
@@ -224,15 +225,15 @@ Name::Name(const std::string &namestring, bool downcase) {
                 // This looks like a bitstring label, which was deprecated.
                 // Intentionally drop it.
                 isc_throw(BadLabelType,
-                          "invalid label type in " << namestring);
+                          "invalid label type in " << string(orig_s, send));
             }
             state = ft_escape;
             // FALLTHROUGH
         case ft_escape:
             if (!isdigit(c & 0xff)) {
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
-                              "label is too long in " << namestring);
+                              "label is too long in " << string(orig_s, send));
                 }
                 ndata.push_back(downcase ? maptolower[c] : c);
                 state = ft_ordinary;
@@ -246,7 +247,7 @@ Name::Name(const std::string &namestring, bool downcase) {
             if (!isdigit(c & 0xff)) {
                 isc_throw(BadEscape,
                           "mixture of escaped digit and non-digit in "
-                          << namestring);
+                          << string(orig_s, send));
             }
             value *= 10;
             value += digitvalue[c];
@@ -255,11 +256,11 @@ Name::Name(const std::string &namestring, bool downcase) {
                 if (value > 255) {
                     isc_throw(BadEscape,
                               "escaped decimal is too large in "
-                              << namestring);
+                              << string(orig_s, send));
                 }
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
-                              "label is too long in " << namestring);
+                              "label is too long in " << string(orig_s, send));
                 }
                 ndata.push_back(downcase ? maptolower[value] : value);
                 state = ft_ordinary;
@@ -274,13 +275,14 @@ Name::Name(const std::string &namestring, bool downcase) {
     if (!done) {                // no trailing '.' was found.
         if (ndata.size() == Name::MAX_WIRE) {
             isc_throw(TooLongName,
-                      "name is too long for termination in " << namestring);
+                      "name is too long for termination in " <<
+                      string(orig_s, send));
         }
         assert(s == send);
-        if (state != ft_ordinary && state != ft_at) {
+        if (state != ft_ordinary) {
             isc_throw(IncompleteName,
                       "incomplete textual name in " <<
-                      (namestring.empty() ? "<empty>" : namestring));
+                      (empty ? "<empty>" : string(orig_s, send)));
         }
         if (state == ft_ordinary) {
             assert(count != 0);
@@ -291,12 +293,93 @@ Name::Name(const std::string &namestring, bool downcase) {
             ndata.push_back('\0');
         }
     }
+}
+
+}
+
+Name::Name(const std::string &namestring, bool downcase) {
+    // Prepare inputs for the parser
+    const std::string::const_iterator s = namestring.begin();
+    const std::string::const_iterator send = namestring.end();
+
+    // Prepare outputs
+    NameOffsets offsets;
+    NameString ndata;
+
+    // To the parsing
+    stringParse(s, send, downcase, offsets, ndata);
+
+    // And get the output
+    labelcount_ = offsets.size();
+    assert(labelcount_ > 0 && labelcount_ <= Name::MAX_LABELS);
+    ndata_.assign(ndata.data(), ndata.size());
+    length_ = ndata_.size();
+    offsets_.assign(offsets.begin(), offsets.end());
+}
+
+Name::Name(const char* namedata, size_t data_len, const Name* origin,
+           bool downcase)
+{
+    // Check validity of data
+    if (namedata == NULL || data_len == 0) {
+        isc_throw(isc::InvalidParameter,
+                  "No data provided to Name constructor");
+    }
+    // If the last character is not a dot, it is a relative to origin.
+    // It is safe to check now, we know there's at least one character.
+    const bool absolute = (namedata[data_len - 1] == '.');
+    // If we are not absolute, we need the origin to complete the name.
+    if (!absolute && origin == NULL) {
+        isc_throw(MissingNameOrigin,
+                  "No origin available and name is relative");
+    }
+    // Prepare inputs for the parser
+    const char* end = namedata + data_len;
+
+    // Prepare outputs
+    NameOffsets offsets;
+    NameString ndata;
+
+    // Do the actual parsing
+    stringParse(namedata, end, downcase, offsets, ndata);
 
+    // Get the output
     labelcount_ = offsets.size();
     assert(labelcount_ > 0 && labelcount_ <= Name::MAX_LABELS);
     ndata_.assign(ndata.data(), ndata.size());
     length_ = ndata_.size();
     offsets_.assign(offsets.begin(), offsets.end());
+
+    if (!absolute) {
+        // Now, extend the data with the ones from origin. But eat the
+        // last label (the empty one).
+
+        // Drop the last character of the data (the \0) and append a copy of
+        // the origin's data
+        ndata_.erase(ndata_.end() - 1);
+        ndata_.append(origin->ndata_);
+
+        // Do a similar thing with offsets. However, we need to move them
+        // so they point after the prefix we parsed before.
+        size_t offset = offsets_.back();
+        offsets_.pop_back();
+        size_t offset_count = offsets_.size();
+        offsets_.insert(offsets_.end(), origin->offsets_.begin(),
+                        origin->offsets_.end());
+        for (NameOffsets::iterator it(offsets_.begin() + offset_count);
+             it != offsets_.end(); ++it) {
+            *it += offset;
+        }
+
+        // Adjust sizes.
+        length_ = ndata_.size();
+        labelcount_ = offsets_.size();
+
+        // And check the sizes are OK.
+        if (labelcount_ > Name::MAX_LABELS || length_ > Name::MAX_WIRE) {
+            isc_throw(TooLongName, "Combined name is too long");
+        }
+    }
 }
 
 namespace {

src/lib/dns/name.h

@@ -105,6 +105,17 @@ public:
         NameParserException(file, line, what) {}
 };
 
+/// \brief Thrown when origin is NULL and is needed.
+///
+/// The exception is thrown when the Name constructor for master file
+/// is used, the provided data is relative and the origin parameter is
+/// set to NULL.
+class MissingNameOrigin : public NameParserException {
+public:
+    MissingNameOrigin(const char* file, size_t line, const char* what) :
+        NameParserException(file, line, what) {}
+};
+
 ///
 /// This is a supplemental class used only as a return value of
 /// Name::compare() and LabelSequence::compare().
@@ -261,6 +272,37 @@ public:
     /// \param namestr A string representation of the name to be constructed.
     /// \param downcase Whether to convert upper case alphabets to lower case.
     explicit Name(const std::string& namestr, bool downcase = false);
+
+    /// \brief Constructor for master file parser
+    ///
+    /// This acts similar to the above. But the data is passed as raw C-string
+    /// instead of wrapped-up C++ std::string.
+    ///
+    /// Also, when the origin is non-NULL and the name_data is not ending with
+    /// a dot, it is considered relative and the origin is appended to it.
+    ///
+    /// If the name_data is equal to "@", the content of origin is copied.
+    ///
+    /// \param name_data The raw data of the name.
+    /// \param data_len How many bytes in name_data is valid and considered
+    ///     part of the name.
+    /// \param origin If non-NULL, it is taken as the origin to complete
+    ///     relative names.
+    /// \param downcase Whether to convert upper case letters to lower case.
+    /// \throw NameParserException or any of its descendants in case the
+    ///     input data is invalid.
+    /// \throw isc::InvalidParameter In case name_data is NULL or data_len is
+    ///     0.
+    /// \throw std::bad_alloc In case allocation fails.
+    /// \note This constructor is specially designed for the use of master
+    ///     file parser. It mimics the behaviour of names in the master file
+    ///     and accepts raw data. It is not recommended to be used by anything
+    ///     else.
+    /// \todo Should we make it private and the parser a friend, to hide the
+    ///     constructor?
+    Name(const char* name_data, size_t data_len, const Name* origin,
+         bool downcase = false);
+
     /// Constructor from wire-format %data.
     ///
     /// The \c buffer parameter normally stores a complete DNS message

src/lib/dns/tests/name_unittest.cc

@@ -40,6 +40,22 @@ using namespace isc::util;
 const size_t Name::MAX_WIRE;
 const size_t Name::MAX_LABELS;
 
+// This is a name of maximum allowed number of labels
+const char* max_labels_str = "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 40
+                             "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 80
+                             "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 120
+                             "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 160
+                             "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 200
+                             "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 240
+                             "0.1.2.3.4.5.6";
+// This is a name of maximum allowed length
+const char* max_len_str = "123456789.123456789.123456789.123456789.123456789."
+                          "123456789.123456789.123456789.123456789.123456789."
+                          "123456789.123456789.123456789.123456789.123456789."
+                          "123456789.123456789.123456789.123456789.123456789."
+                          "123456789.123456789.123456789.123456789.123456789."
+                          "123";
+
 namespace {
 class NameTest : public ::testing::Test {
 protected:
@@ -47,6 +63,8 @@ protected:
                  example_name_upper("WWW.EXAMPLE.COM"),
                  small_name("aaa.example.com"),
                  large_name("zzz.example.com"),
+                 origin_name("example.com."),
+                 origin_name_upper("EXAMPLE.COM"),
                  buffer_actual(0), buffer_expected(0)
     {}
 
@@ -54,6 +72,8 @@ protected:
     Name example_name_upper;    // this will be modified and cannot be const
     const Name small_name;
     const Name large_name;
+    const Name origin_name;
+    const Name origin_name_upper;
     OutputBuffer buffer_actual, buffer_expected;
 
     //
@@ -137,6 +157,11 @@ checkBadTextName(const string& txt) {
     // NameParserException.
     EXPECT_THROW(Name(txt, false), ExceptionType);
     EXPECT_THROW(Name(txt, false), NameParserException);
+    // The same is thrown when constructing by the master-file constructor
+    EXPECT_THROW(Name(txt.c_str(), txt.length(), &Name::ROOT_NAME()),
+                 ExceptionType);
+    EXPECT_THROW(Name(txt.c_str(), txt.length(), &Name::ROOT_NAME()),
+                 NameParserException);
 }
 
 TEST_F(NameTest, fromText) {
@@ -201,27 +226,99 @@ TEST_F(NameTest, fromText) {
                                   "123456789.123456789.123456789.123456789."
                                   "123456789.1234");
     // This is a possible longest name and should be accepted
-    EXPECT_NO_THROW(Name("123456789.123456789.123456789.123456789.123456789."
-                         "123456789.123456789.123456789.123456789.123456789."
-                         "123456789.123456789.123456789.123456789.123456789."
-                         "123456789.123456789.123456789.123456789.123456789."
-                         "123456789.123456789.123456789.123456789.123456789."
-                         "123"));
+    EXPECT_NO_THROW(Name(string(max_len_str)));
     // \DDD must consist of 3 digits.
     checkBadTextName<IncompleteName>("\\12");
 
     // a name with the max number of labels.  should be constructed without
     // an error, and its length should be the max value.
-    Name maxlabels = Name("0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 40
-                          "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 80
-                          "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 120
-                          "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 160
-                          "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 200
-                          "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 240
-                          "0.1.2.3.4.5.6.");
+    Name maxlabels = Name(string(max_labels_str));
     EXPECT_EQ(Name::MAX_LABELS, maxlabels.getLabelCount());
 }
 
+// on the rest while we prepare it.
+// Check the @ syntax is accepted and it just copies the origin.
+TEST_F(NameTest, copyOrigin) {
+    EXPECT_EQ(origin_name, Name("@", 1, &origin_name));
+    // The downcase works on the origin too. But only when we provide it.
+    EXPECT_EQ(origin_name, Name("@", 1, &origin_name_upper, true));
+    EXPECT_EQ(origin_name_upper, Name("@", 1, &origin_name_upper, true));
+    // If we don't provide the origin, it throws
+    EXPECT_THROW(Name("@", 1, NULL), MissingNameOrigin);
+}
+
+// Test the master-file constructor does not append the origin when the
+// provided name is absolute
+TEST_F(NameTest, dontAppendOrigin) {
+    EXPECT_EQ(example_name, Name("www.example.com.", 16, &origin_name));
+    // The downcase works (only if provided, though)
+    EXPECT_EQ(example_name, Name("WWW.EXAMPLE.COM.", 16, &origin_name, true));
+    EXPECT_EQ(example_name_upper, Name("WWW.EXAMPLE.COM.", 16, &origin_name));
+    // And it does not require the origin to be provided
+    EXPECT_NO_THROW(Name("www.example.com.", 16, NULL));
+}
+
+// Test the master-file constructor properly appends the origin when
+// the provided name is relative.
+TEST_F(NameTest, appendOrigin) {
+    EXPECT_EQ(example_name, Name("www", 3, &origin_name));
+    // Check the downcase works (if provided)
+    EXPECT_EQ(example_name, Name("WWW", 3, &origin_name, true));
+    EXPECT_EQ(example_name, Name("WWW", 3, &origin_name_upper, true));
+    EXPECT_EQ(example_name_upper, Name("WWW", 3, &origin_name_upper));
+    // Check we can prepend more than one label
+    EXPECT_EQ(Name("a.b.c.d.example.com."), Name("a.b.c.d", 7, &origin_name));
+    // When the name is relative, we throw.
+    EXPECT_THROW(Name("www", 3, NULL), MissingNameOrigin);
+}
+
+// When we don't provide the data, it throws
+TEST_F(NameTest, noDataProvided) {
+    EXPECT_THROW(Name(NULL, 10, NULL), isc::InvalidParameter);
+    EXPECT_THROW(Name(NULL, 10, &origin_name), isc::InvalidParameter);
+    EXPECT_THROW(Name("www", 0, NULL), isc::InvalidParameter);
+    EXPECT_THROW(Name("www", 0, &origin_name), isc::InvalidParameter);
+}
+
+// When we combine the first part and the origin together, the resulting name
+// is too long. It should throw. Other test checks this is valid when alone
+// (without the origin appended).
+TEST_F(NameTest, combinedTooLong) {
+    EXPECT_THROW(Name(max_len_str, strlen(max_len_str), &origin_name),
+                 TooLongName);
+    EXPECT_THROW(Name(max_labels_str, strlen(max_labels_str), &origin_name),
+                 TooLongName);
+    // Appending the root should be OK
+    EXPECT_NO_THROW(Name(max_len_str, strlen(max_len_str),
+                         &Name::ROOT_NAME()));
+    EXPECT_NO_THROW(Name(max_labels_str, strlen(max_labels_str),
+                         &Name::ROOT_NAME()));
+}
+
+// Test the handling of @ in the name. If it is alone, it is the origin (when
+// it exists) or the root. If it is somewhere else, it has no special meaning.
+TEST_F(NameTest, atSign) {
+    // If it is alone, it is the origin
+    EXPECT_EQ(origin_name, Name("@", 1, &origin_name));
+    EXPECT_THROW(Name("@", 1, NULL), MissingNameOrigin);
+    EXPECT_EQ(Name::ROOT_NAME(), Name("@"));
+
+    // It is not alone. It is taken verbatim. We check the name converted
+    // back to the textual form, since checking it agains other name object
+    // may be wrong -- if we create it wrong the same way as the tested
+    // object.
+    EXPECT_EQ("\\@.", Name("@.").toText());
+    EXPECT_EQ("\\@.", Name("@.", 2, NULL).toText());
+    EXPECT_EQ("\\@something.", Name("@something").toText());
+    EXPECT_EQ("something\\@.", Name("something@").toText());
+    EXPECT_EQ("\\@x.example.com.", Name("@x", 2, &origin_name).toText());
+    EXPECT_EQ("x\\@.example.com.", Name("x@", 2, &origin_name).toText());
+
+    // An escaped at-sign isn't active
+    EXPECT_EQ("\\@.", Name("\\@").toText());
+    EXPECT_EQ("\\@.example.com.", Name("\\@", 2, &origin_name).toText());
+}
+
 TEST_F(NameTest, fromWire) {
     //
     // test cases derived from BIND9 tests.
@@ -520,7 +617,6 @@ TEST_F(NameTest, downcase) {
     // confirm the calling object is actually modified
     example_name_upper.downcase();
     compareInWireFormat(example_name_upper, example_name);
-    
 }
 
 TEST_F(NameTest, at) {