Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#990:Implemented escape sequence for jar option with trailing whitespaces #472

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions exaudfclient/base/javacontainer/script_options/converter.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include "base/javacontainer/script_options/converter.h"
#include "base/javacontainer/script_options/string_ops.h"

#include <iostream>

namespace SWIGVMContainers {
Expand All @@ -11,11 +11,11 @@ Converter::Converter()
, m_whitespace(" \t\f\v") {}

void Converter::convertScriptClassName(const std::string & value) {
std::string trimmedValue(value);
StringOps::trim(trimmedValue);
if (value != "") {
m_jvmOptions.push_back("-Dexasol.scriptclass=" + trimmedValue);

if (value.empty()) {
return;
}
m_jvmOptions.push_back("-Dexasol.scriptclass=" + value);
}

void Converter::convertJvmOption(const std::string & value) {
Expand Down
13 changes: 11 additions & 2 deletions exaudfclient/base/javacontainer/script_options/parser_ctpg.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "base/utils/exceptions.h"
#include "base/script_options_parser/exception.h"
#include "base/swig_factory/swig_factory.h"
#include "base/javacontainer/script_options/string_ops.h"
#include <sstream>


Expand All @@ -24,7 +25,11 @@ void ScriptOptionLinesParserCTPG::prepareScriptCode(const std::string & scriptCo

void ScriptOptionLinesParserCTPG::parseForScriptClass(std::function<void(const std::string &option)> callback) {
try {
parseForSingleOption(m_keywords.scriptClassKeyword(), callback);
parseForSingleOption(m_keywords.scriptClassKeyword(), [&](const std::string &option) {
std::string trimmedValue(option);
StringOps::trim(trimmedValue);
callback(trimmedValue);
});
} catch(const ExecutionGraph::OptionParserException& ex) {
Utils::rethrow(ex, "F-UDF-CL-SL-JAVA-1623");
}
Expand All @@ -40,7 +45,11 @@ void ScriptOptionLinesParserCTPG::parseForJvmOptions(std::function<void(const st

void ScriptOptionLinesParserCTPG::parseForExternalJars(std::function<void(const std::string &option)> callback) {
try {
parseForMultipleOption(m_keywords.jarKeyword(), callback);
parseForMultipleOption(m_keywords.jarKeyword(), [callback] (const std::string &option) {
std::string formattedValue(option);
StringOps::replaceTrailingEscapeWhitespaces(formattedValue);
callback(formattedValue);
});
} catch(const ExecutionGraph::OptionParserException& ex) {
Utils::rethrow(ex, "F-UDF-CL-SL-JAVA-1625");
}
Expand Down
77 changes: 77 additions & 0 deletions exaudfclient/base/javacontainer/script_options/string_ops.cc
Original file line number Diff line number Diff line change
@@ -1 +1,78 @@
#include "base/javacontainer/script_options/string_ops.h"
#include <regex>
#include <iostream>

namespace SWIGVMContainers {

namespace JavaScriptOptions {

namespace StringOps {

inline uint32_t countBackslashesBackwards(const std::string & s, size_t pos) {
uint32_t retVal(0);
while (pos >= 0 && s.at(pos--) == '\\') retVal++;
return retVal;
}

inline size_t replaceOnlyBackslashSequencesButKeepChar(std::string & s, size_t backslashStartIdx, size_t nBackslashes) {
const uint32_t nHalfBackslashes = (nBackslashes>>1);
if (nHalfBackslashes > 0) {
s = s.erase(backslashStartIdx, nHalfBackslashes );
}
const size_t newBackslashEndIdx = backslashStartIdx + nHalfBackslashes;
return newBackslashEndIdx + 1; //+1 because of the last None-Whitespace character we need to keep
}

inline size_t replaceBackslashSequencesAndWhitespaceSequence(std::string & s, size_t backslashStartIdx,
size_t nBackslashes, const char* replacement) {
const uint32_t nHalfBackslashes = (nBackslashes>>1);
s = s.erase(backslashStartIdx, nHalfBackslashes+1 ); //Delete also backslash of whitespace escape sequence
const size_t newBackslashEndIdx = backslashStartIdx + nHalfBackslashes;
s = s.replace(newBackslashEndIdx, 1, replacement);
return newBackslashEndIdx + 1; //+1 because of the replaced whitespace character
}

inline size_t replaceCharAtPositionAndBackslashes(std::string & s, size_t pos, const char* replacement) {
const size_t nBackslashes = (pos > 0) ? countBackslashesBackwards(s, pos-1) : 0;

const size_t backslashStartIdx = pos-nBackslashes;
if(nBackslashes % 2 == 0) {
return replaceOnlyBackslashSequencesButKeepChar(s, backslashStartIdx, nBackslashes);
}
else {
return replaceBackslashSequencesAndWhitespaceSequence(s, backslashStartIdx, nBackslashes, replacement);
}
}

void replaceTrailingEscapeWhitespaces(std::string & s) {
if (s.size() > 0) {
const size_t lastNoneWhitespaceIdx = s.find_last_not_of(" \t\v\f");
if (lastNoneWhitespaceIdx != std::string::npos) {
size_t firstWhitespaceAfterNoneWhitespaceIdx = lastNoneWhitespaceIdx + 1;
if (s.size() > 1) {
if(s[lastNoneWhitespaceIdx] == 't') {
firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx, "\t");
} else if (s[lastNoneWhitespaceIdx] == '\\' && lastNoneWhitespaceIdx < (s.size() - 1) && s.at(lastNoneWhitespaceIdx+1) == ' ') {
firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx+1, " ");
} else if (s[lastNoneWhitespaceIdx] == 'f') {
firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx, "\f");
} else if (s[lastNoneWhitespaceIdx] == 'v') {
firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx, "\v");
}
}
if (firstWhitespaceAfterNoneWhitespaceIdx != std::string::npos &&
firstWhitespaceAfterNoneWhitespaceIdx < s.size()) {
s = s.substr(0, firstWhitespaceAfterNoneWhitespaceIdx); //Right Trim the string
}
} else {
s = "";
}
}
}

} //namespace StringOps


} //namespace JavaScriptOptions

} //namespace SWIGVMContainers
2 changes: 2 additions & 0 deletions exaudfclient/base/javacontainer/script_options/string_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ inline void trim(std::string &s) {
rtrim(s);
}

void replaceTrailingEscapeWhitespaces(std::string & s);

} //namespace StringOps


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class ConverterV2JarTest : public ::testing::TestWithParam<std::pair<std::string
TEST_P(ConverterV2JarTest, jar) {
const std::pair<std::string, std::vector<std::string>> option_value = GetParam();
const std::string jar_option_value = option_value.first;
std::cerr << "DEBUG: " << jar_option_value << std::endl;

ConverterV2 converter;
converter.convertExternalJar(option_value.first);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,35 @@ TEST(StringOpsTest, trimWithNoneASCII) {
EXPECT_EQ(sample, "\xa0Hello World\xa0");
}

class ReplaceTrailingEscapeWhitespacesTest : public ::testing::TestWithParam<std::pair<std::string, std::string>> {};

TEST_P(ReplaceTrailingEscapeWhitespacesTest, s) {
const std::pair<std::string, std::string> underTest = GetParam();

std::string str = underTest.first;
StringOps::replaceTrailingEscapeWhitespaces(str);
ASSERT_EQ(str, underTest.second);
}

const std::vector<std::pair<std::string, std::string>> replace_trailing_escape_whitespaces_strings =
{
std::make_pair("hello world", std::string("hello world")),
std::make_pair("hello world ", std::string("hello world")),
std::make_pair("hello world\\t", std::string("hello world\t")),
std::make_pair("hello world\\f", std::string("hello world\f")),
std::make_pair("hello world\\v", std::string("hello world\v")),
std::make_pair("hello world\\\\t", std::string("hello world\\t")),
std::make_pair("hello world\\\\t\t", std::string("hello world\\t")),
std::make_pair("hello world\\\\\\t\t", std::string("hello world\\\t")),
std::make_pair("hello world\\\\\\\\t\t", std::string("hello world\\\\t")),
std::make_pair("hello worl\td\\\\\\\\t\t", std::string("hello worl\td\\\\t")),
std::make_pair("t\t ", std::string("t")),
std::make_pair("hello worl\td\\\\\\\\", std::string("hello worl\td\\\\\\\\")), //If no whitespace escape sequence, backslashes are not interpreted as backslash escape sequence
};


INSTANTIATE_TEST_SUITE_P(
StringOpsTest,
ReplaceTrailingEscapeWhitespacesTest,
::testing::ValuesIn(replace_trailing_escape_whitespaces_strings)
);
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,19 @@ TEST(JavaContainer, import_script_with_escaped_options) {
"-XX:+UseSerialGC" };
EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions);
}

TEST(JavaContainer, basic_jar_with_trailing_escape) {
const std::string script_code = "%scriptclass com.exasol.udf_profiling.UdfProfiler;\n"
"%jar base/javacontainer/test/test.jar\\t\t;";
EXPECT_THROW({
try
{
JavaVMTest vm(script_code);
}
catch( const SWIGVMContainers::JavaVMach::exception& e )
{
EXPECT_THAT( e.what(), MatchesRegex("^.*Java VM cannot find 'base/javacontainer/test/test\\.jar\t': No such file or directory$"));
throw;
}
}, SWIGVMContainers::JavaVMach::exception );
}
14 changes: 0 additions & 14 deletions exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,8 @@ TEST(JavaContainer, basic_jar_script_class_with_white_spaces) {
TEST(JavaContainer, basic_jar_with_white_spaces) {
const std::string script_code = "%jar base/javacontainer/test/test.jar \t ;";

#ifndef USE_EXTRACTOR_V2 //The parsers behave differently: The legacy parser removes trailing white spaces.
JavaVMTest vm(script_code);
EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/test.jar");
#else
EXPECT_THROW({
try
{
JavaVMTest vm(script_code);
}
catch( const SWIGVMContainers::JavaVMach::exception& e )
{
EXPECT_THAT( e.what(), MatchesRegex("^.*Java VM cannot find 'base/javacontainer/test/test\\.jar \t ': No such file or directory$"));
throw;
}
}, SWIGVMContainers::JavaVMach::exception );
#endif
}

TEST(JavaContainer, basic_jars_ordering) {
Expand Down
Loading