BIND 10 trac826, updated. 352788bf0e7df12ed4c6ef8c0761af8acac577be libcc attempt

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Jun 28 09:44:39 UTC 2012


The branch, trac826 has been updated
       via  352788bf0e7df12ed4c6ef8c0761af8acac577be (commit)
      from  8866cc711f5869f227a0e00a941399fa08d91a8e (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 352788bf0e7df12ed4c6ef8c0761af8acac577be
Author: Francis Dupont <fdupont at isc.org>
Date:   Thu Jun 28 11:44:28 2012 +0200

    libcc attempt

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

Summary of changes:
 src/lib/cc/.gitignore                 |    4 ++
 src/lib/cc/cc_messages.mes            |    2 +-
 src/lib/cc/data.cc                    |  114 +++++++++++++++++++++++++++------
 src/lib/cc/logger.h                   |   25 ++++----
 src/lib/cc/session.cc                 |   36 ++++++++++-
 src/lib/cc/session.h                  |    9 +++
 src/lib/cc/tests/.gitignore           |    2 +
 src/lib/cc/tests/Makefile.am          |   10 ++-
 src/lib/cc/tests/data_unittests.cc    |   64 +++++++++++++++++-
 src/lib/cc/tests/session_unittests.cc |   24 +++++++
 10 files changed, 251 insertions(+), 39 deletions(-)
 create mode 100644 src/lib/cc/.gitignore
 create mode 100644 src/lib/cc/tests/.gitignore

-----------------------------------------------------------------------
diff --git a/src/lib/cc/.gitignore b/src/lib/cc/.gitignore
new file mode 100644
index 0000000..cb2800f
--- /dev/null
+++ b/src/lib/cc/.gitignore
@@ -0,0 +1,4 @@
+/cc_messages.cc
+/cc_messages.h
+/session_config.h
+/session_config.h.pre
diff --git a/src/lib/cc/cc_messages.mes b/src/lib/cc/cc_messages.mes
index 8370cdd..94b955a 100644
--- a/src/lib/cc/cc_messages.mes
+++ b/src/lib/cc/cc_messages.mes
@@ -14,7 +14,7 @@
 
 $NAMESPACE isc::cc
 
-% CC_ASYNC_READ_FAILED asynchronous read failed
+% CC_ASYNC_READ_FAILED asynchronous read failed (error code = %1)
 This marks a low level error, we tried to read data from the message queue
 daemon asynchronously, but the ASIO library returned an error.
 
diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc
index c4feb4c..1873f5f 100644
--- a/src/lib/cc/data.cc
+++ b/src/lib/cc/data.cc
@@ -35,6 +35,10 @@
 
 using namespace std;
 
+namespace {
+const char* WHITESPACE = " \b\f\n\r\t";
+} // end anonymous namespace
+
 namespace isc {
 namespace data {
 
@@ -319,15 +323,49 @@ str_from_stringstream(std::istream &in, const std::string& file, const int line,
     } else {
         throwJSONError("String expected", file, line, pos);
     }
+
     while (c != EOF && c != '"') {
-        ss << c;
-        if (c == '\\' && in.peek() == '"') {
-            ss << in.get();
+        if (c == '\\') {
+            // see the spec for allowed escape characters
+            switch (in.peek()) {
+            case '"':
+                c = '"';
+                break;
+            case '/':
+                c = '/';
+                break;
+            case '\\':
+                c = '\\';
+                break;
+            case 'b':
+                c = '\b';
+                break;
+            case 'f':
+                c = '\f';
+                break;
+            case 'n':
+                c = '\n';
+                break;
+            case 'r':
+                c = '\r';
+                break;
+            case 't':
+                c = '\t';
+                break;
+            default:
+                throwJSONError("Bad escape", file, line, pos);
+            }
+            // drop the escaped char
+            in.get();
             ++pos;
         }
+        ss << c;
         c = in.get();
         ++pos;
     }
+    if (c == EOF) {
+        throwJSONError("Unterminated string", file, line, pos);
+    }
     return (ss.str());
 }
 
@@ -432,12 +470,12 @@ from_stringstream_list(std::istream &in, const std::string& file, int& line,
     ElementPtr list = Element::createList();
     ConstElementPtr cur_list_element;
 
-    skip_chars(in, " \t\n", line, pos);
+    skip_chars(in, WHITESPACE, line, pos);
     while (c != EOF && c != ']') {
         if (in.peek() != ']') {
             cur_list_element = Element::fromJSON(in, file, line, pos);
             list->add(cur_list_element);
-            skip_to(in, file, line, pos, ",]", " \t\n");
+            skip_to(in, file, line, pos, ",]", WHITESPACE);
         }
         c = in.get();
         pos++;
@@ -450,7 +488,7 @@ from_stringstream_map(std::istream &in, const std::string& file, int& line,
                       int& pos)
 {
     ElementPtr map = Element::createMap();
-    skip_chars(in, " \t\n", line, pos);
+    skip_chars(in, WHITESPACE, line, pos);
     char c = in.peek();
     if (c == EOF) {
         throwJSONError(std::string("Unterminated map, <string> or } expected"), file, line, pos);
@@ -461,7 +499,7 @@ from_stringstream_map(std::istream &in, const std::string& file, int& line,
         while (c != EOF && c != '}') {
             std::string key = str_from_stringstream(in, file, line, pos);
 
-            skip_to(in, file, line, pos, ":", " \t\n");
+            skip_to(in, file, line, pos, ":", WHITESPACE);
             // skip the :
             in.get();
             pos++;
@@ -469,7 +507,7 @@ from_stringstream_map(std::istream &in, const std::string& file, int& line,
             ConstElementPtr value = Element::fromJSON(in, file, line, pos);
             map->set(key, value);
             
-            skip_to(in, file, line, pos, ",}", " \t\n");
+            skip_to(in, file, line, pos, ",}", WHITESPACE);
             c = in.get();
             pos++;
         }
@@ -548,7 +586,7 @@ Element::fromJSON(std::istream &in, const std::string& file, int& line,
     char c = 0;
     ElementPtr element;
     bool el_read = false;
-    skip_chars(in, " \n\t", line, pos);
+    skip_chars(in, WHITESPACE, line, pos);
     while (c != EOF && !el_read) {
         c = in.get();
         pos++;
@@ -615,7 +653,14 @@ ElementPtr
 Element::fromJSON(const std::string &in) {
     std::stringstream ss;
     ss << in;
-    return (fromJSON(ss, "<string>"));
+    int line = 1, pos = 1;
+    ElementPtr result(fromJSON(ss, "<string>", line, pos));
+    skip_chars(ss, WHITESPACE, line, pos);
+    // ss must now be at end
+    if (ss.peek() != EOF) {
+        throwJSONError("Extra data", "<string>", line, pos);
+    }
+    return result;
 }
 
 // to JSON format
@@ -647,7 +692,39 @@ NullElement::toJSON(std::ostream& ss) const {
 void
 StringElement::toJSON(std::ostream& ss) const {
     ss << "\"";
-    ss << stringValue();
+    char c;
+    const std::string& str = stringValue();
+    for (size_t i = 0; i < str.size(); ++i) {
+        c = str[i];
+        // Escape characters as defined in JSON spec
+        // Note that we do not escape forward slash; this
+        // is allowed, but not mandatory.
+        switch (c) {
+        case '"':
+            ss << '\\' << c;
+            break;
+        case '\\':
+            ss << '\\' << c;
+            break;
+        case '\b':
+            ss << '\\' << 'b';
+            break;
+        case '\f':
+            ss << '\\' << 'f';
+            break;
+        case '\n':
+            ss << '\\' << 'n';
+            break;
+        case '\r':
+            ss << '\\' << 'r';
+            break;
+        case '\t':
+            ss << '\\' << 't';
+            break;
+        default:
+            ss << c;
+        }
+    }
     ss << "\"";
 }
 
@@ -785,11 +862,11 @@ StringElement::equals(const Element& other) const {
 bool
 ListElement::equals(const Element& other) const {
     if (other.getType() == Element::list) {
-        const unsigned int s = size();
+        const size_t s = size();
         if (s != other.size()) {
             return (false);
         }
-        for (unsigned int i = 0; i < s; ++i) {
+        for (size_t i = 0; i < s; ++i) {
             if (!get(i)->equals(*other.get(i))) {
                 return (false);
             }
@@ -848,15 +925,16 @@ removeIdentical(ElementPtr a, ConstElementPtr b) {
         isc_throw(TypeError, "Non-map Elements passed to removeIdentical");
     }
 
- again:
-    const std::map<std::string, ConstElementPtr>& m = a->mapValue();
+    // As maps do not allow entries with multiple keys, we can either iterate
+    // over a checking for identical entries in b or vice-versa.  As elements
+    // are removed from a if a match is found, we choose to iterate over b to
+    // avoid problems with element removal affecting the iterator.
+    const std::map<std::string, ConstElementPtr>& m = b->mapValue();
     for (std::map<std::string, ConstElementPtr>::const_iterator it = m.begin();
          it != m.end() ; ++it) {
-        if (b->contains((*it).first)) {
+        if (a->contains((*it).first)) {
             if (a->get((*it).first)->equals(*b->get((*it).first))) {
                 a->remove((*it).first);
-		// temporary fix
-		goto again;
             }
         }
     }
diff --git a/src/lib/cc/logger.h b/src/lib/cc/logger.h
index 567ccee..d6253d0 100644
--- a/src/lib/cc/logger.h
+++ b/src/lib/cc/logger.h
@@ -18,7 +18,7 @@
 #include <cc/cc_messages.h>
 #include <log/macros.h>
 
-/// \file logger.h
+/// \file cc/logger.h
 /// \brief Command Channel library global logger
 ///
 /// This holds the logger for the CC library. It is a private header
@@ -28,20 +28,19 @@
 namespace isc {
 namespace cc {
 
-enum {
-    /// \brief Trace basic operation
-    DBG_TRACE_BASIC = 10,
-    /// \brief Trace even details
-    ///
-    /// This includes messages being sent and received, waiting for messages
-    /// and alike.
-    DBG_TRACE_DETAILED = 80
-};
+/// Trace basic operation
+const int DBG_TRACE_BASIC = DBGLVL_TRACE_BASIC;
 
-/// \brief Logger for this library
+/// This includes messages being sent and received, waiting for messages
+/// and alike.
+const int DBG_TRACE_DETAILED = DBGLVL_TRACE_DETAIL;
+
+// Declaration of the logger.
 extern isc::log::Logger logger;
 
-}
-}
+} // namespace cc
+} // namespace isc
+
+/// \brief Logger for this library
 
 #endif
diff --git a/src/lib/cc/session.cc b/src/lib/cc/session.cc
index 024c6e4..47ee2a3 100644
--- a/src/lib/cc/session.cc
+++ b/src/lib/cc/session.cc
@@ -93,6 +93,11 @@ public:
     void startRead(boost::function<void()> user_handler);
     void setTimeout(size_t seconds) { timeout_ = seconds; };
     size_t getTimeout() const { return timeout_; };
+#ifdef _WIN32
+    SOCKET getSocketDesc();
+#else
+    int getSocketDesc();
+#endif
 
     long int sequence_; // the next sequence number to use
     std::string lname_;
@@ -276,12 +281,28 @@ SessionImpl::internalRead(const asio::error_code& error,
         }
         user_handler_();
     } else {
-        LOG_ERROR(logger, CC_ASYNC_READ_FAILED);
+        LOG_ERROR(logger, CC_ASYNC_READ_FAILED).arg(error.value());
         isc_throw(SessionError, "asynchronous read failed");
     }
 }
 
-Session::Session(io_service& io_service) : impl_(new SessionImpl(io_service))
+#ifdef _WIN32
+SOCKET
+#else
+int
+#endif
+SessionImpl::getSocketDesc() {
+    /// @todo boost 1.42 uses native() method, but it is deprecated
+    /// in 1.49 and native_handle() is recommended instead
+    if (!socket_.is_open()) {
+        isc_throw(InvalidOperation,
+                  "Can't return socket descriptor: no socket opened.");
+    }
+    return socket_.native();
+}
+
+Session::Session(asio::io_service& io_service) :
+    impl_(new SessionImpl(io_service))
 {}
 
 Session::~Session() {
@@ -299,6 +320,15 @@ Session::startRead(boost::function<void()> read_callback) {
     impl_->startRead(read_callback);
 }
 
+#ifdef _WIN32
+SOCKET
+#else
+int
+#endif
+Session::getSocketDesc() const {
+    return impl_->getSocketDesc();
+}
+
 namespace {                     // maybe unnecessary.
 // This is a helper class to make the establish() method (below) exception-safe
 // with the RAII approach.
@@ -393,7 +423,7 @@ Session::recvmsg(ConstElementPtr& env, ConstElementPtr& msg,
     size_t length = impl_->readDataLength();
     if (hasQueuedMsgs()) {
         ConstElementPtr q_el;
-        for (unsigned int i = 0; i < impl_->queue_->size(); i++) {
+        for (size_t i = 0; i < impl_->queue_->size(); i++) {
             q_el = impl_->queue_->get(i);
             if (( seq == -1 &&
                   !q_el->get(0)->contains("reply")
diff --git a/src/lib/cc/session.h b/src/lib/cc/session.h
index e91dd76..4968a74 100644
--- a/src/lib/cc/session.h
+++ b/src/lib/cc/session.h
@@ -141,6 +141,15 @@ namespace isc {
             virtual bool hasQueuedMsgs() const;
             virtual void setTimeout(size_t milliseconds);
             virtual size_t getTimeout() const;
+
+            /// @brief returns socket descriptor from underlying socket connection
+            ///
+            /// @param returns socket descriptor used for session connection
+#ifdef _WIN32
+            virtual SOCKET getSocketDesc() const;
+#else
+            virtual int getSocketDesc() const;
+#endif
     private:
             void sendmsg(isc::data::ConstElementPtr msg);
             void sendmsg(isc::data::ConstElementPtr env,
diff --git a/src/lib/cc/tests/.gitignore b/src/lib/cc/tests/.gitignore
new file mode 100644
index 0000000..f10451c
--- /dev/null
+++ b/src/lib/cc/tests/.gitignore
@@ -0,0 +1,2 @@
+/run_unittests
+/session_unittests_config.h
diff --git a/src/lib/cc/tests/Makefile.am b/src/lib/cc/tests/Makefile.am
index 4760855..7291abe 100644
--- a/src/lib/cc/tests/Makefile.am
+++ b/src/lib/cc/tests/Makefile.am
@@ -16,6 +16,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	$(LIBTOOL) --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
@@ -24,11 +27,14 @@ run_unittests_SOURCES = data_unittests.cc session_unittests.cc run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 
-run_unittests_LDADD = $(GTEST_LDADD)
-run_unittests_LDADD +=  $(top_builddir)/src/lib/cc/libcc.la
+# We need to put our libs first, in case gtest (or any dependency, really)
+# is installed in the same location as a different version of bind10
+# Otherwise the linker may not use the source tree libs
+run_unittests_LDADD =  $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/exceptions/libexceptions.la
+run_unittests_LDADD += $(GTEST_LDADD)
 
 endif
 
diff --git a/src/lib/cc/tests/data_unittests.cc b/src/lib/cc/tests/data_unittests.cc
index 53d5ab8..87d92f6 100644
--- a/src/lib/cc/tests/data_unittests.cc
+++ b/src/lib/cc/tests/data_unittests.cc
@@ -20,6 +20,7 @@
 
 using namespace isc::data;
 
+#include <sstream>
 #include <iostream>
 using std::oct;
 #include <iomanip>
@@ -90,7 +91,7 @@ TEST(Element, from_and_to_json) {
     sv.push_back("-1");
     sv.push_back("-1.234");
     sv.push_back("-123.456");
-    
+
     BOOST_FOREACH(const std::string& s, sv) {
         // test << operator, which uses Element::str()
         std::ostringstream stream;
@@ -122,8 +123,16 @@ TEST(Element, from_and_to_json) {
     sv.push_back("{ \"a\": None}");
     sv.push_back("");
     sv.push_back("nul");
+    sv.push_back("hello\"foobar\"");
+    sv.push_back("\"foobar\"hello");
+    sv.push_back("[]hello");
+    sv.push_back("{}hello");
+    // String not delimited correctly
+    sv.push_back("\"hello");
+    sv.push_back("hello\"");
+
+
     BOOST_FOREACH(std::string s, sv) {
-        
         EXPECT_THROW(el = Element::fromJSON(s), isc::data::JSONError);
     }
 
@@ -150,6 +159,9 @@ TEST(Element, from_and_to_json) {
     EXPECT_EQ("false", Element::fromJSON("FALSE")->str());
     EXPECT_EQ("true", Element::fromJSON("True")->str());
     EXPECT_EQ("true", Element::fromJSON("TRUE")->str());
+    EXPECT_EQ("\"\"", Element::fromJSON("  \n \t \r \f \b \"\" \n \f \t \r \b")->str());
+    EXPECT_EQ("{  }", Element::fromJSON("{  \n  \r \t  \b \f }")->str());
+    EXPECT_EQ("[  ]", Element::fromJSON("[  \n  \r \f \t  \b  ]")->str());
 
     // number overflows
     EXPECT_THROW(Element::fromJSON("12345678901234567890")->str(), JSONError);
@@ -299,6 +311,43 @@ TEST(Element, create_and_value_throws) {
 
 }
 
+// Helper for escape check; it puts the given string in a StringElement,
+// then checks for the following conditions:
+// stringValue() must be same as input
+// toJSON() output must be escaped
+// fromJSON() on the previous output must result in original input
+void
+escapeHelper(const std::string& input, const std::string& expected) {
+    StringElement str_element = StringElement(input);
+    EXPECT_EQ(input, str_element.stringValue());
+    std::stringstream os;
+    str_element.toJSON(os);
+    EXPECT_EQ(expected, os.str());
+    ElementPtr str_element2 = Element::fromJSON(os.str());
+    EXPECT_EQ(str_element.stringValue(), str_element2->stringValue());
+}
+
+TEST(Element, escape) {
+    // Test whether quotes are escaped correctly when creating direct
+    // String elements.
+    escapeHelper("foo\"bar", "\"foo\\\"bar\"");
+    escapeHelper("foo\\bar", "\"foo\\\\bar\"");
+    escapeHelper("foo\bbar", "\"foo\\bbar\"");
+    escapeHelper("foo\fbar", "\"foo\\fbar\"");
+    escapeHelper("foo\nbar", "\"foo\\nbar\"");
+    escapeHelper("foo\rbar", "\"foo\\rbar\"");
+    escapeHelper("foo\tbar", "\"foo\\tbar\"");
+    // Bad escapes
+    EXPECT_THROW(Element::fromJSON("\\a"), JSONError);
+    EXPECT_THROW(Element::fromJSON("\\"), JSONError);
+    // Can't have escaped quotes outside strings
+    EXPECT_THROW(Element::fromJSON("\\\"\\\""), JSONError);
+    // Inside strings is OK
+    EXPECT_NO_THROW(Element::fromJSON("\"\\\"\\\"\""));
+    // A whitespace test
+    EXPECT_NO_THROW(Element::fromJSON("\"  \n  \r \t \f  \n \n    \t\""));
+}
+
 TEST(Element, ListElement) {
     // this function checks the specific functions for ListElements
     ElementPtr el = Element::fromJSON("[ 1, \"bar\", 3 ]");
@@ -523,6 +572,12 @@ TEST(Element, removeIdentical) {
     removeIdentical(a, b);
     EXPECT_EQ(*a, *c);
 
+    a = Element::fromJSON("{ \"a\": 1, \"b\": 2, \"c\": 3 }");
+    b = Element::fromJSON("{ \"c\": 3, \"b\": 2 }");
+    c = Element::fromJSON("{ \"a\": 1 }");
+    removeIdentical(a, b);
+    EXPECT_EQ(*a, *c);
+
     EXPECT_THROW(removeIdentical(Element::create(1), Element::create(2)), TypeError);
 }
 
@@ -567,6 +622,11 @@ TEST(Element, constRemoveIdentical) {
     c = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
     EXPECT_EQ(*removeIdentical(a, b), *c);
 
+    a = Element::fromJSON("{ \"a\": 1, \"b\": 2, \"c\": 3 }");
+    b = Element::fromJSON("{ \"c\": 3, \"b\": 2 }");
+    c = Element::fromJSON("{ \"a\": 1 }");
+    EXPECT_EQ(*removeIdentical(a, b), *c);
+
     EXPECT_THROW(removeIdentical(Element::create(1), Element::create(2)),
                  TypeError);
 }
diff --git a/src/lib/cc/tests/session_unittests.cc b/src/lib/cc/tests/session_unittests.cc
index 72dd213..0705257 100644
--- a/src/lib/cc/tests/session_unittests.cc
+++ b/src/lib/cc/tests/session_unittests.cc
@@ -38,6 +38,9 @@ TEST(AsioSession, establish) {
     asio::io_service io_service_;
     Session sess(io_service_);
 
+    // can't return socket desciptor before session is established
+    EXPECT_THROW(sess.getSocketDesc(), isc::InvalidOperation);
+
     EXPECT_THROW(
         sess.establish("/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
                        "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
@@ -276,3 +279,24 @@ TEST_F(SessionTest, run_with_handler_timeout) {
     // No followup message, should time out.
     ASSERT_THROW(my_io_service.run(), SessionTimeout);
 }
+
+TEST_F(SessionTest, get_socket_descr) {
+    tds->setSendLname();
+    sess.establish(BIND10_TEST_SOCKET_FILE);
+
+#ifdef _WIN32
+    SOCKET socket = INVALID_SOCKET;
+    // session is established, so getSocketDesc() should work
+    EXPECT_NO_THROW(socket = sess.getSocketDesc());
+
+    // expect actual socket handle to be returned, not INVALID_SOCKET
+    EXPECT_NE(INVALID_SOCKET, socket);
+#else
+    int socket = 0;
+    // session is established, so getSocketDesc() should work
+    EXPECT_NO_THROW(socket = sess.getSocketDesc());
+
+    // expect actual socket handle to be returned, not 0
+    EXPECT_LT(0, socket);
+#endif
+}



More information about the bind10-changes mailing list