BIND 10 master, updated. be570bcaf88318286027da7da40a600cbe4223e9 [master] Update .gitignore files

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jan 7 05:39:14 UTC 2013


The branch, master has been updated
       via  be570bcaf88318286027da7da40a600cbe4223e9 (commit)
       via  e5a354694f4d0192132d00e25223d16eb24f25d7 (commit)
       via  a92495cd29fbe405bef744b98d01ff200caa05c6 (commit)
       via  f516fc484544b7e08475947d6945bc87636d4115 (commit)
       via  50e76dc960ad76ec97004832d718cb337390fce8 (commit)
       via  3e9a69424117d8f9841d76cf71dab353b6aa5435 (commit)
       via  a6ce7e4332223277ca50cd68a65e7e404621717c (commit)
       via  e17ddc8b0f6726a185e982c22dbf787351c22cdd (commit)
       via  8960a571bc2db7129d4ce8861f83808fb71dc81c (commit)
       via  3ff7531f6be864ca6965a60ef1e2744e945a3569 (commit)
       via  376b3b0760c5198dbf5ccf740c666ed9c9acaf45 (commit)
       via  27e7210b344cea5c7098bf233481d5f2bf7cac97 (commit)
       via  230078e5f45f29fec72e35a77e2fd897fb6d3fb7 (commit)
       via  77b42eabd7630b5ba7f5f9fa383de73e0e7fe49d (commit)
       via  31a4bc5aacc85f79416c6c4da2e33e642077e9f9 (commit)
       via  89f331dff577268f90d885dc871cd2a152940ef8 (commit)
       via  c6c569cae91e19aa6267081ef9926f52b87d903d (commit)
       via  772ce4c2888583e2fe1e9959c705608f6d1b7e98 (commit)
      from  9e85dc2c98c9fde0f1d73c1d6c0d0d6188b41439 (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 be570bcaf88318286027da7da40a600cbe4223e9
Author: Mukund Sivaraman <muks at isc.org>
Date:   Mon Jan 7 11:08:34 2013 +0530

    [master] Update .gitignore files

commit e5a354694f4d0192132d00e25223d16eb24f25d7
Author: Mukund Sivaraman <muks at isc.org>
Date:   Mon Jan 7 10:57:25 2013 +0530

    [2535] Update loadzone test data for newly added escaping

commit a92495cd29fbe405bef744b98d01ff200caa05c6
Author: Mukund Sivaraman <muks at isc.org>
Date:   Mon Jan 7 10:43:01 2013 +0530

    [2535] Add ChangeLog

commit f516fc484544b7e08475947d6945bc87636d4115
Merge: 50e76dc 9e85dc2
Author: Mukund Sivaraman <muks at isc.org>
Date:   Mon Jan 7 10:39:58 2013 +0530

    Merge branch 'master' into trac2535

-----------------------------------------------------------------------

Summary of changes:
 .gitignore                                         |    3 +-
 ChangeLog                                          |    7 +++
 src/bin/auth/tests/testdata/.gitignore             |    9 ++-
 src/bin/loadzone/tests/correct/known.test.out      |    4 +-
 .../datasrc/tests/zone_finder_context_unittest.cc  |   32 ++++++++++-
 src/lib/dns/rdata/generic/detail/char_string.cc    |   27 ++++++++-
 src/lib/dns/rdata/generic/detail/char_string.h     |   19 ++++++-
 src/lib/dns/rdata/generic/detail/txt_like.h        |   10 +---
 src/lib/dns/tests/rdata_char_string_unittest.cc    |   59 ++++++++++++++------
 src/lib/dns/tests/rdata_txt_like_unittest.cc       |   18 ++++++
 10 files changed, 153 insertions(+), 35 deletions(-)

-----------------------------------------------------------------------
diff --git a/.gitignore b/.gitignore
index dd6903c..7bc41b4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,5 +34,6 @@ TAGS
 /all.info
 /coverage-cpp-html
 /dns++.pc
-/report.info
+/local.zone.sqlite3
 /logger_lockfile
+/report.info
diff --git a/ChangeLog b/ChangeLog
index 7a02a50..3b5fb24 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+538.	[bug]		muks
+	Added escaping of special characters (double-quotes, semicolon,
+	backslash, etc.) in text-like RRType's toText() implementation.
+	Without this change, some TXT and SPF RDATA were incorrectly
+	stored in SQLite3 datasource as they were not escaped.
+	(Trac #2535, git f516fc484544b7e08475947d6945bc87636d4115)
+
 537.	[func]		tomek
 	b10-dhcp6: Support for RELEASE message has been added. Clients
 	are now able to release their non-temporary IPv6 addresses.
diff --git a/src/bin/auth/tests/testdata/.gitignore b/src/bin/auth/tests/testdata/.gitignore
index ccd1f57..37acc8a 100644
--- a/src/bin/auth/tests/testdata/.gitignore
+++ b/src/bin/auth/tests/testdata/.gitignore
@@ -6,8 +6,13 @@
 /shortanswer_fromWire.wire
 /simplequery_fromWire.wire
 /simpleresponse_fromWire.wire
+/example-base.sqlite3
+/example-base.sqlite3.copied
+/example-base.zone
 /example-base.zone
+/example-common-inc.zone
 /example-nsec3-inc.zone
-/example-base.sqlite3
+/example-nsec3.sqlite3
+/example-nsec3.sqlite3.copied
 /example-nsec3.zone
-/example-base.zone
+/example.zone
diff --git a/src/bin/loadzone/tests/correct/known.test.out b/src/bin/loadzone/tests/correct/known.test.out
index eec692e..377158d 100644
--- a/src/bin/loadzone/tests/correct/known.test.out
+++ b/src/bin/loadzone/tests/correct/known.test.out
@@ -80,6 +80,6 @@ ns5.example.com.		90	IN	A	4.4.4.4
 comment.example.com.		60	IN	SOA	ns1.example.com. hostmaster.example.com. 1 43200 900 1814400 7200
 comment.example.com.		60	IN	NS	ns1.example.com.
 comment.example.com.		60	IN	TXT	"Simple text"
-comment.example.com.		60	IN	TXT	"; No comment"
+comment.example.com.		60	IN	TXT	"\; No comment"
 comment.example.com.		60	IN	TXT	"Also no comment here"
-comment.example.com.		60	IN	TXT	"A combination ; see?"
+comment.example.com.		60	IN	TXT	"A combination \; see?"
diff --git a/src/lib/datasrc/tests/zone_finder_context_unittest.cc b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
index 6844712..85b167e 100644
--- a/src/lib/datasrc/tests/zone_finder_context_unittest.cc
+++ b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
@@ -81,7 +81,7 @@ addRRset(ZoneUpdaterPtr updater, ConstRRsetPtr rrset) {
 }
 
 DataSourceClientPtr
-createSQLite3Client(RRClass zclass, const Name& zname) {
+createSQLite3Client(RRClass zclass, const Name& zname, stringstream& ss) {
     // We always begin with an empty template SQLite3 DB file and install
     // the zone data from the zone file to ensure both cases have the
     // same test data.
@@ -93,7 +93,6 @@ createSQLite3Client(RRClass zclass, const Name& zname) {
     // Note that neither updater nor SQLite3 accessor checks this condition,
     // so this should succeed.
     ZoneUpdaterPtr updater = client->getUpdater(zname, false);
-    stringstream ss("ns.example.com. 3600 IN A 192.0.2.7");
     masterLoad(ss, Name::ROOT_NAME(), zclass,
                boost::bind(addRRset, updater, _1));
     updater->commit();
@@ -101,6 +100,12 @@ createSQLite3Client(RRClass zclass, const Name& zname) {
     return (client);
 }
 
+DataSourceClientPtr
+createSQLite3ClientWithNS(RRClass zclass, const Name& zname) {
+    stringstream ss("ns.example.com. 3600 IN A 192.0.2.7");
+    return (createSQLite3Client(zclass, zname, ss));
+}
+
 // The test class.  Its parameterized so we can share the test scnearios
 // for any concrete data source implementaitons.
 class ZoneFinderContextTest :
@@ -134,7 +139,7 @@ protected:
 // We test the in-memory and SQLite3 data source implementations.
 INSTANTIATE_TEST_CASE_P(, ZoneFinderContextTest,
                         ::testing::Values(createInMemoryClient,
-                                          createSQLite3Client));
+                                          createSQLite3ClientWithNS));
 
 TEST_P(ZoneFinderContextTest, getAdditionalAuthNS) {
     ZoneFinderContextPtr ctx = finder_->find(qzone_, RRType::NS());
@@ -430,4 +435,25 @@ TEST_P(ZoneFinderContextTest, getAdditionalWithRRSIGOnly) {
                 result_sets_.begin(), result_sets_.end());
 }
 
+TEST(ZoneFinderContextSQLite3Test, escapedText) {
+    // This test checks that TXTLike data, when written to a database,
+    // is escaped correctly before stored in the database. The actual
+    // escaping is done in the toText() method of TXTLike objects, but
+    // we check anyway if this also carries over to the ZoneUpdater.
+    RRClass zclass(RRClass::IN());
+    Name zname("example.org");
+    stringstream ss("escaped.example.org. 3600 IN TXT Hello~World\\;\\\"");
+    DataSourceClientPtr client = createSQLite3Client(zclass, zname, ss);
+    ZoneFinderPtr finder = client->findZone(zname).zone_finder;
+
+    // If there is no escaping, the following will throw an exception
+    // when it tries to construct a TXT RRset using the data from the
+    // database.
+    ZoneFinderContextPtr ctx = finder->find(Name("escaped.example.org"),
+                                            RRType::TXT());
+    EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
+    EXPECT_EQ("escaped.example.org. 3600 IN TXT \"Hello~World\\;\\\"\"\n",
+              ctx->rrset->toText());
+}
+
 }
diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc
index fb4c9b4..e2857b3 100644
--- a/src/lib/dns/rdata/generic/detail/char_string.cc
+++ b/src/lib/dns/rdata/generic/detail/char_string.cc
@@ -57,8 +57,8 @@ decimalToNumber(const char* s, const char* s_end) {
 }
 
 void
-strToCharString(const MasterToken::StringRegion& str_region,
-                CharString& result)
+stringToCharString(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);
@@ -91,6 +91,29 @@ strToCharString(const MasterToken::StringRegion& str_region,
     result[0] = result.size() - 1;
 }
 
+std::string
+charStringToString(const CharString& char_string) {
+    std::string s;
+    for (CharString::const_iterator it = char_string.begin() + 1;
+         it != char_string.end(); ++it) {
+        const uint8_t ch = *it;
+        if ((ch < 0x20) || (ch >= 0x7f)) {
+            // convert to escaped \xxx (decimal) format
+            s.push_back('\\');
+            s.push_back('0' + ((ch / 100) % 10));
+            s.push_back('0' + ((ch / 10) % 10));
+            s.push_back('0' + (ch % 10));
+            continue;
+        }
+        if ((ch == '"') || (ch == ';') || (ch == '\\')) {
+            s.push_back('\\');
+        }
+        s.push_back(ch);
+    }
+
+    return (s);
+}
+
 } // end of detail
 } // end of generic
 } // end of rdata
