This is an automated email from the ASF dual-hosted git repository.
bowenliang pushed a commit to branch branch-1.10
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/branch-1.10 by this push:
new fdabc36671 [KYUUBI #6772] Fix ProcessBuilder to properly handle Java
opts as a list
fdabc36671 is described below
commit fdabc36671842708a39b546e7c0c379eab352eef
Author: senmiaoliu <[email protected]>
AuthorDate: Wed Oct 23 13:28:58 2024 +0800
[KYUUBI #6772] Fix ProcessBuilder to properly handle Java opts as a list
# :mag: Description
## Issue References ๐
## Describe Your Solution ๐ง
This PR addresses an issue in the ProcessBuilder class where Java options
passed as a single string (e.g., "-Dxxx -Dxxx") do not take effect. The command
list must separate these options into individual elements to ensure they are
recognized correctly by the Java runtime.
## Types of changes :bookmark:
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
## Test Plan ๐งช
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist ๐
- [x] This patch was not authored or co-authored using [Generative
Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
Closes #6772 from lsm1/branch-fix-processBuilder.
Closes #6772
fb6d53234 [senmiaoliu] fix process builder java opts
Authored-by: senmiaoliu <[email protected]>
Signed-off-by: Bowen Liang <[email protected]>
(cherry picked from commit f876600c4a95af988949734d566d34126843777a)
Signed-off-by: Bowen Liang <[email protected]>
---
.../kyuubi/engine/flink/FlinkProcessBuilder.scala | 3 +-
.../kyuubi/engine/hive/HiveProcessBuilder.scala | 2 +-
.../kyuubi/engine/jdbc/JdbcProcessBuilder.scala | 5 +-
.../kyuubi/engine/trino/TrinoProcessBuilder.scala | 2 +-
.../kyuubi/util/command/CommandLineUtils.scala | 86 ++++++++++++++++++++++
.../kyuubi/util/command/CommandUtilsSuite.scala | 7 ++
6 files changed, 99 insertions(+), 6 deletions(-)
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
index 9f6e7e68c1..e5ead4e53d 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala
@@ -198,9 +198,8 @@ class FlinkProcessBuilder(
buffer += s"-Xmx$memory"
val javaOptions =
conf.get(ENGINE_FLINK_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
if (javaOptions.isDefined) {
- buffer += javaOptions.get
+ buffer ++= parseOptionString(javaOptions.get)
}
-
val classpathEntries = new mutable.LinkedHashSet[String]
// flink engine runtime jar
mainResource.foreach(classpathEntries.add)
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
index 8fc14d4ca5..bbdf316445 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala
@@ -65,7 +65,7 @@ class HiveProcessBuilder(
buffer += s"-Xmx$memory"
val javaOptions =
conf.get(ENGINE_HIVE_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
if (javaOptions.isDefined) {
- buffer += javaOptions.get
+ buffer ++= parseOptionString(javaOptions.get)
}
// -Xmx5g
// java options
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/jdbc/JdbcProcessBuilder.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/jdbc/JdbcProcessBuilder.scala
index 6d7b66aac0..9e76afa190 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/jdbc/JdbcProcessBuilder.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/jdbc/JdbcProcessBuilder.scala
@@ -73,8 +73,9 @@ class JdbcProcessBuilder(
buffer += s"-Xmx$memory"
val javaOptions =
conf.get(ENGINE_JDBC_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
- javaOptions.foreach(buffer += _)
-
+ if (javaOptions.isDefined) {
+ buffer ++= parseOptionString(javaOptions.get)
+ }
val classpathEntries = new mutable.LinkedHashSet[String]
mainResource.foreach(classpathEntries.add)
mainResource.foreach { path =>
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
index 6c21215b56..43c8dc7781 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
@@ -65,7 +65,7 @@ class TrinoProcessBuilder(
buffer += s"-Xmx$memory"
val javaOptions =
conf.get(ENGINE_TRINO_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
if (javaOptions.isDefined) {
- buffer += javaOptions.get
+ buffer ++= parseOptionString(javaOptions.get)
}
val classpathEntries = new mutable.LinkedHashSet[String]
diff --git
a/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/command/CommandLineUtils.scala
b/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/command/CommandLineUtils.scala
index 91327223a6..9e805052be 100644
---
a/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/command/CommandLineUtils.scala
+++
b/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/command/CommandLineUtils.scala
@@ -19,6 +19,7 @@ package org.apache.kyuubi.util.command
import java.io.File
+import scala.collection.mutable.ListBuffer
import scala.util.matching.Regex
object CommandLineUtils {
@@ -72,4 +73,89 @@ object CommandLineUtils {
}
}
}
+
+ /**
+ * copy from org.apache.spark.launcher.CommandBuilderUtils#parseOptionString
+ * Parse a string as if it were a list of arguments, following bash
semantics.
+ * For example:
+ *
+ * Input: "\"ab cd\" efgh 'i \" j'"
+ * Output: [ "ab cd", "efgh", "i \" j" ]
+ */
+ def parseOptionString(s: String): List[String] = {
+ val opts = ListBuffer[String]()
+ val opt = new StringBuilder()
+ var inOpt = false
+ var inSingleQuote = false
+ var inDoubleQuote = false
+ var escapeNext = false
+ var hasData = false
+
+ var i = 0
+ while (i < s.length) {
+ val c = s.codePointAt(i)
+ if (escapeNext) {
+ opt.appendAll(Character.toChars(c))
+ escapeNext = false
+ } else if (inOpt) {
+ c match {
+ case '\\' =>
+ if (inSingleQuote) {
+ opt.appendAll(Character.toChars(c))
+ } else {
+ escapeNext = true
+ }
+ case '\'' =>
+ if (inDoubleQuote) {
+ opt.appendAll(Character.toChars(c))
+ } else {
+ inSingleQuote = !inSingleQuote
+ }
+ case '"' =>
+ if (inSingleQuote) {
+ opt.appendAll(Character.toChars(c))
+ } else {
+ inDoubleQuote = !inDoubleQuote
+ }
+ case _ =>
+ if (!Character.isWhitespace(c) || inSingleQuote || inDoubleQuote) {
+ opt.appendAll(Character.toChars(c))
+ } else {
+ opts += opt.toString()
+ opt.setLength(0)
+ inOpt = false
+ hasData = false
+ }
+ }
+ } else {
+ c match {
+ case '\'' =>
+ inSingleQuote = true
+ inOpt = true
+ hasData = true
+ case '"' =>
+ inDoubleQuote = true
+ inOpt = true
+ hasData = true
+ case '\\' =>
+ escapeNext = true
+ inOpt = true
+ hasData = true
+ case _ =>
+ if (!Character.isWhitespace(c)) {
+ inOpt = true
+ hasData = true
+ opt.appendAll(Character.toChars(c))
+ }
+ }
+ }
+ i += Character.charCount(c)
+ }
+
+ require(!inSingleQuote && !inDoubleQuote && !escapeNext, s"Invalid option
string: $s")
+ if (hasData) {
+ opts += opt.toString()
+ }
+ opts.toList
+ }
}
diff --git
a/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/command/CommandUtilsSuite.scala
b/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/command/CommandUtilsSuite.scala
index e000e7478b..104d02b07b 100644
---
a/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/command/CommandUtilsSuite.scala
+++
b/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/command/CommandUtilsSuite.scala
@@ -47,4 +47,11 @@ class CommandUtilsSuite extends AnyFunSuite {
assertResult(Seq("-cp", "/path/a.jar:/path2/b*.jar"))(
genClasspathOption(Seq("/path/a.jar", "/path2/b*.jar")))
}
+
+ test("parseOptionString should parse a string as a list of arguments") {
+ val input = "\"ab cd\" efgh 'i \" j'"
+ val expectedOutput = List("ab cd", "efgh", "i \" j")
+ val actualOutput = CommandLineUtils.parseOptionString(input)
+ assert(actualOutput == expectedOutput)
+ }
}
