[1626] escape JSON strings correctly

2 files changed, 56 insertions, 5 deletions

diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc
index 77f948aea8..8c050fe40d 100644
--- a/src/lib/cc/data.cc
+++ b/src/lib/cc/data.cc
@@ -314,12 +314,21 @@ 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();
-            ++pos;
+        if (c == '\\') {
+            // next char must be either another \ or "
+            // see the spec for allowed escape characters
+            if (strchr("\"\\/\b\f\n\r\t", in.peek()) != NULL) {
+                // drop the escape
+                c = in.get();
+                ++pos;
+            } else {
+                std::cout << "[XX] cur string: " << ss.str() << std::endl;
+                throwJSONError(std::string("Bad escape for: ") + (char)in.peek(), file, line, pos);
+            }
         }
+        ss << c;
         c = in.get();
         ++pos;
     }
@@ -642,7 +651,16 @@ 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
+        if (strchr("\"\\/\b\f\n\r\t", c) != NULL) {
+            ss << '\\';
+        }
+        ss << c;
+    }
     ss << "\"";
 }
 
diff --git a/src/lib/cc/tests/data_unittests.cc b/src/lib/cc/tests/data_unittests.cc
index d8624cbaa0..e09b4fee5c 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>
@@ -299,6 +300,38 @@ 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/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);
+}
+
 TEST(Element, ListElement) {
     // this function checks the specific functions for ListElements
     ElementPtr el = Element::fromJSON("[ 1, \"bar\", 3 ]");