diff --git a/src/lib/dns/rdata/generic/detail/char_string.h b/src/lib/dns/rdata/generic/detail/char_string.h
index 702af04..4a146db 100644
--- a/src/lib/dns/rdata/generic/detail/char_string.h
+++ b/src/lib/dns/rdata/generic/detail/char_string.h
@@ -17,6 +17,7 @@
 
 #include <dns/master_lexer.h>
 
+#include <string>
 #include <vector>
 #include <stdint.h>
 
@@ -48,8 +49,22 @@ typedef std::vector<uint8_t> CharString;
 /// \brief str_region A string that represents a character-string.
 /// \brief result A placeholder vector where the resulting data are to be
 /// stored.  Expected to be empty, but it's not checked.
-void strToCharString(const MasterToken::StringRegion& str_region,
-                     CharString& result);
+void stringToCharString(const MasterToken::StringRegion& str_region,
+                        CharString& result);
+
+/// \brief Convert a CharString into a textual DNS character-string.
+///
+/// This method converts a binary 8-bit representation of a DNS
+/// character string into a textual string representation, escaping any
+/// special characters in the process. For example, characters like
+/// double-quotes, semi-colon and backspace are prefixed with backspace
+/// character, and characters not in the printable range of [0x20, 0x7e]
+/// (inclusive) are converted to the \xxx 3-digit decimal
+/// representation.
+///
+/// \param char_string The \c CharString to convert.
+/// \return A string representation of \c char_string.
+std::string charStringToString(const CharString& char_string);
 
 } // namespace detail
 } // namespace generic
