This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 6294078ed8 fix quoting for execprocess, fixes #5988 (#7840)
6294078ed8 is described below
commit 6294078ed8562a63b1039560223cb3c690774876
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Aug 7 22:31:18 2026 +0200
fix quoting for execprocess, fixes #5988 (#7840)
---
.../pages/pipeline/transforms/execprocess.adoc | 53 ++++++++++-
.../transforms/execprocess/ExecProcess.java | 102 ++++++++++++++++-----
.../execprocess/messages/messages_en_US.properties | 1 +
.../transforms/execprocess/ExecProcessTest.java | 102 ++++++++++++++++++++-
4 files changed, 234 insertions(+), 24 deletions(-)
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/execprocess.adoc
b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/execprocess.adoc
index f5bec93653..a05dce32c9 100644
---
a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/execprocess.adoc
+++
b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/execprocess.adoc
@@ -49,7 +49,11 @@ The transform is similar to the
xref:workflow/actions/shell.adoc[Shell] workflow
|Option|Description
|Transform name|Name of the transform; this name has to be unique in a single
pipeline
|Process field|The field name in the data stream that defines the process to
start (shell script / batch file to start).
-Arguments can also be used.
+Arguments can also be used, see <<command-line-parsing,Command line parsing>>.
+|Arguments in fields|Take the arguments from separate fields in the row
instead of parsing them out of the process field.
+The process field then contains only the executable, see
<<command-line-parsing,Command line parsing>>.
+|Argument fields|The fields holding the arguments, in the order they should be
passed to the process.
+Only available when *Arguments in fields* is enabled.
|Fail if not success|Checking this option means if the exit status is
different than zero the transform fails.
You can use error handling to get these rows.
|Output line delimiter|Without defining a line delimiter, all returned lines
are collapsed into a single string with no line delimiters.
@@ -61,3 +65,50 @@ This field is filled by the error stream (stderr) of the
process.
|Exit value|Specify here the name of the exit fieldname (INTEGER) added to the
output stream of the pipeline.
This field is filled by the exit output of the process.
|===
+
+[#command-line-parsing]
+== Command line parsing
+
+The process is started directly, *not through a shell*.
+This is the single most important thing to know about this transform: no shell
interprets what you type, so none of the following work, no matter how the
command line is written:
+
+* pipes and redirection: `|`, `>`, `>>`, `<`
+* command chaining: `&&`, `||`, `;`
+* wildcard (glob) expansion: `*.csv`, `?`, `[a-z]`
+* environment variable expansion by the shell: `$HOME`, `%PATH%`
+* shell built-ins: `cd`, `export`, `set`, `echo` on Windows
+
+These are passed to the process as literal arguments.
+Hop variables such as `{openvar}PROJECT_HOME{closevar}` *are* resolved,
because Hop resolves them before the process is started.
+
+If you need shell features, run a shell explicitly and pass your command line
to it as an argument, for example `/bin/sh` with arguments `-c` and `ls -la
/tmp | wc -l` (see *Arguments in fields* below), or use the
xref:workflow/actions/shell.adoc[Shell] workflow action which is built for
scripts.
+
+=== Splitting the command line
+
+When *Arguments in fields* is disabled, the value of the process field is
split into the executable and its arguments:
+
+* tokens are separated by whitespace
+* a section wrapped in single or double quotes stays a single token, even when
it contains whitespace, and the quotes are removed
+* quoting is positional, so quoted and unquoted parts that touch each other
end up in the same argument: `--path="/my folder"/x` is one argument,
`--path=/my folder/x`
+* there is no backslash escaping: on Windows a backslash is a path separator,
so `C:\my dir` keeps its backslashes and needs quotes to survive the space
+* quotes only take effect when they are balanced. A command line with an
unclosed quote is split on whitespace only and keeps its quote characters, so a
stray apostrophe such as `--message=don\'t` still reaches the process unchanged
+
+[options="header"]
+|===
+|Process field value|Executable|Arguments
+|`ls -la /work/hop/config`|`ls`|`-la`, `/work/hop/config`
+|`ls -la "/work/hop/config"`|`ls`|`-la`, `/work/hop/config`
+|`ls -la "/work/my folder"`|`ls`|`-la`, `/work/my folder`
+|`"C:\Program Files\tool.exe" -v`|`C:\Program Files\tool.exe`|`-v`
+|`echo --message=don\'t`|`echo`|`--message=don\'t`
+|===
+
+NOTE: Quote handling was added in Hop 2.19.
+Up to 2.18 the command line was always split on whitespace only, so `ls -la
"/work/hop/config"` failed with a "does not exist" error on the quoted path.
+If you have a pipeline that relies on quote characters reaching the process
as-is - a JSON argument such as `--json={"a":1}` for example - switch it to
*Arguments in fields*, where nothing is stripped.
+
+=== Arguments in fields
+
+Enable *Arguments in fields* when you would rather not rely on quoting at all,
or when an argument contains characters that are awkward to quote.
+The process field then holds only the executable and each *Argument fields*
entry is passed as one argument exactly as it is, with no splitting and no
quote removal.
+This is the most reliable option for values that come from your data, such as
file paths built at runtime.
diff --git
a/plugins/transforms/execprocess/src/main/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcess.java
b/plugins/transforms/execprocess/src/main/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcess.java
index 05dfd5b292..a859019256 100644
---
a/plugins/transforms/execprocess/src/main/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcess.java
+++
b/plugins/transforms/execprocess/src/main/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcess.java
@@ -23,7 +23,6 @@ import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.StringTokenizer;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.CountDownLatch;
@@ -98,7 +97,11 @@ public class ExecProcess extends
BaseTransform<ExecProcessMeta, ExecProcessData>
execProcess(cmdArray.toArray(new String[0]), processResult);
} else {
- execProcess(processString, processResult);
+ if (isDetailed() && !hasBalancedQuotes(processString)) {
+ logDetailed(
+ BaseMessages.getString(PKG, "ExecProcess.Log.UnbalancedQuotes",
processString));
+ }
+ execProcess(tokenizeCommandLine(processString), processResult);
}
if (meta.isFailWhenNotSuccess() && processResult.getExitValue() != 0) {
@@ -207,10 +210,6 @@ public class ExecProcess extends
BaseTransform<ExecProcessMeta, ExecProcessData>
}
}
- private void execProcess(String process, ProcessResult processresult) throws
HopException {
- execProcess(new String[] {process}, processresult);
- }
-
private void execProcess(String[] process, ProcessResult processresult)
throws HopException {
Process p = null;
waitForLatch = new CountDownLatch(1);
@@ -218,14 +217,7 @@ public class ExecProcess extends
BaseTransform<ExecProcessMeta, ExecProcessData>
String errorMsg = null;
// execute process
try {
- if (!meta.isArgumentsInFields()) {
- // Match historical Runtime.exec(String) whitespace tokenization
without using the
- // deprecated String overload. A single-element String[] would treat
the whole command
- // line (e.g. "/bin/echo hop-single") as the executable name.
- p = data.runtime.exec(tokenizeCommandLine(process[0]));
- } else {
- p = data.runtime.exec(process);
- }
+ p = data.runtime.exec(process);
} catch (Exception e) {
errorMsg = e.getMessage();
}
@@ -328,16 +320,82 @@ public class ExecProcess extends
BaseTransform<ExecProcessMeta, ExecProcessData>
}
/**
- * Tokenize a command line the same way Runtime.exec(String) historically
did (whitespace via
- * {@link StringTokenizer}).
+ * Split a command line into the executable and its arguments. Tokens are
separated by whitespace,
+ * but a section wrapped in single or double quotes is kept together as one
token even when it
+ * contains whitespace, and the quotes themselves are removed - the way a
shell would do it.
+ *
+ * <p>Quoting is positional, so quoted and unquoted parts that touch each
other end up in the same
+ * token: {@code --path="/my folder"} yields the single argument {@code
--path=/my folder}. There
+ * is no backslash escaping: on Windows a backslash is a path separator, so
{@code C:\my dir}
+ * keeps its backslashes and needs quotes to survive the space.
+ *
+ * <p>Quotes only take effect when they are balanced, see {@link
#hasBalancedQuotes(String)}.
+ */
+ static String[] tokenizeCommandLine(String command) throws HopException {
+ List<String> tokens = tokenize(command, hasBalancedQuotes(command));
+ if (tokens.isEmpty()) {
+ throw new HopException(BaseMessages.getString(PKG,
"ExecProcess.ProcessEmpty"));
+ }
+ return tokens.toArray(new String[0]);
+ }
+
+ /**
+ * Whether every single and double quote in the command line is closed again.
+ *
+ * <p>When they are not, we can't tell a mis-typed quote from a quote
character that is simply
+ * part of an argument, so the command line is split on whitespace only and
the quotes are left in
+ * place - what every Hop version up to 2.18 did with any command line. That
keeps a stray
+ * apostrophe ({@code --message=don't}) working instead of turning it into a
failing row.
*/
- static String[] tokenizeCommandLine(String command) {
- StringTokenizer st = new StringTokenizer(command);
- String[] cmdArray = new String[st.countTokens()];
- for (int i = 0; st.hasMoreTokens(); i++) {
- cmdArray[i] = st.nextToken();
+ static boolean hasBalancedQuotes(String command) {
+ char quote = 0;
+ for (int i = 0; i < command.length(); i++) {
+ char c = command.charAt(i);
+ if (quote != 0) {
+ if (c == quote) {
+ quote = 0;
+ }
+ } else if (c == '\'' || c == '"') {
+ quote = c;
+ }
+ }
+ return quote == 0;
+ }
+
+ private static List<String> tokenize(String command, boolean respectQuotes) {
+ List<String> tokens = new ArrayList<>();
+ StringBuilder token = new StringBuilder();
+ boolean inToken = false;
+ char quote = 0;
+
+ for (int i = 0; i < command.length(); i++) {
+ char c = command.charAt(i);
+ if (quote != 0) {
+ // Inside quotes everything is literal until the matching quote shows
up
+ if (c == quote) {
+ quote = 0;
+ } else {
+ token.append(c);
+ }
+ } else if (respectQuotes && (c == '\'' || c == '"')) {
+ quote = c;
+ inToken = true;
+ } else if (Character.isWhitespace(c)) {
+ if (inToken) {
+ tokens.add(token.toString());
+ token.setLength(0);
+ inToken = false;
+ }
+ } else {
+ token.append(c);
+ inToken = true;
+ }
+ }
+
+ if (inToken) {
+ tokens.add(token.toString());
}
- return cmdArray;
+ return tokens;
}
private String getOutputString(BufferedReader b) throws IOException {
diff --git
a/plugins/transforms/execprocess/src/main/resources/org/apache/hop/pipeline/transforms/execprocess/messages/messages_en_US.properties
b/plugins/transforms/execprocess/src/main/resources/org/apache/hop/pipeline/transforms/execprocess/messages/messages_en_US.properties
index 593cd3b62d..d940a4430d 100644
---
a/plugins/transforms/execprocess/src/main/resources/org/apache/hop/pipeline/transforms/execprocess/messages/messages_en_US.properties
+++
b/plugins/transforms/execprocess/src/main/resources/org/apache/hop/pipeline/transforms/execprocess/messages/messages_en_US.properties
@@ -22,6 +22,7 @@ ExecProcess.Error.ResultFieldMissing=Result fieldname is
missing\!
ExecProcess.ErrorInTransformRunning=Error running transform\!
ExecProcess.Exception.CouldNotFindField=Couldn''t find field ''{0}'' in row\!
ExecProcess.LineNumber=linenr {0}
+ExecProcess.Log.UnbalancedQuotes=Unbalanced quote in command line ''{0}'',
splitting on whitespace only and keeping the quotes.
ExecProcess.Name=Execute a process
ExecProcess.ProcessEmpty=The process to execute is empty\!
ExecProcessDialog.ArgumentField.Label=Field Name
diff --git
a/plugins/transforms/execprocess/src/test/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcessTest.java
b/plugins/transforms/execprocess/src/test/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcessTest.java
index 832673cbcd..9046054115 100644
---
a/plugins/transforms/execprocess/src/test/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcessTest.java
+++
b/plugins/transforms/execprocess/src/test/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcessTest.java
@@ -218,11 +218,111 @@ class ExecProcessTest {
}
@Test
- void tokenizeCommandLine_splitsOnWhitespaceLikeRuntimeExecString() {
+ void tokenizeCommandLine_splitsOnWhitespace() throws HopException {
assertEquals(
Arrays.asList("/bin/echo", "hop-single"),
Arrays.asList(ExecProcess.tokenizeCommandLine("/bin/echo
hop-single")));
assertEquals(List.of("cmd"),
Arrays.asList(ExecProcess.tokenizeCommandLine("cmd")));
+ assertEquals(
+ Arrays.asList("ls", "-la", "/work/hop/config"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine(" ls\t-la
/work/hop/config ")));
+ }
+
+ /** Regression for issue #5988 : quoted arguments must survive as a single,
unquoted token. */
+ @Test
+ void tokenizeCommandLine_keepsQuotedSectionsTogetherAndStripsQuotes() throws
HopException {
+ List<String> expected = Arrays.asList("ls", "-la", "/work/hop/config");
+
+ assertEquals(
+ expected, Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la
/work/hop/config")));
+ assertEquals(
+ expected, Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la
\"/work/hop/config\"")));
+ assertEquals(
+ expected, Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la
'/work/hop/config'")));
+ }
+
+ @Test
+ void tokenizeCommandLine_quotedWhitespaceStaysInOneArgument() throws
HopException {
+ assertEquals(
+ Arrays.asList("ls", "-la", "/work/my folder"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la \"/work/my
folder\"")));
+ assertEquals(
+ Arrays.asList("ls", "-la", "/work/my folder"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la '/work/my
folder'")));
+ }
+
+ @Test
+ void tokenizeCommandLine_handlesPartialQuotingAndNestedQuoteCharacters()
throws HopException {
+ // Quoting is positional : quoted and unquoted parts that touch stay in
the same token
+ assertEquals(
+ Arrays.asList("cmd", "--path=/my folder/x"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("cmd --path=\"/my
folder\"/x")));
+ // The other quote character is literal inside quotes
+ assertEquals(
+ Arrays.asList("echo", "it's here"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("echo \"it's here\"")));
+ assertEquals(
+ Arrays.asList("echo", "say \"hi\""),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("echo 'say \"hi\"'")));
+ // An empty quoted section is a real (empty) argument
+ assertEquals(
+ Arrays.asList("cmd", "", "x"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("cmd \"\" x")));
+ }
+
+ /**
+ * An unbalanced quote falls back to the pre-2.19 behaviour - split on
whitespace, keep the quotes
+ * - so a stray apostrophe in an argument keeps working instead of failing
the row.
+ */
+ @Test
+ void tokenizeCommandLine_unbalancedQuoteFallsBackToWhitespaceSplitting()
throws HopException {
+ assertEquals(
+ Arrays.asList("echo", "--message=don't", "x"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("echo --message=don't
x")));
+ assertEquals(
+ Arrays.asList("ls", "-la", "\"/work/my", "folder"),
+ Arrays.asList(ExecProcess.tokenizeCommandLine("ls -la \"/work/my
folder")));
+ }
+
+ @Test
+ void hasBalancedQuotes_detectsUnclosedQuotes() {
+ assertTrue(ExecProcess.hasBalancedQuotes("ls -la \"/work/hop/config\""));
+ assertTrue(ExecProcess.hasBalancedQuotes("echo 'it\"s'"));
+ assertTrue(ExecProcess.hasBalancedQuotes("ls -la /work/hop/config"));
+ assertFalse(ExecProcess.hasBalancedQuotes("ls -la \"/work/hop"));
+ assertFalse(ExecProcess.hasBalancedQuotes("echo don't"));
+ }
+
+ @Test
+ void tokenizeCommandLine_blankCommandThrows() {
+ assertThrows(HopException.class, () -> ExecProcess.tokenizeCommandLine("
"));
+ }
+
+ @Test
+ @EnabledOnOs({OS.LINUX, OS.MAC})
+ void processRow_singleCommandString_quotedArgumentWithSpaces() throws
HopException {
+ HopEnvironment.init();
+
+ ExecProcessMeta meta = new ExecProcessMeta();
+ meta.setDefault();
+ meta.setProcessField("cmdline");
+ meta.setResultFieldName("result_out");
+ meta.setErrorFieldName("result_err");
+ meta.setExitValueFieldName("result_exit");
+ meta.setArgumentsInFields(false);
+
+ RowMeta rowMeta = new RowMeta();
+ rowMeta.addValueMeta(new ValueMetaString("cmdline"));
+
+ Object[] row = new Object[] {"/bin/echo \"hop single\" 'and more'"};
+ ExecProcess transform = newTransform(meta, rowMeta, row);
+
+ assertTrue(transform.processRow());
+ assertFalse(transform.processRow());
+
+ Object[] out = readSingleOutputRow(transform);
+ assertEquals("hop single and more", out[1].toString().trim());
+ assertEquals(0L, out[3]);
}
@Test
