Skip to content

Commit 8d553bb

Browse files
committed
feat(java_opts): replace eval with shell-free javaexec launcher (#1301)
Remove `eval "exec java $JAVA_OPTS ..."` from all container start commands. Replace with a two-layer approach: - Pure-bash `_expand_env_vars` in profile.d assembles JAVA_OPTS from .opts files, expanding $VAR/${VAR} references without executing command substitutions. - Shell-free `javaexec` binary tokenizes JAVA_OPTS at JVM launch (POSIX word-split + quote-removal, no globbing, no $(...) execution) and execs the JVM directly. User JAVA_OPTS from the environment are also expanded ($VAR/${VAR} only) and warned about if they contain $(...) or backtick command substitutions. Closes #1301
1 parent 7b46997 commit 8d553bb

23 files changed

Lines changed: 809 additions & 64 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ tmp/
1515
.cache/
1616
bin/detect
1717
bin/finalize
18+
bin/javaexec
1819
bin/release
1920
bin/supply
2021
/*.md

docs/framework-ordering.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ This ensures:
9494
2. **Container Security Provider runs BEFORE JRebel** (07 < 20)
9595
3. **User JAVA_OPTS override everything** (99 runs last)
9696

97+
> **Note (safe expansion):** the snippet above is simplified. The real
98+
> `00_java_opts.sh` does **not** use `eval`. It expands only `$VAR` / `${VAR}`
99+
> references in `.opts` content via a pure-bash expander, so embedded command
100+
> substitutions (`$(...)`, backticks) are never executed. The one trusted
101+
> substitution the buildpack emits, `-XX:ActiveProcessorCount=$(nproc)`, is
102+
> resolved explicitly at runtime; any other surviving `$(...)` triggers a
103+
> warning. At launch the JVM is started through the shell-free `javaexec`
104+
> launcher (`$DEPS_DIR/<idx>/bin/javaexec`), which tokenizes `JAVA_OPTS`
105+
> without re-invoking a shell, rather than `eval "exec java $JAVA_OPTS"`.
106+
97107
## Critical Ordering Dependencies
98108

99109
### Container Security Provider (Priority 17, Line 51)

manifest.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ include_files:
99
- bin/compile
1010
- bin/detect
1111
- bin/finalize
12+
- bin/javaexec
1213
- bin/release
1314
- bin/supply
1415
- manifest.yml

src/integration/java_main_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func testJavaMain(platform switchblade.Platform, fixtures string) func(*testing.
126126

127127
// Verify SAPMachine JRE was installed from manifest
128128
Expect(logs.String()).To(ContainSubstring("Java Buildpack"))
129-
Expect(logs.String()).To(ContainSubstring("Installing SAP Machine"))
129+
Expect(logs.String()).To(ContainSubstring("Installing SapMachine"))
130130
Expect(logs.String()).To(ContainSubstring("17."))
131131
})
132132
})

src/integration/tomcat_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
171171
Expect(err).NotTo(HaveOccurred(), logs.String)
172172

173173
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (8."))
174-
Expect(logs.String()).To(ContainSubstring("Tomcat 9"))
174+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9."))
175175
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
176176
})
177177

@@ -185,7 +185,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
185185
Expect(err).NotTo(HaveOccurred(), logs.String)
186186

187187
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11."))
188-
Expect(logs.String()).To(ContainSubstring("Tomcat 9"))
188+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9."))
189189
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
190190
})
191191

@@ -198,7 +198,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
198198
Expect(err).NotTo(HaveOccurred(), logs.String)
199199

200200
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17."))
201-
Expect(logs.String()).To(ContainSubstring("Tomcat 9"))
201+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9."))
202202
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
203203
})
204204

@@ -224,7 +224,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
224224
Expect(err).NotTo(HaveOccurred(), logs.String)
225225

226226
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11."))
227-
Expect(logs.String()).To(ContainSubstring("Tomcat 10"))
227+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10."))
228228
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
229229
})
230230

@@ -237,7 +237,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
237237
Expect(err).NotTo(HaveOccurred(), logs.String)
238238

239239
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17."))
240-
Expect(logs.String()).To(ContainSubstring("Tomcat 10"))
240+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10."))
241241
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
242242
})
243243
})
@@ -258,7 +258,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
258258
Expect(err).NotTo(HaveOccurred(), logs.String)
259259

260260
Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17."))
261-
Expect(logs.String()).To(ContainSubstring("Tomcat 10.1."))
261+
Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.1."))
262262
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
263263
})
264264
})
@@ -371,7 +371,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
371371

372372
Expect(err).NotTo(HaveOccurred(), logs.String)
373373

374-
Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (17."))
374+
Expect(logs.String()).To(ContainSubstring("Installing SapMachine (17."))
375375
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
376376
})
377377

@@ -384,7 +384,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
384384

385385
Expect(err).NotTo(HaveOccurred(), logs.String)
386386

387-
Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (21."))
387+
Expect(logs.String()).To(ContainSubstring("Installing SapMachine (21."))
388388
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
389389
})
390390

@@ -397,7 +397,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
397397

398398
Expect(err).NotTo(HaveOccurred(), logs.String)
399399

400-
Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (25."))
400+
Expect(logs.String()).To(ContainSubstring("Installing SapMachine (25."))
401401
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
402402
})
403403
})

src/java/containers/container.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,24 @@ func (r *Registry) RegisterStandardContainers() {
127127
r.Register(NewJavaMainContainer(r.context))
128128
}
129129

130+
// JavaExecCommand builds a start command of the form:
131+
//
132+
// exec $DEPS_DIR/<idx>/bin/javaexec "$JAVA_HOME/bin/java" <javaArgs>
133+
//
134+
// The javaexec launcher reads JAVA_OPTS from the environment and tokenizes it
135+
// without a shell, so glob characters, quotes, and shell metacharacters in
136+
// user-provided JAVA_OPTS are never expanded or executed. This replaces the
137+
// previous `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS <javaArgs>"` form, which
138+
// let the shell glob-expand $JAVA_OPTS and execute embedded command
139+
// substitutions.
140+
//
141+
// javaArgs are buildpack-generated (jar paths, Main-Class, classpath, and
142+
// shell expansions like ${CLASSPATH} or BOOT-INF/lib/*) and are placed on a
143+
// normal command line, where the shell expands them exactly as before.
144+
func JavaExecCommand(depsIdx, javaArgs string) string {
145+
return `exec $DEPS_DIR/` + depsIdx + `/bin/javaexec "$JAVA_HOME/bin/java" ` + javaArgs
146+
}
147+
130148
// This script is used to process the CLASSPATH assembled from various framework scripts sourced from profile.d
131149
// to further create symlinks to the corresponding framework dependencies in WEB-INF/lib, BOOT-INF/lib and where ever
132150
// needed thus they are available for application classloading

src/java/containers/java_main.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -286,19 +286,20 @@ func (j *JavaMainContainer) Release() (string, error) {
286286

287287
args := ""
288288
if cfg.Arguments != "" {
289+
// Passed through as-is onto the start command line. The command is parsed
290+
// once by the shell at launch (no eval), so any quoting in the arguments is
291+
// applied exactly once, like a normal shell command line.
289292
args = " " + cfg.Arguments
290293
}
291294

292295
// JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class.
293296
// Use classpath mode so the configured class is actually invoked (not the manifest's).
294297
if cfg.JavaMainClass != "" {
295-
return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args), nil
298+
return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args)), nil
296299
}
297300

298301
if j.jarFile != "" {
299-
// JAR has its own Main-Class in the manifest — java -jar handles it
300-
// Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity)
301-
return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -jar %s%s", j.jarFile, args), nil
302+
return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-jar %s%s", j.jarFile, args)), nil
302303
}
303304

304305
// Classpath mode: need an explicit main class
@@ -311,6 +312,5 @@ func (j *JavaMainContainer) Release() (string, error) {
311312
j.context.Log.Debug("Main Class %s found in JAVA_MAIN_CLASS", mainClass)
312313
}
313314

314-
// Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity)
315-
return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args), nil
315+
return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args)), nil
316316
}

src/java/containers/java_main_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ var _ = Describe("Java Main Container", func() {
154154
})
155155

156156
Describe("Release", func() {
157+
expectSafeLaunch := func(cmd string) {
158+
Expect(cmd).To(ContainSubstring("/bin/javaexec"))
159+
Expect(cmd).To(ContainSubstring(`"$JAVA_HOME/bin/java"`))
160+
Expect(cmd).NotTo(ContainSubstring("eval"))
161+
Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS"))
162+
}
163+
157164
Context("with JAR file", func() {
158165
BeforeEach(func() {
159166
Expect(createJar(
@@ -278,6 +285,78 @@ var _ = Describe("Java Main Container", func() {
278285
Expect(err).NotTo(HaveOccurred())
279286
Expect(cmd).To(ContainSubstring("--foo=bar"))
280287
})
288+
289+
It("passes double quotes in arguments through unescaped", func() {
290+
os.Setenv("JBP_CONFIG_JAVA_MAIN", `{arguments: "--spring.datasource.url=\"jdbc:h2:mem:test\""}`)
291+
Expect(createJar(
292+
filepath.Join(buildDir, "app.jar"),
293+
"Manifest-Version: 1.0\nMain-Class: com.example.Main\n",
294+
)).To(Succeed())
295+
container.Detect()
296+
cmd, err := container.Release()
297+
Expect(err).NotTo(HaveOccurred())
298+
// No eval: the start command is parsed once by the shell, which
299+
// consumes the quotes, so they must NOT be backslash-escaped.
300+
Expect(cmd).To(ContainSubstring(`--spring.datasource.url="jdbc:h2:mem:test"`))
301+
})
302+
})
303+
304+
// Regression tests for issue #1301: start command must use the javaexec
305+
// launcher and keep $JAVA_OPTS out of the command, so glob chars in
306+
// JAVA_OPTS are never expanded by the shell.
307+
Context("with JAR file (javaexec launch)", func() {
308+
BeforeEach(func() {
309+
Expect(createJar(
310+
filepath.Join(buildDir, "app.jar"),
311+
"Manifest-Version: 1.0\nMain-Class: com.example.Main\n",
312+
)).To(Succeed())
313+
container.Detect()
314+
})
315+
316+
It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() {
317+
cmd, err := container.Release()
318+
Expect(err).NotTo(HaveOccurred())
319+
expectSafeLaunch(cmd)
320+
})
321+
})
322+
323+
Context("with JAVA_MAIN_CLASS env variable (javaexec launch)", func() {
324+
BeforeEach(func() {
325+
os.Setenv("JAVA_MAIN_CLASS", "com.example.Main")
326+
os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644)
327+
container.Detect()
328+
})
329+
330+
AfterEach(func() {
331+
os.Unsetenv("JAVA_MAIN_CLASS")
332+
})
333+
334+
It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() {
335+
cmd, err := container.Release()
336+
Expect(err).NotTo(HaveOccurred())
337+
expectSafeLaunch(cmd)
338+
})
339+
})
340+
341+
Context("with JBP_CONFIG_JAVA_MAIN java_main_class (javaexec launch)", func() {
342+
BeforeEach(func() {
343+
os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}")
344+
Expect(createJar(
345+
filepath.Join(buildDir, "app.jar"),
346+
"Manifest-Version: 1.0\nMain-Class: com.example.Main\n",
347+
)).To(Succeed())
348+
container.Detect()
349+
})
350+
351+
AfterEach(func() {
352+
os.Unsetenv("JBP_CONFIG_JAVA_MAIN")
353+
})
354+
355+
It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() {
356+
cmd, err := container.Release()
357+
Expect(err).NotTo(HaveOccurred())
358+
expectSafeLaunch(cmd)
359+
})
281360
})
282361

283362
Context("without main class or JAR", func() {
@@ -351,6 +430,7 @@ var _ = Describe("Java Main Container", func() {
351430
Expect(cmd).To(ContainSubstring("."))
352431
})
353432
})
433+
354434
})
355435

356436
Describe("Finalize", func() {

src/java/containers/play.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,9 @@ func (p *PlayContainer) Release() (string, error) {
548548
libPath = filepath.ToSlash(relPath)
549549
}
550550
}
551-
// Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity)
552-
cmd = fmt.Sprintf("eval exec java $JAVA_OPTS -cp $HOME/%s/* play.core.server.NettyServer $HOME", libPath)
551+
// JAVA_OPTS is applied by the javaexec launcher, which tokenizes it
552+
// without a shell (no glob/word-splitting/command execution).
553+
cmd = fmt.Sprintf(`exec $DEPS_DIR/%s/bin/javaexec "$JAVA_HOME/bin/java" -cp $HOME/%s/* play.core.server.NettyServer $HOME`, p.context.Stager.DepsIdx(), libPath)
553554
}
554555

555556
p.context.Log.Debug("Play Framework release command: %s", cmd)

src/java/containers/play_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@ var _ = Describe("Play Container", func() {
185185
It("returns correct java command", func() {
186186
cmd, err := container.Release()
187187
Expect(err).NotTo(HaveOccurred())
188-
Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp"))
188+
Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`))
189+
Expect(cmd).NotTo(ContainSubstring("eval"))
190+
Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS"))
189191
Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME"))
190192
})
191193
})
@@ -201,7 +203,9 @@ var _ = Describe("Play Container", func() {
201203
It("returns correct java command", func() {
202204
cmd, err := container.Release()
203205
Expect(err).NotTo(HaveOccurred())
204-
Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp"))
206+
Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`))
207+
Expect(cmd).NotTo(ContainSubstring("eval"))
208+
Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS"))
205209
Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME"))
206210
})
207211
})

0 commit comments

Comments
 (0)