Skip to content

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

Open
stokpop wants to merge 4 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 4 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

@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.

@stokpop

stokpop commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Addressed in the three commits just pushed:

fix(finalize): build javaexec in bin/finalize for source/git buildpack usage

  • bin/finalize now builds src/java/javaexec/cli into the same temp dir as the finalize binary and passes the path via JAVAEXEC_BINARY_PATH.
  • InstallJavaexecLauncher() (renamed from private) prefers JAVAEXEC_BINARY_PATH, falls back to <buildpackDir>/bin/javaexec for packaged buildpacks — no change to packaged behaviour.
  • Three unit tests: packaged path, env-var override (source/git path), missing-binary error.

docs: document javaexec binary and source/git buildpack path

  • DEVELOPING.md: javaexec added to build output, project structure, manual build section, and a new Source/Git Buildpack Usage section explaining the JAVAEXEC_BINARY_PATH mechanism.
  • design.md: finalize phase step for javaexec install + source/git note.
  • src/integration/README.md: explicit note that source/git path is not covered by integration tests, pointing to unit tests and the smoke script.

test(scripts): add JAVA_OPTS tokenization smoke test to source-path script

scripts/test-javaexec-source-path.sh — verifies the full source/git path locally without CF: builds both binaries, runs the unit tests with the env-var override, then invokes the real javaexec binary against a fake JVM to confirm tokenization (quoted spaces, cron *, $(...) not executed). Useful when custom_buildpacks is disabled on the target CF.

@stokpop

stokpop commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@ramonskie thanks for bringing the source/git use case to my attention, I was not aware of that.
Can you please have another look?

@ramonskie

Copy link
Copy Markdown
Contributor

could you squash the commit or if needed squash it in sensible parts..

stokpop added 4 commits June 16, 2026 10:30
…dfoundry#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 cloudfoundry#1301
…e behavior

- Integration tests: Spring Boot and Play containers verify $(...) in user
  JAVA_OPTS reaches JVM as literal text (not executed).
- Unit tests: end-to-end javaexec tests covering tokenization, word-split
  behavior for vars-with-spaces, and command-substitution warning output.
- Docs: framework-java_opts.md covers runtime expansion rules; ruby-migration
  guide documents differences from old eval behavior.
…ion warning

- jvmkill and memory-calculator now use $DEPS_DIR instead of hardcoded
  /home/vcap/deps so they work in non-default CF dep layouts.
- javaexec tokenizer preserves $(...), ${...}, and backtick spans as single
  tokens (no splitting on spaces inside them).
- Warning for command substitution in JAVA_OPTS now shows only the matching
  token, not the full JAVA_OPTS value.
…k usage

bin/finalize only built src/java/finalize/cli into a temp dir; packaged
buildpacks got bin/javaexec from scripts/build.sh, but source/git usage
(e.g. WithBuildpacks("https://github.com/cloudfoundry/java-buildpack.git"))
had no bin/javaexec and finalize aborted before release completed.

- bin/finalize now also builds src/java/javaexec/cli into the same temp dir
  and passes the path via JAVAEXEC_BINARY_PATH.
- InstallJavaexecLauncher() (renamed from private) prefers JAVAEXEC_BINARY_PATH
  and falls back to <buildpackDir>/bin/javaexec for packaged buildpacks.
- Three unit tests cover packaged path, source/git env-var override, and the
  missing-binary error case.
@stokpop stokpop force-pushed the issue-1301-replace-eval-opts-expansion branch from 5dc7b57 to de38d37 Compare June 16, 2026 10:31
@stokpop

stokpop commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@ramonskie cleaned up the commits

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