diff --git a/src/lib/dns/rdata/generic/detail/txt_like.h b/src/lib/dns/rdata/generic/detail/txt_like.h
index d1916e3..b48d109 100644
--- a/src/lib/dns/rdata/generic/detail/txt_like.h
+++ b/src/lib/dns/rdata/generic/detail/txt_like.h
@@ -119,7 +119,7 @@ private:
                 break;
             }
             string_list_.push_back(std::vector<uint8_t>());
-            strToCharString(token.getStringRegion(), string_list_.back());
+            stringToCharString(token.getStringRegion(), string_list_.back());
         }
 
         // Let upper layer handle eol/eof.
@@ -177,18 +177,14 @@ public:
     toText() const {
         std::string s;
 
-        // XXX: this implementation is not entirely correct.  for example, it
-        // should escape double-quotes if they appear in the character string.
         for (std::vector<std::vector<uint8_t> >::const_iterator it =
-                 string_list_.begin();
-             it != string_list_.end();
-             ++it)
+                 string_list_.begin(); it != string_list_.end(); ++it)
         {
             if (!s.empty()) {
                 s.push_back(' ');
             }
             s.push_back('"');
-            s.insert(s.end(), (*it).begin() + 1, (*it).end());
+            s.append(charStringToString(*it));
             s.push_back('"');
         }
 
