Skip to content

Commit f876600

Browse files
lsm1bowenliang123
authored andcommitted
[KYUUBI #6772] Fix ProcessBuilder to properly handle Java opts as a list
# 🔍 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 🔖 - [ ] 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 ⚰️ #### Behavior With This Pull Request 🎉 #### 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 fb6d532 [senmiaoliu] fix process builder java opts Authored-by: senmiaoliu <[email protected]> Signed-off-by: Bowen Liang <[email protected]>
1 parent 1d668ea commit f876600

File tree

6 files changed

+99
-6
lines changed

6 files changed

+99
-6
lines changed

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,8 @@ class FlinkProcessBuilder(
198198
buffer += s"-Xmx$memory"
199199
val javaOptions = conf.get(ENGINE_FLINK_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
200200
if (javaOptions.isDefined) {
201-
buffer += javaOptions.get
201+
buffer ++= parseOptionString(javaOptions.get)
202202
}
203-
204203
val classpathEntries = new mutable.LinkedHashSet[String]
205204
// flink engine runtime jar
206205
mainResource.foreach(classpathEntries.add)

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class HiveProcessBuilder(
6565
buffer += s"-Xmx$memory"
6666
val javaOptions = conf.get(ENGINE_HIVE_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
6767
if (javaOptions.isDefined) {
68-
buffer += javaOptions.get
68+
buffer ++= parseOptionString(javaOptions.get)
6969
}
7070
// -Xmx5g
7171
// java options

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/jdbc/JdbcProcessBuilder.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ class JdbcProcessBuilder(
7373
buffer += s"-Xmx$memory"
7474

7575
val javaOptions = conf.get(ENGINE_JDBC_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
76-
javaOptions.foreach(buffer += _)
77-
76+
if (javaOptions.isDefined) {
77+
buffer ++= parseOptionString(javaOptions.get)
78+
}
7879
val classpathEntries = new mutable.LinkedHashSet[String]
7980
mainResource.foreach(classpathEntries.add)
8081
mainResource.foreach { path =>

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class TrinoProcessBuilder(
6565
buffer += s"-Xmx$memory"
6666
val javaOptions = conf.get(ENGINE_TRINO_JAVA_OPTIONS).filter(StringUtils.isNotBlank(_))
6767
if (javaOptions.isDefined) {
68-
buffer += javaOptions.get
68+
buffer ++= parseOptionString(javaOptions.get)
6969
}
7070

7171
val classpathEntries = new mutable.LinkedHashSet[String]

kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/command/CommandLineUtils.scala

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package org.apache.kyuubi.util.command
1919

2020
import java.io.File
2121

22+
import scala.collection.mutable.ListBuffer
2223
import scala.util.matching.Regex
2324

2425
object CommandLineUtils {
@@ -72,4 +73,89 @@ object CommandLineUtils {
7273
}
7374
}
7475
}
76+
77+
/**
78+
* copy from org.apache.spark.launcher.CommandBuilderUtils#parseOptionString
79+
* Parse a string as if it were a list of arguments, following bash semantics.
80+
* For example:
81+
*
82+
* Input: "\"ab cd\" efgh 'i \" j'"
83+
* Output: [ "ab cd", "efgh", "i \" j" ]
84+
*/
85+
def parseOptionString(s: String): List[String] = {
86+
val opts = ListBuffer[String]()
87+
val opt = new StringBuilder()
88+
var inOpt = false
89+
var inSingleQuote = false
90+
var inDoubleQuote = false
91+
var escapeNext = false
92+
var hasData = false
93+
94+
var i = 0
95+
while (i < s.length) {
96+
val c = s.codePointAt(i)
97+
if (escapeNext) {
98+
opt.appendAll(Character.toChars(c))
99+
escapeNext = false
100+
} else if (inOpt) {
101+
c match {
102+
case '\\' =>
103+
if (inSingleQuote) {
104+
opt.appendAll(Character.toChars(c))
105+
} else {
106+
escapeNext = true
107+
}
108+
case '\'' =>
109+
if (inDoubleQuote) {
110+
opt.appendAll(Character.toChars(c))
111+
} else {
112+
inSingleQuote = !inSingleQuote
113+
}
114+
case '"' =>
115+
if (inSingleQuote) {
116+
opt.appendAll(Character.toChars(c))
117+
} else {
118+
inDoubleQuote = !inDoubleQuote
119+
}
120+
case _ =>
121+
if (!Character.isWhitespace(c) || inSingleQuote || inDoubleQuote) {
122+
opt.appendAll(Character.toChars(c))
123+
} else {
124+
opts += opt.toString()
125+
opt.setLength(0)
126+
inOpt = false
127+
hasData = false
128+
}
129+
}
130+
} else {
131+
c match {
132+
case '\'' =>
133+
inSingleQuote = true
134+
inOpt = true
135+
hasData = true
136+
case '"' =>
137+
inDoubleQuote = true
138+
inOpt = true
139+
hasData = true
140+
case '\\' =>
141+
escapeNext = true
142+
inOpt = true
143+
hasData = true
144+
case _ =>
145+
if (!Character.isWhitespace(c)) {
146+
inOpt = true
147+
hasData = true
148+
opt.appendAll(Character.toChars(c))
149+
}
150+
}
151+
}
152+
i += Character.charCount(c)
153+
}
154+
155+
require(!inSingleQuote && !inDoubleQuote && !escapeNext, s"Invalid option string: $s")
156+
if (hasData) {
157+
opts += opt.toString()
158+
}
159+
opts.toList
160+
}
75161
}

kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/command/CommandUtilsSuite.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,11 @@ class CommandUtilsSuite extends AnyFunSuite {
4747
assertResult(Seq("-cp", "/path/a.jar:/path2/b*.jar"))(
4848
genClasspathOption(Seq("/path/a.jar", "/path2/b*.jar")))
4949
}
50+
51+
test("parseOptionString should parse a string as a list of arguments") {
52+
val input = "\"ab cd\" efgh 'i \" j'"
53+
val expectedOutput = List("ab cd", "efgh", "i \" j")
54+
val actualOutput = CommandLineUtils.parseOptionString(input)
55+
assert(actualOutput == expectedOutput)
56+
}
5057
}

0 commit comments

Comments
 (0)