[2442] handled various corner cases

src/lib/dns/rdata/generic/detail/char_string.cc

@@ -12,10 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
+#include <dns/rdata.h>
 #include <dns/master_lexer.h>
 #include <dns/rdata/generic/detail/char_string.h>
 
+#include <boost/lexical_cast.hpp>
+
+#include <cassert>
+#include <cctype>
+#include <cstring>
 #include <vector>
+
 #include <stdint.h>
 
 namespace isc {
@@ -24,18 +33,65 @@ namespace rdata {
 namespace generic {
 namespace detail {
 
+namespace {
+// Convert a DDD form to the corresponding integer
+int
+decimalToNumber(const char* s, const char* s_end) {
+    if (s_end - s < 3) {
+        isc_throw(InvalidRdataText, "Escaped digits too short");
+    }
+
+    // Make a nul-terminated copy of the 'DDD' for lexical_cast
+    char buf[4];
+    std::memcpy(buf, s, 3);
+    buf[3] = 0;
+
+    try {
+        const int i = boost::lexical_cast<int>(buf);
+        if (i > 255) {
+            isc_throw(InvalidRdataText, "Escaped digits too large: " << buf);
+        }
+        return (i);
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(InvalidRdataText,
+                  "Invalid form for escaped digits: " << buf);
+    }
+}
+}
+
 void
 strToCharString(const MasterToken::StringRegion& str_region,
                 CharString& result)
 {
+    // make a space for the 1-byte length field; filled in at the end
     result.push_back(0);
-    size_t n = str_region.len;
+
+    bool escape = false;
     const char* s = str_region.beg;
-    while (n-- != 0) {
-        const int c = (*s++) & 0xff;
+    const char* const s_end = str_region.beg + str_region.len;
+
+    for (size_t n = str_region.len; n != 0; --n, ++s) {
+        int c = (*s & 0xff);
+        if (escape && std::isdigit(c) != 0) {
+            c = decimalToNumber(s, s_end);
+            assert(n >= 3);
+            n -= 2;
+            s += 2;
+        } else if (!escape && c == '\\') {
+            escape = true;
+            continue;
+        }
+        escape = false;
         result.push_back(c);
     }
-    result[0] = str_region.len; // FIXME: this is not always correct
+    if (escape) {               // terminated by non-escaped '\'
+        isc_throw(InvalidRdataText, "character-string ends with '\\'");
+    }
+    if (result.size() > MAX_CHARSTRING_LEN + 1) { // '+ 1' due to the len field
+        isc_throw(CharStringTooLong, "character-string is too long: " <<
+                  result.size() << " bytes");
+    }
+    result[0] = result.size() - 1;
 }
 
 } // end of detail

src/lib/dns/tests/rdata_char_string_unittest.cc

@@ -13,6 +13,8 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/unittests/wiredata.h>
+
+#include <dns/rdata.h>
 #include <dns/rdata/generic/detail/char_string.h>
 
 #include <gtest/gtest.h>
@@ -35,19 +37,111 @@ const uint8_t test_charstr[] = {
 class CharStringTest : public ::testing::Test {
 protected:
     CharStringTest() :
-        test_str("Test String")
+        // char-string representation for test data using two types of escape
+        // ('r' = 114)
+        test_str("Test\\ St\\114ing")
     {
         str_region.beg = &test_str[0];
         str_region.len = test_str.size();
     }
-    CharString chstr;           // placeholder
+    CharString chstr;           // place holder
     const std::string test_str;
     MasterToken::StringRegion str_region;
 };
 
-TEST_F(CharStringTest, test) {
+MasterToken::StringRegion
+createStringRegion(const std::string& str) {
+    MasterToken::StringRegion region;
+    region.beg = &str[0];       // note this works even if str is empty
+    region.len = str.size();
+    return (region);
+}
+
+TEST_F(CharStringTest, normalConversion) {
+    uint8_t tmp[3];             // placeholder for expected sequence
+
     strToCharString(str_region, chstr);
     matchWireData(test_charstr, sizeof(test_charstr), &chstr[0], chstr.size());
+
+    // Empty string
+    chstr.clear();
+    strToCharString(createStringRegion(""), chstr);
+    tmp[0] = 0;
+    matchWireData(tmp, 1, &chstr[0], chstr.size());
+
+    // Possible largest char string
+    chstr.clear();
+    std::string long_str(255, 'x');
+    strToCharString(createStringRegion(long_str), chstr);
+    std::vector<uint8_t> expected;
+    expected.push_back(255);    // len of char string
+    expected.insert(expected.end(), long_str.begin(), long_str.end());
+    matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size());
+
+    // Same data as the previous case, but the original string is longer than
+    // the max; this shouldn't be rejected
+    chstr.clear();
+    long_str.at(254) = '\\';    // replace the last 'x' with '\'
+    long_str.append("120");     // 'x' = 120
+    strToCharString(createStringRegion(long_str), chstr);
+    matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size());
+
+    // Escaped '\'
+    chstr.clear();
+    tmp[0] = 1;
+    tmp[1] = '\\';
+    strToCharString(createStringRegion("\\\\"), chstr);
+    matchWireData(tmp, 2, &chstr[0], chstr.size());
+
+    // Boundary values for \DDD
+    chstr.clear();
+    tmp[0] = 1;
+    tmp[1] = 0;
+    strToCharString(createStringRegion("\\000"), chstr);
+    matchWireData(tmp, 2, &chstr[0], chstr.size());
+
+    chstr.clear();
+    strToCharString(createStringRegion("\\255"), chstr);
+    tmp[0] = 1;
+    tmp[1] = 255;
+    matchWireData(tmp, 2, &chstr[0], chstr.size());
+
+    // Another digit follows DDD; it shouldn't cause confusion
+    chstr.clear();
+    strToCharString(createStringRegion("\\2550"), chstr);
+    tmp[0] = 2;                 // string len is now 2
+    tmp[2] = '0';
+    matchWireData(tmp, 3, &chstr[0], chstr.size());
+}
+
+TEST_F(CharStringTest, badConversion) {
+    // string cannot exceed 255 bytes
+    EXPECT_THROW(strToCharString(createStringRegion(std::string(256, 'a')),
+                                 chstr),
+                 CharStringTooLong);
+
+    // input string ending with (non escaped) '\'
+    chstr.clear();
+    EXPECT_THROW(strToCharString(createStringRegion("foo\\"), chstr),
+                 InvalidRdataText);
+}
+
+TEST_F(CharStringTest, badDDD) {
+    // Check various type of bad form of \DDD
+
+    // Not a number
+    EXPECT_THROW(strToCharString(createStringRegion("\\1a2"), chstr),
+                 InvalidRdataText);
+    EXPECT_THROW(strToCharString(createStringRegion("\\12a"), chstr),
+                 InvalidRdataText);
+
+    // Not in the range of uint8_t
+    EXPECT_THROW(strToCharString(createStringRegion("\\256"), chstr),
+                 InvalidRdataText);
+
+    // Short buffer
+    EXPECT_THROW(strToCharString(createStringRegion("\\42"), chstr),
+                 InvalidRdataText);
 }
 
 } // unnamed namespace