diff --git a/src/lib/dns/tests/rdata_char_string_unittest.cc b/src/lib/dns/tests/rdata_char_string_unittest.cc
index 9d23622..44bc979 100644
--- a/src/lib/dns/tests/rdata_char_string_unittest.cc
+++ b/src/lib/dns/tests/rdata_char_string_unittest.cc
@@ -25,7 +25,8 @@
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using isc::dns::rdata::generic::detail::CharString;
-using isc::dns::rdata::generic::detail::strToCharString;
+using isc::dns::rdata::generic::detail::stringToCharString;
+using isc::dns::rdata::generic::detail::charStringToString;
 using isc::util::unittests::matchWireData;
 
 namespace {
@@ -60,19 +61,19 @@ createStringRegion(const std::string& str) {
 TEST_F(CharStringTest, normalConversion) {
     uint8_t tmp[3];             // placeholder for expected sequence
 
-    strToCharString(str_region, chstr);
+    stringToCharString(str_region, chstr);
     matchWireData(test_charstr, sizeof(test_charstr), &chstr[0], chstr.size());
 
     // Empty string
     chstr.clear();
-    strToCharString(createStringRegion(""), chstr);
+    stringToCharString(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);
+    stringToCharString(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());
@@ -83,32 +84,32 @@ TEST_F(CharStringTest, normalConversion) {
     chstr.clear();
     long_str.at(254) = '\\';    // replace the last 'x' with '\'
     long_str.append("120");     // 'x' = 120
-    strToCharString(createStringRegion(long_str), chstr);
+    stringToCharString(createStringRegion(long_str), chstr);
     matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size());
 
     // Escaped '\'
     chstr.clear();
     tmp[0] = 1;
     tmp[1] = '\\';
-    strToCharString(createStringRegion("\\\\"), chstr);
+    stringToCharString(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);
+    stringToCharString(createStringRegion("\\000"), chstr);
     matchWireData(tmp, 2, &chstr[0], chstr.size());
 
     chstr.clear();
-    strToCharString(createStringRegion("\\255"), chstr);
+    stringToCharString(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);
+    stringToCharString(createStringRegion("\\2550"), chstr);
     tmp[0] = 2;                 // string len is now 2
     tmp[2] = '0';
     matchWireData(tmp, 3, &chstr[0], chstr.size());
@@ -116,13 +117,13 @@ TEST_F(CharStringTest, normalConversion) {
 
 TEST_F(CharStringTest, badConversion) {
     // string cannot exceed 255 bytes
-    EXPECT_THROW(strToCharString(createStringRegion(std::string(256, 'a')),
-                                 chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion(std::string(256, 'a')),
+                                    chstr),
                  CharStringTooLong);
 
     // input string ending with (non escaped) '\'
     chstr.clear();
-    EXPECT_THROW(strToCharString(createStringRegion("foo\\"), chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion("foo\\"), chstr),
                  InvalidRdataText);
 }
 
@@ -130,18 +131,44 @@ TEST_F(CharStringTest, badDDD) {
     // Check various type of bad form of \DDD
 
     // Not a number
-    EXPECT_THROW(strToCharString(createStringRegion("\\1a2"), chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion("\\1a2"), chstr),
                  InvalidRdataText);
-    EXPECT_THROW(strToCharString(createStringRegion("\\12a"), chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion("\\12a"), chstr),
                  InvalidRdataText);
 
     // Not in the range of uint8_t
-    EXPECT_THROW(strToCharString(createStringRegion("\\256"), chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion("\\256"), chstr),
                  InvalidRdataText);
 
     // Short buffer
-    EXPECT_THROW(strToCharString(createStringRegion("\\42"), chstr),
+    EXPECT_THROW(stringToCharString(createStringRegion("\\42"), chstr),
                  InvalidRdataText);
 }
 
+const struct TestData {
+    const char *data;
+    const char *expected;
+} conversion_data[] = {
+    {"Test\"Test", "Test\\\"Test"},
+    {"Test;Test", "Test\\;Test"},
+    {"Test\\Test", "Test\\\\Test"},
+    {"Test\x1fTest", "Test\\031Test"},
+    {"Test ~ Test", "Test ~ Test"},
+    {"Test\x7fTest", "Test\\127Test"},
+    {NULL, NULL}
+};
+
+TEST_F(CharStringTest, charStringToString) {
+    for (const TestData* cur = conversion_data; cur->data != NULL; ++cur) {
+        uint8_t idata[32];
+        size_t length = std::strlen(cur->data);
+        // length (1 byte) + string (length bytes)
+        assert(sizeof(idata) > length);
+        idata[0] = static_cast<uint8_t>(length);
+        std::memcpy(idata + 1, cur->data, length);
+        const CharString test_data(idata, idata + length + 1);
+        EXPECT_EQ(cur->expected, charStringToString(test_data));
+    }
+}
+
 } // unnamed namespace
diff --git a/src/lib/dns/tests/rdata_txt_like_unittest.cc b/src/lib/dns/tests/rdata_txt_like_unittest.cc
index d045875..ef9bdfe 100644
--- a/src/lib/dns/tests/rdata_txt_like_unittest.cc
+++ b/src/lib/dns/tests/rdata_txt_like_unittest.cc
@@ -334,6 +334,24 @@ TYPED_TEST(Rdata_TXT_LIKE_Test, toWireRenderer) {
 
 TYPED_TEST(Rdata_TXT_LIKE_Test, toText) {
     EXPECT_EQ("\"Test-String\"", this->rdata_txt_like.toText());
+    EXPECT_EQ("\"\"", this->rdata_txt_like_empty.toText());
+    EXPECT_EQ("\"Test-String\"", this->rdata_txt_like_quoted.toText());
+
+    // Check escape behavior
+    const TypeParam double_quotes("Test-String\"Test-String\"");
+    EXPECT_EQ("\"Test-String\\\"Test-String\\\"\"", double_quotes.toText());
+    const TypeParam semicolon("Test-String\\;Test-String");
+    EXPECT_EQ("\"Test-String\\;Test-String\"", semicolon.toText());
+    const TypeParam backslash("Test-String\\\\Test-String");
+    EXPECT_EQ("\"Test-String\\\\Test-String\"", backslash.toText());
+    const TypeParam before_x20("Test-String\\031Test-String");
+    EXPECT_EQ("\"Test-String\\031Test-String\"", before_x20.toText());
+    const TypeParam from_x20_to_x7e("\"Test-String ~ Test-String\"");
+    EXPECT_EQ("\"Test-String ~ Test-String\"", from_x20_to_x7e.toText());
+    const TypeParam from_x20_to_x7e_2("Test-String\\032\\126\\032Test-String");
+    EXPECT_EQ("\"Test-String ~ Test-String\"", from_x20_to_x7e_2.toText());
+    const TypeParam after_x7e("Test-String\\127Test-String");
+    EXPECT_EQ("\"Test-String\\127Test-String\"", after_x7e.toText());
 }
 
 TYPED_TEST(Rdata_TXT_LIKE_Test, assignment) {



More information about the bind10-changes mailing list