[5680] Changes after review

diff --git a/doc/examples/kea4/with-ddns.json b/doc/examples/kea4/with-ddns.json
index 57544f66855928879168a6d1b9ff1931e83152bd..e192c6e446d35087fedfe5368f15428d1bfaaece 100644 (file)
--- a/doc/examples/kea4/with-ddns.json
+++ b/doc/examples/kea4/with-ddns.json
@@ -54,7 +54,9 @@
         "override-client-update" : true,
         "replace-client-name" : "when-present",
         "generated-prefix" : "test.prefix",
-        "qualifying-suffix" : "test.suffix."
+        "qualifying-suffix" : "test.suffix.",
+        "hostname-char-set": "[^A-Za-z0-9.-]",
+        "hostname-char-replacement": "x"
     }
 },
 

diff --git a/doc/examples/kea6/with-ddns.json b/doc/examples/kea6/with-ddns.json
index 6792429b7def35b8b334e257e7bf2a0aef42c74a..2f5bbe308cff555217146110c983b74c24f7a105 100644 (file)
--- a/doc/examples/kea6/with-ddns.json
+++ b/doc/examples/kea6/with-ddns.json
@@ -56,7 +56,9 @@
         "override-client-update" : true,
         "replace-client-name" : "when-present",
         "generated-prefix" : "test.prefix",
-        "qualifying-suffix" : "test.suffix."
+        "qualifying-suffix" : "test.suffix.",
+        "hostname-char-set": "[^A-Za-z0-9.-]",
+        "hostname-char-replacement": "x"
   }
 
 },

diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 2c1c6857fb90abab8f5308f9a12f7e008ffc35a8..f1f3f8e229357a305b13884b5f91298709037b1c 100644 (file)
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -1742,22 +1742,22 @@ TEST_F(NameDhcpv4SrvTest, sanitizeHost) {
 
     Pkt4Ptr resp;
     OptionStringPtr hostname;
-    for (auto scenario = scenarios.begin(); scenario != scenarios.end(); ++scenario) {
-        SCOPED_TRACE((*scenario).description_);
+    for (auto scenario : scenarios) {
+        SCOPED_TRACE((scenario).description_);
         {
-        // Set the hostname option.
-        ASSERT_NO_THROW(client.includeHostname((*scenario).original_));
-
-        // Send the DHCPDISCOVER and make sure that the server responded.
-        ASSERT_NO_THROW(client.doDiscover());
-        resp = client.getContext().response_;
-        ASSERT_TRUE(resp);
-        ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
-
-        // Make sure the response hostname is what we expect.
-        hostname = boost::dynamic_pointer_cast<OptionString>(resp->getOption(DHO_HOST_NAME));
-        ASSERT_TRUE(hostname);
-        EXPECT_EQ((*scenario).sanitized_, hostname->getValue());
+            // Set the hostname option.
+            ASSERT_NO_THROW(client.includeHostname((scenario).original_));
+
+            // Send the DHCPDISCOVER and make sure that the server responded.
+            ASSERT_NO_THROW(client.doDiscover());
+            resp = client.getContext().response_;
+            ASSERT_TRUE(resp);
+            ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
+
+            // Make sure the response hostname is what we expect.
+            hostname = boost::dynamic_pointer_cast<OptionString>(resp->getOption(DHO_HOST_NAME));
+            ASSERT_TRUE(hostname);
+            EXPECT_EQ((scenario).sanitized_, hostname->getValue());
         }
     }
 }

diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
index 13cf020ac053fca43f15954d9e19f895a9a95736..bf3a3d826cec1ff7b5bb91bbd2a63a33a965a149 100644 (file)
--- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc
+++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
@@ -1239,40 +1239,40 @@ TEST(D2ClientMgr, sanitizeFqdnV6) {
 
     std::vector<Scenario> scenarios = {
         {
-        "full FQDN, name unchanged",
-        "One.123.example.com.",
-        Option6ClientFqdn::FULL,
-        "one.123.example.com."
+            "full FQDN, name unchanged",
+            "One.123.example.com.",
+            Option6ClientFqdn::FULL,
+            "one.123.example.com."
         },
         {
-        "partial FQDN, name unchanged, but qualified",
-        "One.123",
-        Option6ClientFqdn::PARTIAL,
-        "one.123.suffix.com."
+            "partial FQDN, name unchanged, but qualified",
+            "One.123",
+            Option6ClientFqdn::PARTIAL,
+            "one.123.suffix.com."
         },
         {
-        "full FQDN, scrubbed",
-        "O#n^e.123.ex&a*mple.com.",
-        Option6ClientFqdn::FULL,
-        "oxnxe.123.exxaxmple.com."
+            "full FQDN, scrubbed",
+            "O#n^e.123.ex&a*mple.com.",
+            Option6ClientFqdn::FULL,
+            "oxnxe.123.exxaxmple.com."
         },
         {
-        "partial FQDN, scrubbed and qualified",
-        "One.1+2|3",
-        Option6ClientFqdn::PARTIAL,
-        "one.1x2x3.suffix.com."
+            "partial FQDN, scrubbed and qualified",
+            "One.1+2|3",
+            Option6ClientFqdn::PARTIAL,
+            "one.1x2x3.suffix.com."
         },
         {
-        "full FQDN with characters that get escaped",
-        "O n e.123.exa(m)ple.com.",
-        Option6ClientFqdn::FULL,
-        "oxnxe.123.exaxmxple.com."
+            "full FQDN with characters that get escaped",
+            "O n e.123.exa(m)ple.com.",
+            Option6ClientFqdn::FULL,
+            "oxnxe.123.exaxmxple.com."
         },
         {
-        "full FQDN with escape sequences",
-        "O\032n\032e.123.example.com.",
-        Option6ClientFqdn::FULL,
-        "oxnxe.123.example.com."
+            "full FQDN with escape sequences",
+            "O\032n\032e.123.example.com.",
+            Option6ClientFqdn::FULL,
+            "oxnxe.123.example.com."
         }
     };
 

diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc
index 3cd1330e09e48f227418c9bc5e1101109264769b..6db3cefbf67a62af83771322b4dcd32e0916f0e4 100644 (file)
--- a/src/lib/dns/tests/labelsequence_unittest.cc
+++ b/src/lib/dns/tests/labelsequence_unittest.cc
@@ -638,6 +638,12 @@ TEST_F(LabelSequenceTest, toRawText) {
     LabelSequence l(n);
     EXPECT_EQ("a bc.$exa(m)ple.@org", l.toRawText(true));
     EXPECT_EQ("a bc.$exa(m)ple.@org.", l.toRawText(false));
+
+    // toRawText is not supposed to do any sanity checks.
+    // Let's try with a very weird name.
+    Name n2("xtra\tchars\n.in.name");
+    LabelSequence l2(n2);
+    EXPECT_EQ("xtra\tchars\n.in.name.", l2.toRawText(false));
 }
 
 // The following are test data used in the getHash test below.  Normally

diff --git a/src/lib/util/strutil.h b/src/lib/util/strutil.h
index 1cdb833c188389eb08770c98bd38828c95e374d8..cf2000e3f9c2c7c5eea6e500b84bf9a71752cf3e 100644 (file)
--- a/src/lib/util/strutil.h
+++ b/src/lib/util/strutil.h
@@ -263,7 +263,7 @@ class StringSanitizerImpl;
 ///
 /// The implementation uses C++11 regex IF the environemnt supports it
 /// (tested in configure.ac). If not it falls back to C lib regcomp/regexec.
-/// Older compilers, such as  pre Gnu 4.9.0, provided only experimental
+/// Older compilers, such as pre Gnu g++ 4.9.0, provided only experimental
 /// implementations of regex which are recognized as buggy.
 class StringSanitizer {
 public:
@@ -271,7 +271,7 @@ public:
     /// Constructor
     ///
     /// Compiles the given character set into a regular expression, and
-    /// retains the given character replacement.  Thereafter, the instance
+    /// retains the given character replacement. Thereafter, the instance
     /// may be used to scrub an arbitrary number of strings.
     ///
     /// @param char_set string containing a regular expression (POSIX
@@ -291,8 +291,8 @@ public:
 
     /// Returns a scrubbed copy of a given string
     ///
-    /// Replaces all occurrances of characters described by the regular
-    /// expression with the character replacement .
+    /// Replaces all occurrences of characters described by the regular
+    /// expression with the character replacement.
     ///
     /// @param original the string to scrub
     /// @throw Unexpected if an error occurs during scrubbing

diff --git a/src/lib/util/tests/strutil_unittest.cc b/src/lib/util/tests/strutil_unittest.cc
index 893409621de296f205cdb9cc0f36264251096a28..2672521fb4e8b786d1ec1289f6b4fdee7a33709f 100644 (file)
--- a/src/lib/util/tests/strutil_unittest.cc
+++ b/src/lib/util/tests/strutil_unittest.cc
@@ -463,7 +463,7 @@ TEST(StringUtilTest, decodeFormattedHexString) {
                  isc::BadValue);
 }
 
-/// @brief Fucntion used to test StringSantitizer
+/// @brief Function used to test StringSantitizer
 /// @param original - string to sanitize
 /// @param char_set - regular expression string describing invalid
 /// characters
@@ -507,7 +507,7 @@ TEST(StringUtilTest, stringSanitizer) {
     sanitizeStringTest("abc.123", "[b-c2]", "*",
                        "a**.1*3");
 
-    // Inverted list of valid chars should work: (b,c,2 are invalid)
+    // Inverted list of valid chars should work: (b,c,2 are valid)
     sanitizeStringTest("abc.123", "[^b-c2]", "*",
                        "*bc**2*");
 
@@ -527,7 +527,7 @@ TEST(StringUtilTest, stringSanitizer) {
     sanitizeStringTest("%%A%%B%%C%%", "[^A-Za-z0-9_]", "x",
                        "xxAxxBxxCxx");
 
-    // Removing than one non-matching in a row should work.
+    // Removing more than one non-matching in a row should work.
     sanitizeStringTest("%%A%%B%%C%%", "[^A-Za-z0-9_]", "",
                        "ABC");