Skip to content

fix: replace eval-based start command with shell-free javaexec launcher (#1301)#1316

Open
stokpop wants to merge 23 commits into
cloudfoundry:mainfrom
stokpop:issue-1301-replace-eval-opts-expansion
Open

fix: replace eval-based start command with shell-free javaexec launcher (#1301)#1316
stokpop wants to merge 23 commits into
cloudfoundry:mainfrom
stokpop:issue-1301-replace-eval-opts-expansion

Conversation

@stokpop

@stokpop stokpop commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Problem

The start command used eval "exec java $JAVA_OPTS ..." to launch the JVM.
eval reinterprets its argument as shell code, which made it fundamentally
difficult to preserve JAVA_OPTS values exactly as specified:

  • Backslashes were consumed as escape characters (C:\pathC:path)
  • Quotes were stripped, so space-containing values split into multiple
    JVM arguments (-Dfoo="bar baz"-Dfoo=bar + baz)
  • Glob characters in unquoted values expanded (*/7 → filenames)
  • Pipe and semicolon characters were interpreted as shell operators

Each issue required a new escaping workaround; combinations of edge cases
kept producing regressions. The root cause is that eval is the wrong tool
for passing an arbitrary string as JVM arguments.

Note: this PR replaces #1303 (which can be closed if this is merged, or revived from draft if this PR is not to be merged).

Solution

1. javaexec launcher (src/java/javaexec/)

A compiled Go binary invoked as exec $DEPS_DIR/<idx>/bin/javaexec "$JAVA_HOME/bin/java" <args>.
It tokenizes $JAVA_OPTS using POSIX word-splitting and quote-removal rules,
but performs no further processing: $(...), ${...}, backtick
substitutions, and shell metacharacters (&, |, ;, >) are all passed
literally to the JVM. Nested $(...) and ${...} forms with spaces inside
are kept as one token.

2. Pure-bash _expand_env_vars in profile.d/00_java_opts.sh

Expands $VAR / ${VAR} references using only bash builtins — never
re-interprets quotes, backslashes, or metacharacters. The single known trusted
substitution $(nproc) is resolved explicitly. \$VAR passes a literal
$VAR to the JVM (Ruby buildpack parity). A WARNING is emitted to stderr
(showing only the offending token) when $(...) or a backtick appears in
user JAVA_OPTS.

Other fixes included

  • memory_calculator.go, jvmkill.go: replace hardcoded /home/vcap/deps/
    with $DEPS_DIR so installs work on non-default CF stacks.

Test coverage

  • Tokenizer unit tests: quoting, metacharacters, $(...) grouping,
    ${...} extended forms, backtick, nested parens, full manifest scenario.
  • End-to-end runStartCommand tests: exact v5.x regression: JAVA_OPTS handling broken (pipes/newlines in 5.0.2, quoting in 5.0.3) #1301 reproducer (cron + quoted
    spaces), pipe, metacharacters, command substitution not executed (marker
    file guard), \$VAR escape, warning token extraction, POSIX word-split
    documentation.
  • Integration: runStartCommand smoke test verifying javaexec fires for
    Spring Boot, Java Main, Play, Tomcat containers.

Closes #1301

stokpop added 23 commits May 28, 2026 16:28
…cloudfoundry#1301)

- Replace xargs-based JAVA_OPTS normalization with newline-only normalization
  to preserve quotes and backslashes.
- Protect user-provided JAVA_OPTS during opts-file eval expansion by using a
  placeholder and restoring it after eval.
- Introduce JavaExecCommand() and switch Java Main/Spring Boot start commands
  to eval "exec ... $JAVA_OPTS ..." so shell globbing/word-splitting does not
  corrupt args.
- Add regression tests for quoted values, cron/glob expressions, multiple
  quoted args, backslashes, and start-command invocation behavior.
- Clean up overlapping/no-op test code and improve test readability.
The profile.d assembly script expanded remaining environment variables in
framework .opts file content using eval, which also executes embedded command
substitutions ($(...) / backticks) — an unnecessary code-execution surface.

Replace eval with a dependency-free pure-bash expander that expands only
$VAR / ${VAR} references and never executes command substitutions. envsubst
is not used because gettext-base is not present on the cflinuxfs stacks
(verified: envsubst not found at runtime).

The buildpack itself emits one trusted command substitution into .opts
content: -XX:ActiveProcessorCount=$(nproc) (jres/openjdk.go). Resolve this
single known token by computing the processor count once in the runtime
profile.d script (so nproc reflects the running app container's CPUs, as the
old eval did) and substituting it literally, so the JVM still receives a
numeric value. Any other $(...) or backtick that survives expansion is
buildpack-emitted and would reach the JVM literally, so emit a warning to
surface such a regression.

Expansion remains single-pass to match the previous eval behavior. Add tests
for arbitrary runtime variable expansion, command substitutions being
preserved literally rather than executed, the $(nproc) resolution, and the
unresolved-substitution warning.
The container start commands invoked the JVM via
  eval "exec $JAVA_HOME/bin/java $JAVA_OPTS <args>"
so the shell re-parsed JAVA_OPTS, executing any embedded command
substitutions and treating shell metacharacters as operators — a
code-execution surface and a source of quoting bugs (issue cloudfoundry#1301).

Introduce a small Go launcher (src/java/javaexec) that tokenizes JAVA_OPTS
with POSIX word-splitting and quote removal only — never expanding variables,
globs, or command substitutions — then syscall.Exec's java in place. The
start command becomes
  exec $DEPS_DIR/<idx>/bin/javaexec "$JAVA_HOME/bin/java" <trusted args>
where JAVA_OPTS is read from the environment by the launcher, so user-provided
options can never be re-interpreted by a shell. Trusted, buildpack-built args
(classpath, globs, $PWD) remain on the normal command line and expand as
before.

The finalize step installs the launcher binary into the dependency bin dir;
it now hard-fails if the binary is missing so a packaging error is caught at
staging rather than at launch. The binary is a build artifact (built by
scripts/build.sh) and is gitignored.

Covers java_main, spring_boot and play containers.
…nused test param

Arguments from JBP_CONFIG_JAVA_MAIN containing double quotes would break the
outer eval "..." string in the start command. Escape them before building
the javaArgs string. Add regression test.

Rename unused 'javaOpts' param in setupScript test helper to '_' to document
that it has no effect on script generation.
eval "printf '%s' \"\$opts_content\"" mangles .opts content containing
literal double quotes (e.g. Datadog writes -Ddd.service="myapp"): the inner "
terminates the outer eval string and the value is stripped.

Escape \ (first) and " in opts_content into a temporary _eval_safe variable
before expanding it into the eval command, so both chars are preserved.

Add regression tests for double-quoted and backslash .opts values.
cloudfoundry#1288 BaseJRE refactor renamed the install log from
"Installing SAP Machine" to "Installing SapMachine"; four
assertions in tomcat_test.go and java_main_test.go were missed.
Commit 90abbc2 changed the log format from
"Installed Tomcat version %s" to "Installed Tomcat (%s)" but did not
update the integration test assertions that matched "Tomcat 9" and
"Tomcat 10". Those substrings no longer appear in the output, causing
all JRE-selection Tomcat tests to fail.
…y#1301)

Replace eval-based JAVA_OPTS assembly with a pure-bash _expand_env_vars
function that expands $VAR/${VAR} but never executes $(...) or backtick
substitutions. Resolves the one trusted $(nproc) token by exact string
match instead of execution.

User JAVA_OPTS and .opts file content are both run through the expander.
A \$VAR prefix is honoured as a literal $ (Ruby buildpack parity) via a
placeholder round-trip. CRLF line endings from Windows-edited manifests
are stripped before assembly.

Renames _nproc_token/_nproc_value -> _nproc_count; inlines the
\$(nproc) pattern literal so the variable name no longer shadows the
shell command it references.

java_main: remove double-quote escaping that was only needed because
the old eval consumed one extra parse layer; javaexec sees the command
once so quotes pass through unmodified.

Closes cloudfoundry#1301
Replace the stale eval-era "Escaping strings" section with a
"Runtime variable expansion" section covering $VAR expansion, the
$(nproc) exception, \$VAR literal-dollar escape, POSIX quoting, and
a lookup table for common escaping needs.

Add docs/java_opts-ruby-migration.md comparing Ruby vs Go buildpack
escaping rules with a quick migration checklist.
Add integration-level smoke tests verifying that command substitutions
in user JAVA_OPTS are not executed and that shell metacharacters do not
break app launch under the javaexec launcher (Spring Boot, Play).

Note in README that JAVA_OPTS character-fidelity is covered at unit
level in java_opts_writer_test.go so integration tests focus on
end-to-end launch correctness only.
…#1301 scenarios

Add runStartCommand tests (full assembly + javaexec tokenization) for
the exact reproducer from cloudfoundry#1301 and two additional shell-metacharacter
cases that were only tested at the profile.d output level:

- Combined quoted-value + cron: '-Dfoo="bar baz" -DcronSched="0 */7 * * * *"'
  verifies both args arrive with correct boundaries (previously
  xargs stripped quotes → * glob-expanded → ClassNotFoundException)
- Pipe character: -Dpattern=foo|bar passes through without shell pipe
- Shell metacharacters &, ;, > treated as literals by javaexec

Refs cloudfoundry#1301
Hardcoded path breaks app startup if CF ever mounts deps elsewhere:
- memory_calculator: bad path breaks the && chain before exec javaexec,
  JVM never starts
- jvmkill: bad -agentpath causes JVM to abort on agent load before main()

Use \$DEPS_DIR/<idx>/... in both. The platform sets \$DEPS_DIR before
the start command runs; the profile.d assembly script expands it in
.opts file content before passing to the JVM.
Spaces inside command/parameter substitutions in JAVA_OPTS caused
javaexec to split the token. e.g.
  -Dwhere=\$( hostname | tr '\n' | curl -v 'https://example.com')
  -Dfoo=\${MY_VAR:-hello world}
  -Dx=\`hostname -f\`
split on the space, and Java received 'hostname' as a class name
→ ClassNotFoundException, app did not start.

Tokenizer now treats \$(...), \${...}, and \`...\` as unbreakable
units using depth-tracked delimiters. Nested parens/braces are
handled correctly. None are executed; all pass literally to the JVM.

profile.d: add \$( / backtick WARNING to the USER_JAVA_OPTS path
(was only emitted for .opts file content before).

Tests added before fix; full manifest scenario covers all reported
edge cases from issue cloudfoundry#1301.

Refs cloudfoundry#1301
…ars with spaces

Vars whose values contain spaces split into multiple JVM args when unquoted
(identical to old eval behavior). Double-quoting the reference — "-Dfoo=$VAR" —
passes the expanded value as one token via javaexec's double-quote handler.

Three end-to-end tests cover: unquoted split, double-quoted user JAVA_OPTS,
double-quoted .opts file content.
…VA_OPTS

Full value in the warning was noisy for long option sets and risked leaking
secrets embedded in JAVA_OPTS. Extract just the first $(...) or backtick
token so the user can identify the offending substitution without exposing
unrelated values.
@ramonskie

Copy link
Copy Markdown
Contributor

Medium: bin/javaexec is required but not built for git/source buildpack usage.

installJavaexecLauncher() now reads <buildpackDir>/bin/javaexec, which works for packaged buildpacks because scripts/build.sh creates it. But direct git/source usage only builds the finalize binary via bin/finalize; it does not build src/java/javaexec/cli, and bin/javaexec is not checked in.

This means source buildpack usage such as WithBuildpacks("https://github.com/cloudfoundry/java-buildpack.git") can fail during finalize with “javaexec launcher binary not found”.

Suggested fix: update the source/git wrapper to build javaexec too, or make installJavaexecLauncher() use a generated/temp javaexec path when running from source.

@ramonskie

Copy link
Copy Markdown
Contributor

I did a deeper pass on the javaexec integration. The launcher approach itself looks like the strongest option for the problem this PR is solving: bash arrays, JAVA_TOOL_OPTIONS/JDK_JAVA_OPTIONS, generated shell wrappers, or per-container fixes either reintroduce a parser/eval problem, do not preserve current JAVA_OPTS semantics/order reliably, or only address part of the eval surface.

The issue I think still needs addressing is lifecycle/packaging integration:

  • installJavaexecLauncher() runs unconditionally during finalize.
  • It requires <buildpackDir>/bin/javaexec to exist.
  • Packaged buildpacks get that from scripts/build.sh, but source/git usage via bin/finalize only builds src/java/finalize/cli into a temp dir.
  • Because bin/javaexec is ignored/not checked in, source/git usage can fail staging for any app type, including apps whose final start command does not use javaexec, since finalize aborts before release completes.

WriteProfileD itself does not look directly broken. CreateJavaOptsAssemblyScript() still writes profile.d/00_java_opts.sh before the launcher install step. The failure mode is that profile.d may be written successfully, then finalize aborts while copying the missing launcher.

A safer fix would be to make the source/git wrapper build src/java/javaexec/cli into the same temp directory as the finalize binary, then pass that explicit binary path into finalize, for example via an env var. installJavaexecLauncher() could prefer that override and fall back to <buildpackDir>/bin/javaexec for packaged buildpacks. That keeps packaged behavior unchanged, avoids mutating the source checkout during staging, and preserves the good part of this PR: shell-free handling of user-provided JAVA_OPTS at the final JVM exec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v5.x regression: JAVA_OPTS handling broken (pipes/newlines in 5.0.2, quoting in 5.0.3)

2 participants