Remove escaping all Python code, instead only escape strings

Previous attempt to support multi-line strings (#8) has escaped newline
characters from entire Python code. After reconsidering the use case,
single-line code only will be supported. However, the string values of
the records might still contain new lines which has now been addressed
when for selected records VAL fields only. Single quote escaping has
also been fixed which closes #9 - double qoutes are not permitted by
EPICS records unless they're escaped in the record already.

PyDevice.files

@@ -16,6 +16,7 @@ src/pydev_stringout.cpp
 src/pydev_waveform.cpp
 src/pywrapper.cpp
 src/pywrapper.h
+src/unittest/test_util.cpp
 src/util.cpp
 src/util.h
 testApp/src/O.linux-x86_64/pydevioc_registerRecordDeviceDriver.cpp

README.md

@@ -79,7 +79,7 @@ pydev("from mydevice import MyDevice")
 pydev("device1 = MyDevice('127.0.0.1')")
 ```
 
-*Hint: `pydev()` function allows to execute arbitrary Python code from IOC shell.*
+*Hint: `pydev()` function allows to execute arbitrary single-line Python code from IOC shell.*
 
 
 ### Trigger connect() from database
@@ -111,7 +111,7 @@ record(stringout, "Device1:Send")
 
 PyDevice will replace certain macros in double percent (%) sign with current record's fields right before the Python code is executed. Ie. %VAL% will be automatically replaced with string in VAL field, by default 127.0.0.1. Fields from other records can not be referenced this way.
 
-*Hint: Arbitrary Python code can be executed from any of the supported records.*
+*Hint: Arbitrary single-line Python code can be executed from any of the supported records.*
 
 ### Getting values from Python code to records
 

src/pydev_lsi.cpp

@@ -83,7 +83,7 @@ static void processRecordCb(lsiRecord* rec)
 
     auto fields = Util::getReplacables(rec->inp.value.instio.string);
     for (auto& keyval: fields) {
-        if (keyval.first == "%VAL%")       keyval.second = rec->val;
+        if (keyval.first == "%VAL%")       keyval.second = Util::escape(rec->val);
         else if (keyval.first == "%NAME%") keyval.second = rec->name;
         else if (keyval.first == "%SIZV%") keyval.second = std::to_string(rec->sizv);
         else if (keyval.first == "%LEN%")  keyval.second = std::to_string(rec->len);

src/pydev_lso.cpp

@@ -83,7 +83,7 @@ static void processRecordCb(lsoRecord* rec)
 
     auto fields = Util::getReplacables(rec->out.value.instio.string);
     for (auto& keyval: fields) {
-        if (keyval.first == "%VAL%")       keyval.second = rec->val;
+        if (keyval.first == "%VAL%")       keyval.second = Util::escape(rec->val);
         else if (keyval.first == "%NAME%") keyval.second = rec->name;
         else if (keyval.first == "%SIZV%") keyval.second = std::to_string(rec->sizv);
         else if (keyval.first == "%LEN%")  keyval.second = std::to_string(rec->len);

src/pydev_stringin.cpp

@@ -83,7 +83,7 @@ static void processRecordCb(stringinRecord* rec)
 
     auto fields = Util::getReplacables(rec->inp.value.instio.string);
     for (auto& keyval: fields) {
-        if      (keyval.first == "%VAL%")  keyval.second = rec->val;
+        if      (keyval.first == "%VAL%")  keyval.second = Util::escape(rec->val);
         else if (keyval.first == "%NAME%") keyval.second = rec->name;
     }
     std::string code = Util::replace(rec->inp.value.instio.string, fields);

src/pydev_stringout.cpp

@@ -83,7 +83,7 @@ static void processRecordCb(stringoutRecord* rec)
 
     auto fields = Util::getReplacables(rec->out.value.instio.string);
     for (auto& keyval: fields) {
-        if      (keyval.first == "%VAL%")  keyval.second = rec->val;
+        if      (keyval.first == "%VAL%")  keyval.second = Util::escape(rec->val);
         else if (keyval.first == "%NAME%") keyval.second = rec->name;
     }
     std::string code = Util::replace(rec->out.value.instio.string, fields);

src/pywrapper.cpp

@@ -17,7 +17,6 @@ static PyObject* locDict = nullptr;
 static PyThreadState* mainThread = nullptr;
 static std::map<std::string, std::pair<PyWrapper::Callback, PyObject*>> params;
 
-
 /**
  * Function for caching parameter value or notifying record of new value.
  *
@@ -344,9 +343,7 @@ void PyWrapper::exec(const std::string& line, bool debug)
 {
     PyGIL gil;
 
-    auto line_escaped = escapeNewLine(line);
-
-    PyObject* r = PyRun_String(line_escaped.c_str(), Py_file_input, globDict, locDict);
+    PyObject* r = PyRun_String(line.c_str(), Py_file_input, globDict, locDict);
     if (r == nullptr) {
         if (debug && PyErr_Occurred()) {
             PyErr_Print();
@@ -362,10 +359,8 @@ bool PyWrapper::exec(const std::string& line, bool debug, T* val)
 {
     PyGIL gil;
 
-    auto line_escaped = escapeNewLine(line);
-
     if (val != nullptr) {
-        PyObject* r = PyRun_String(line_escaped.c_str(), Py_eval_input, globDict, locDict);
+        PyObject* r = PyRun_String(line.c_str(), Py_eval_input, globDict, locDict);
         if (r != nullptr) {
             T value;
             bool converted = convert(r, value);
@@ -383,7 +378,7 @@ bool PyWrapper::exec(const std::string& line, bool debug, T* val)
     }
 
     // Either *val == nullptr or eval failed, let's try executing code instead
-    PyObject* r = PyRun_String(line_escaped.c_str(), Py_single_input, globDict, locDict);
+    PyObject* r = PyRun_String(line.c_str(), Py_single_input, globDict, locDict);
     if (r == nullptr) {
         if (PyErr_Occurred()) {
             PyErr_Print();
@@ -403,9 +398,7 @@ bool PyWrapper::exec(const std::string& line, bool debug, std::string& val)
 {
     PyGIL gil;
 
-    auto line_escaped = escapeNewLine(line);
-
-    PyObject* r = PyRun_String(line_escaped.c_str(), Py_eval_input, globDict, locDict);
+    PyObject* r = PyRun_String(line.c_str(), Py_eval_input, globDict, locDict);
     if (r != nullptr) {
         std::string value;
         bool converted = convert(r, value);
@@ -422,7 +415,7 @@ bool PyWrapper::exec(const std::string& line, bool debug, std::string& val)
     PyErr_Clear();
 
     // Still here, let's try executing code instead
-    r = PyRun_String(line_escaped.c_str(), Py_single_input, globDict, locDict);
+    r = PyRun_String(line.c_str(), Py_single_input, globDict, locDict);
     if (r == nullptr) {
         if (PyErr_Occurred()) {
             PyErr_Print();
@@ -440,9 +433,7 @@ bool PyWrapper::exec(const std::string& line, bool debug, std::vector<T>& arr)
     PyGIL gil;
     arr.clear();
 
-    auto line_escaped = escapeNewLine(line);
-
-    PyObject* r = PyRun_String(line_escaped.c_str(), Py_eval_input, globDict, locDict);
+    PyObject* r = PyRun_String(line.c_str(), Py_eval_input, globDict, locDict);
     if (r != nullptr) {
         bool converted = convert(r, arr);
         Py_DecRef(r);
@@ -457,7 +448,7 @@ bool PyWrapper::exec(const std::string& line, bool debug, std::vector<T>& arr)
     PyErr_Clear();
 
     // Still here, let's try executing code instead
-    r = PyRun_String(line_escaped.c_str(), Py_single_input, globDict, locDict);
+    r = PyRun_String(line.c_str(), Py_single_input, globDict, locDict);
     if (r == nullptr) {
         if (PyErr_Occurred()) {
             PyErr_Print();
@@ -470,12 +461,3 @@ bool PyWrapper::exec(const std::string& line, bool debug, std::vector<T>& arr)
 }
 template bool PyWrapper::exec(const std::string&, bool, std::vector<long>&);
 template bool PyWrapper::exec(const std::string&, bool, std::vector<double>&);
-
-std::string PyWrapper::escapeNewLine(const std::string &text)
-{
-    const static std::map<std::string, std::string> newlines = {
-        { "\n", "\\n" },
-        { "\r", "\\r" },
-    };
-    return Util::replace(text, newlines);
-}

src/pywrapper.h

@@ -14,7 +14,6 @@ class PyWrapper {
     private:
         template <typename T> static bool convert(void* in, T& out);
         template <typename T> static bool convert(void* in, std::vector<T>& out);
-        static std::string escapeNewLine(const std::string& text);
     public:
         using Callback = std::function<void()>;
         static bool init();

src/unittest/Makefile

@@ -0,0 +1,23 @@
+# Makefile
+TOP = ../..
+include $(TOP)/configure/CONFIG
+#----------------------------------------
+#  ADD MACRO DEFINITIONS AFTER THIS LINE
+
+USR_LDFLAGS += $(shell $(PYTHON_CONFIG) --ldflags)
+USR_CXXFLAGS += $(subst -Wstrict-prototypes,,$(shell $(PYTHON_CONFIG) --cflags))
+USR_CXXFLAGS += -I$(TOP)/src
+USR_CXXFLAGS += -std=c++11 -g -ggdb -O0
+SRC_DIRS += ../..
+
+PROD_LIBS = Com
+
+TESTPROD_HOST = testutil
+testutil_SRCS += test_util.cpp
+testutil_SRCS += util.cpp
+TESTS += testutil
+
+TESTSCRIPTS_HOST = $(TESTS:%=%.t)
+
+include $(TOP)/configure/RULES
+

src/unittest/test_util.cpp

@@ -0,0 +1,96 @@
+#include <util.h>
+
+#include <epicsUnitTest.h>
+#include <testMain.h>
+
+#include <map>
+#include <string>
+
+struct TestEscape {
+    static void newLine()
+    {
+        testOk1(Util::escape("new\nline") == "new\\nline");
+        testOk1(Util::escape("one\ntwo\nthree") == "one\\ntwo\\nthree");
+    }
+
+    static void singleQuote()
+    {
+        testOk1(Util::escape("single ' quote") == "single \\' quote");
+        testOk1(Util::escape("embedded 'string'") == "embedded \\'string\\'");
+        testOk1(Util::escape("h'ello") == "h\\'ello");
+    }
+};
+
+struct TestReplace {
+    static void basic()
+    {
+        const static std::map<std::string, std::string> fields = {
+            { "%VAL%", "hello" }
+        };
+        testOk1(Util::replace("test string %VAL%", fields) == "test string hello");
+        testOk1(Util::replace("%VAL%", fields) == "hello");
+        testOk1(Util::replace("'%VAL%'", fields) == "'hello'");
+    }
+
+    static void multipleInstances()
+    {
+        const static std::map<std::string, std::string> fields = {
+            { "%VAL%", "hello" }
+        };
+        testOk1(Util::replace("%VAL% %VAL%", fields) == "hello hello");
+        testOk1(Util::replace("%VAL%,%VAL%", fields) == "hello,hello");
+        testOk1(Util::replace("%VAL%/%VAL%/%VAL%", fields) == "hello/hello/hello");
+    }
+
+    static void multipleFields()
+    {
+        const static std::map<std::string, std::string> fields = {
+            { "%VAL%", "hello" },
+            { "%NAME%", "Test:PV" },
+            { "%HIGH%", "5" }
+        };
+        testOk1(Util::replace("test string %VAL%", fields) == "test string hello");
+        testOk1(Util::replace("%NAME%=%VAL%", fields) == "Test:PV=hello");
+        testOk1(Util::replace("%NAME%: VAL=%VAL% HIGH=%HIGH%", fields) == "Test:PV: VAL=hello HIGH=5");
+    }
+
+    static void singleChar()
+    {
+        const static std::map<std::string, std::string> fields = {
+            { "\n", "\\n" },
+            { "a", "b" },
+            { "b", "c" },
+        };
+        testOk1(Util::replace("\n", fields) == "\\n");
+        testOk1(Util::replace("a", fields) == "b");
+        testOk1(Util::replace("aaa", fields) == "bbb");
+        testOk1(Util::replace("aba", fields) == "bcb");
+    }
+
+};
+
+MAIN(testutil)
+{
+    testPlan(18);
+    TestReplace::basic();
+    TestReplace::multipleInstances();
+    TestReplace::multipleFields();
+    TestReplace::singleChar();
+    TestEscape::newLine();
+    TestEscape::singleQuote();
+    /*
+    testSetup();
+    logger_config_env();
+    Tester().loopback(false);
+    Tester().loopback(true);
+    Tester().lazy();
+    Tester().timeout();
+    Tester().cancel();
+    Tester().orphan();
+    TestPutBuilder().testSet();
+    testRO();
+    testError();
+    cleanup_for_valgrind();
+    */
+    return testDone();
+}

src/util.cpp

@@ -28,15 +28,27 @@ std::string replace(const std::string& text, const std::map<std::string, std::st
 {
     std::string out = text;
 
-    for (auto& field: fields) {
-        auto pos = out.find(field.first);
-        while (pos != out.npos) {
-            out.replace(pos, field.first.size(), field.second);
-            pos = out.find(field.first, pos+field.second.size());
+    for (size_t pos = 0; pos < out.length(); pos++) {
+        for (auto& field: fields) {
+            if (out.compare(pos, field.first.length(), field.first) == 0) {
+                out.replace(pos, field.first.length(), field.second);
+                pos += field.second.length() - 1;
+                break;
+            }
         }
     }
 
     return out;
 }
 
+std::string escape(const std::string& text)
+{
+    const static std::map<std::string, std::string> escapables = {
+        { "\n", "\\n" },
+        { "\r", "\\r" },
+        { "'",  "\\'" },
+    };
+    return Util::replace(text, escapables);
+}
+
 }; // namespace Util

src/util.h

@@ -14,6 +14,7 @@ namespace Util {
 
 std::map<std::string, std::string> getReplacables(const std::string& text);
 std::string replace(const std::string& text, const std::map<std::string, std::string>& fields);
+std::string escape(const std::string& text);
 
 template <typename T>
 std::string arrayToStr(const std::vector<T>& val)