From 4b6afde9c8c70c223217ff439e9aa9d2927042b1 Mon Sep 17 00:00:00 2001 From: "Vodopivec, Klemen" Date: Sat, 20 Mar 2021 09:25:43 -0400 Subject: [PATCH] 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 | 1 + README.md | 4 +- src/pydev_lsi.cpp | 2 +- src/pydev_lso.cpp | 2 +- src/pydev_stringin.cpp | 2 +- src/pydev_stringout.cpp | 2 +- src/pywrapper.cpp | 32 +++---------- src/pywrapper.h | 1 - src/unittest/Makefile | 23 +++++++++ src/unittest/test_util.cpp | 96 ++++++++++++++++++++++++++++++++++++++ src/util.cpp | 22 +++++++-- src/util.h | 1 + 12 files changed, 151 insertions(+), 37 deletions(-) create mode 100644 src/unittest/Makefile create mode 100644 src/unittest/test_util.cpp diff --git a/PyDevice.files b/PyDevice.files index 483a2b3..f2d0c47 100644 --- a/PyDevice.files +++ b/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 diff --git a/README.md b/README.md index f730360..1e01baa 100644 --- a/README.md +++ b/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 diff --git a/src/pydev_lsi.cpp b/src/pydev_lsi.cpp index 15b674e..93ba2c8 100644 --- a/src/pydev_lsi.cpp +++ b/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); diff --git a/src/pydev_lso.cpp b/src/pydev_lso.cpp index a01fd4c..cb4bc82 100644 --- a/src/pydev_lso.cpp +++ b/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); diff --git a/src/pydev_stringin.cpp b/src/pydev_stringin.cpp index c137d83..c91abd5 100644 --- a/src/pydev_stringin.cpp +++ b/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); diff --git a/src/pydev_stringout.cpp b/src/pydev_stringout.cpp index d6ddfba..7fac263 100644 --- a/src/pydev_stringout.cpp +++ b/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); diff --git a/src/pywrapper.cpp b/src/pywrapper.cpp index f40eea7..170f3b0 100644 --- a/src/pywrapper.cpp +++ b/src/pywrapper.cpp @@ -17,7 +17,6 @@ static PyObject* locDict = nullptr; static PyThreadState* mainThread = nullptr; static std::map> 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& 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& 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& arr) } template bool PyWrapper::exec(const std::string&, bool, std::vector&); template bool PyWrapper::exec(const std::string&, bool, std::vector&); - -std::string PyWrapper::escapeNewLine(const std::string &text) -{ - const static std::map newlines = { - { "\n", "\\n" }, - { "\r", "\\r" }, - }; - return Util::replace(text, newlines); -} diff --git a/src/pywrapper.h b/src/pywrapper.h index d9dd38f..9a58165 100644 --- a/src/pywrapper.h +++ b/src/pywrapper.h @@ -14,7 +14,6 @@ class PyWrapper { private: template static bool convert(void* in, T& out); template static bool convert(void* in, std::vector& out); - static std::string escapeNewLine(const std::string& text); public: using Callback = std::function; static bool init(); diff --git a/src/unittest/Makefile b/src/unittest/Makefile new file mode 100644 index 0000000..633cb66 --- /dev/null +++ b/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 + diff --git a/src/unittest/test_util.cpp b/src/unittest/test_util.cpp new file mode 100644 index 0000000..364a3e8 --- /dev/null +++ b/src/unittest/test_util.cpp @@ -0,0 +1,96 @@ +#include + +#include +#include + +#include +#include + +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 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 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 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 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(); +} diff --git a/src/util.cpp b/src/util.cpp index 6a29df9..c84542c 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -28,15 +28,27 @@ std::string replace(const std::string& text, const std::map escapables = { + { "\n", "\\n" }, + { "\r", "\\r" }, + { "'", "\\'" }, + }; + return Util::replace(text, escapables); +} + }; // namespace Util diff --git a/src/util.h b/src/util.h index 25afc6e..91dec96 100644 --- a/src/util.h +++ b/src/util.h @@ -14,6 +14,7 @@ namespace Util { std::map getReplacables(const std::string& text); std::string replace(const std::string& text, const std::map& fields); +std::string escape(const std::string& text); template std::string arrayToStr(const std::vector& val)