#878: Fixed jvmoption and parameters containing a space

exasol · Oct 29, 2024 · 2eb480b · 2eb480b
1 parent cf43d08
commit 2eb480b
Show file tree

Hide file tree

Showing 7 changed files with 190 additions and 19 deletions.
diff --git a/exaudfclient/base/javacontainer/script_options/converter.cc b/exaudfclient/base/javacontainer/script_options/converter.cc
@@ -18,21 +18,6 @@ void Converter::convertScriptClassName(const std::string & value) {
     m_jvmOptions.push_back("-Dexasol.scriptclass=" + value);
 }
 
-void Converter::convertJvmOption(const std::string & value) {
-    for (size_t start = 0, delim = 0; ; start = delim + 1) {
-        start = value.find_first_not_of(m_whitespace, start);
-        if (start == std::string::npos)
-            break;
-        delim = value.find_first_of(m_whitespace, start);
-        if (delim != std::string::npos) {
-            m_jvmOptions.push_back(value.substr(start, delim - start));
-        }
-        else {
-            m_jvmOptions.push_back(value.substr(start));
-            break;
-        }
-    }
-}
 
 
 } //namespace JavaScriptOptions

diff --git a/exaudfclient/base/javacontainer/script_options/converter.h b/exaudfclient/base/javacontainer/script_options/converter.h
@@ -23,7 +23,7 @@ class Converter {
 
         void convertScriptClassName(const std::string & value);
 
-        void convertJvmOption(const std::string & value);
+        virtual void convertJvmOption(const std::string & value) = 0;
 
         std::vector<std::string>&& moveJvmOptions() {
             return std::move(m_jvmOptions);
@@ -33,7 +33,7 @@ class Converter {
 
         virtual void iterateJarPaths(tJarIteratorCallback callback) const = 0;
 
-    private:
+    protected:
 
         std::vector<std::string> m_jvmOptions;
 

diff --git a/exaudfclient/base/javacontainer/script_options/converter_legacy.cc b/exaudfclient/base/javacontainer/script_options/converter_legacy.cc
@@ -21,6 +21,23 @@ void ConverterLegacy::convertExternalJar(const std::string& value) {
     }
 }
 
+void ConverterLegacy::convertJvmOption(const std::string & value) {
+    for (size_t start = 0, delim = 0; ; start = delim + 1) {
+        start = value.find_first_not_of(m_whitespace, start);
+        if (start == std::string::npos)
+            break;
+        delim = value.find_first_of(m_whitespace, start);
+        if (delim != std::string::npos) {
+            m_jvmOptions.push_back(value.substr(start, delim - start));
+        }
+        else {
+            m_jvmOptions.push_back(value.substr(start));
+            break;
+        }
+    }
+}
+
+
 void ConverterLegacy::iterateJarPaths(Converter::tJarIteratorCallback callback) const {
     std::for_each(m_jarPaths.begin(), m_jarPaths.end(), callback);
 }

diff --git a/exaudfclient/base/javacontainer/script_options/converter_legacy.h b/exaudfclient/base/javacontainer/script_options/converter_legacy.h
@@ -24,7 +24,9 @@ class ConverterLegacy : public Converter {
     public:
         ConverterLegacy();
 
-        void convertExternalJar(const std::string & value);
+        void convertExternalJar(const std::string & value) override;
+
+        void convertJvmOption(const std::string & value)  override;
 
         void iterateJarPaths(tJarIteratorCallback callback) const override;
 

diff --git a/exaudfclient/base/javacontainer/script_options/converter_v2.cc b/exaudfclient/base/javacontainer/script_options/converter_v2.cc
@@ -21,6 +21,144 @@ void ConverterV2::convertExternalJar(const std::string & value) {
     }
 }
 
+class ProcessJvmOptionsStatemachine {
+    public:
+        typedef std::function<void(const std::string &jvmoption)> tOptionFoundCallback;
+        ProcessJvmOptionsStatemachine(tOptionFoundCallback cb, const std::string &whitespaces);
+
+        void processCharacter(const char current_char);
+
+        void finish();
+
+    private:
+        void processWhitespace (const char current_char);
+        void processNormalCharacter(const char current_char);
+        void processBackslash(const char current_char);
+        enum ParserState {
+            START = 0,
+            IN_VALUE = 1,
+            IN_ESCAPE = 2,
+            IN_WHITESPACE = 3
+        };
+
+        ParserState m_state;
+
+        tOptionFoundCallback m_optionsFoundCallback;
+
+        const std::string &m_whitespaces;
+
+        std::string m_currentOption;
+};
+
+ProcessJvmOptionsStatemachine::ProcessJvmOptionsStatemachine(tOptionFoundCallback cb, const std::string &whitespaces)
+: m_state(START)
+, m_optionsFoundCallback(cb) 
+, m_whitespaces(whitespaces)
+, m_currentOption() {}
+
+void ProcessJvmOptionsStatemachine::processCharacter(const char current_char) {
+    if (current_char == '\\') {
+        processBackslash(current_char);
+    }
+    else if (m_whitespaces.find(current_char) != std::string::npos) {
+        processWhitespace(current_char);
+    }
+    else {
+        processNormalCharacter(current_char);
+    }
+}
+
+
+void ProcessJvmOptionsStatemachine::processWhitespace (const char current_char) {
+    switch (m_state) {
+        case START:
+            break;
+        case IN_VALUE:
+            m_state = IN_WHITESPACE;
+            m_optionsFoundCallback(m_currentOption);
+            m_currentOption.clear();
+            break;
+        case IN_ESCAPE:
+            if (current_char == ' ') {
+                m_currentOption.push_back(current_char);
+                m_state = IN_VALUE;
+            } else {
+                std::stringstream ss;
+                ss << "Unexpected escape sequence. Invalid whitespace character '" << current_char << "'";
+                throw std::invalid_argument(ss.str());
+            }
+            break;
+        case IN_WHITESPACE:
+            break;
+        }
+}
+
+void ProcessJvmOptionsStatemachine::processNormalCharacter(const char current_char) {
+    switch (m_state) {
+        case START:
+            m_currentOption.push_back(current_char);
+            m_state = IN_VALUE;
+            break;
+        case IN_VALUE:
+            m_currentOption.push_back(current_char);
+            break;
+        case IN_ESCAPE:
+            if (current_char == 't') {
+                m_currentOption.push_back('\t');
+                m_state = IN_VALUE;
+            } else if (current_char == 'f') {
+                m_currentOption.push_back('\f');
+                m_state = IN_VALUE;
+            } else if (current_char == 'v') {
+                m_currentOption.push_back('\v');
+                m_state = IN_VALUE;
+            } else {
+                std::stringstream ss;
+                ss << "Invalid escape sequence at character '" << current_char << "'";
+                throw std::invalid_argument(ss.str());
+            }
+            break;
+        case IN_WHITESPACE:
+            m_currentOption.push_back(current_char);
+            m_state = IN_VALUE;
+            break;
+        }
+}
+
+void ProcessJvmOptionsStatemachine::processBackslash(const char current_char) {
+    switch (m_state) {
+        case START:
+        case IN_VALUE:
+        case IN_WHITESPACE:
+            m_state = IN_ESCAPE;
+            break;
+        case IN_ESCAPE:
+            m_currentOption.push_back(current_char);
+            m_state = IN_VALUE;
+            break;
+        }
+}
+
+void ProcessJvmOptionsStatemachine::finish() {
+    if (m_currentOption.size() > 0) {
+        m_optionsFoundCallback(m_currentOption);
+    }
+}
+
+
+void ConverterV2::convertJvmOption(const std::string & value) {
+    auto optionFoundCallback = [&] (const std::string & jvmoption) {m_jvmOptions.push_back(jvmoption);};
+    ProcessJvmOptionsStatemachine sm(optionFoundCallback, m_whitespace);
+    try {
+        std::for_each(value.cbegin(), value.cend(), [&sm](const char c) {sm.processCharacter(c);});
+    } catch(const std::invalid_argument & e) {
+        std::stringstream ss;
+        ss << "F-UDF-CL-SL-JAVA-1629 " << "Error parsing jvmoption: " << value << " - " << e.what();
+        throw std::invalid_argument(ss.str());
+    }
+      sm.finish();
+}
+
 void ConverterV2::iterateJarPaths(Converter::tJarIteratorCallback callback) const {
     std::for_each(m_jarPaths.begin(), m_jarPaths.end(), callback);
 }

diff --git a/exaudfclient/base/javacontainer/script_options/converter_v2.h b/exaudfclient/base/javacontainer/script_options/converter_v2.h
@@ -23,7 +23,9 @@ class ConverterV2 : public Converter {
     public:
         ConverterV2();
 
-        void convertExternalJar(const std::string & value);
+        void convertExternalJar(const std::string & value) override;
+
+        void convertJvmOption(const std::string & value)  override;
 
         void iterateJarPaths(tJarIteratorCallback callback) const override;
 

diff --git a/exaudfclient/base/javacontainer/script_options/test/converter_test.cc b/exaudfclient/base/javacontainer/script_options/test/converter_test.cc
@@ -60,3 +60,30 @@ INSTANTIATE_TEST_SUITE_P(
     ConverterV2JarTest,
     ::testing::ValuesIn(jar_strings_v2)
 );
+
+class ConverterV2JvmOptionsTest : public ::testing::TestWithParam<std::pair<std::string, std::vector<std::string>>> {};
+
+TEST_P(ConverterV2JvmOptionsTest, jvm_option) {
+    const std::pair<std::string, std::vector<std::string>> option_value = GetParam();
+    const std::string jvm_option_value = option_value.first;
+
+    ConverterV2 converter;
+    converter.convertJvmOption(option_value.first);
+    std::vector<std::string> result = std::move(converter.moveJvmOptions());
+    ASSERT_EQ(result, option_value.second);
+}
+
+const std::vector<std::pair<std::string, std::vector<std::string>>> jvm_options_strings_v2 =
+        {
+            std::make_pair("optionA=abc optionB=def", std::vector<std::string>({"optionA=abc", "optionB=def"})), //basic splitting
+            std::make_pair("optionA=abc\\ def optionB=ghi", std::vector<std::string>({"optionA=abc def", "optionB=ghi"})), //with escaped whitespace
+            std::make_pair("optionA=abc\\tdef optionB=ghi", std::vector<std::string>({"optionA=abc\tdef", "optionB=ghi"})), //with escaped whitespace
+            std::make_pair("   optionA=abc\\tdef optionB=ghi", std::vector<std::string>({"optionA=abc\tdef", "optionB=ghi"})), //with escaped whitespace
+            std::make_pair("   \\\\optionA=abc\\tdef optionB=ghi", std::vector<std::string>({"\\optionA=abc\tdef", "optionB=ghi"})), //with escaped whitespace
+        };
+
+INSTANTIATE_TEST_SUITE_P(
+    Converter,
+    ConverterV2JvmOptionsTest,
+    ::testing::ValuesIn(jvm_options_strings_v2)
+);