diff --git a/.gitignore b/.gitignore index 6cc6ac3ff3..4c02109d30 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ tmp/ .cache/ bin/detect bin/finalize +bin/javaexec bin/release bin/supply /*.md diff --git a/bin/finalize b/bin/finalize index 5061e97b5b..3c33ad4871 100755 --- a/bin/finalize +++ b/bin/finalize @@ -14,6 +14,8 @@ output_dir=$(mktemp -d -t finalizeXXX) pushd $BUILDPACK_DIR > /dev/null echo "-----> Running go build finalize" GOROOT=$GoInstallDir $GoInstallDir/bin/go build -mod=vendor -o $output_dir/finalize ./src/java/finalize/cli +echo "-----> Running go build javaexec" +GOROOT=$GoInstallDir $GoInstallDir/bin/go build -mod=vendor -o $output_dir/javaexec ./src/java/javaexec/cli popd > /dev/null -$output_dir/finalize "$BUILD_DIR" "$CACHE_DIR" "$DEPS_DIR" "$DEPS_IDX" "$PROFILE_DIR" +JAVAEXEC_BINARY_PATH=$output_dir/javaexec $output_dir/finalize "$BUILD_DIR" "$CACHE_DIR" "$DEPS_DIR" "$DEPS_IDX" "$PROFILE_DIR" diff --git a/docs/DEVELOPING.md b/docs/DEVELOPING.md index 9db4c1bbda..e386827709 100644 --- a/docs/DEVELOPING.md +++ b/docs/DEVELOPING.md @@ -97,6 +97,7 @@ Build the buildpack binaries: This creates executables in the `bin/` directory: - `bin/supply` - Staging phase binary (downloads and installs dependencies) - `bin/finalize` - Finalization phase binary (configures runtime) +- `bin/javaexec` - Shell-free JVM launcher (tokenizes `JAVA_OPTS` without `eval`) ## Project Structure @@ -118,6 +119,7 @@ java-buildpack/ │ ├── jres/ # JRE implementations (7 providers) │ ├── supply/cli/ # Supply phase entrypoint │ ├── finalize/cli/ # Finalize phase entrypoint +│ ├── javaexec/cli/ # Shell-free JVM launcher entrypoint │ ├── resources/ # Resource configuration files │ └── integration/ # Integration tests ├── scripts/ # Build and test scripts @@ -156,6 +158,7 @@ Build for the default platform (Linux): ``` -----> Building supply for linux -----> Building finalize for linux +-----> Building javaexec for linux -----> Build complete ``` @@ -193,6 +196,25 @@ go build -mod vendor -o bin/supply src/java/supply/cli/main.go # Build finalize go build -mod vendor -o bin/finalize src/java/finalize/cli/main.go + +# Build javaexec (shell-free JVM launcher, required at runtime) +go build -mod vendor -o bin/javaexec src/java/javaexec/cli/main.go +``` + +### Source/Git Buildpack Usage + +When deploying with a git URL (`cf push -b https://github.com/.../java-buildpack.git`), +Cloud Foundry runs `bin/finalize` directly from the cloned source. In that mode +`bin/javaexec` does not exist (only packaged buildpacks have it). `bin/finalize` +therefore builds `javaexec` into a temp directory alongside the finalize binary +and passes the path via the `JAVAEXEC_BINARY_PATH` environment variable. +`InstallJavaexecLauncher()` prefers this override and falls back to +`bin/javaexec` for packaged buildpacks. + +To verify this path locally (no CF required): + +```bash +bash scripts/test-javaexec-source-path.sh ``` ## Running Tests diff --git a/docs/design.md b/docs/design.md index 47e91e6b96..8b24afe1b5 100644 --- a/docs/design.md +++ b/docs/design.md @@ -186,6 +186,12 @@ During the finalize phase (`bin/finalize`), the buildpack: 1. **Finalizes JRE**: Configures JVM options, memory calculator 2. **Finalizes Frameworks**: Adds agent paths, system properties 3. **Finalizes Container**: Generates launch command +4. **Installs `javaexec`**: Copies the shell-free JVM launcher into + `$DEPS_DIR//bin/javaexec`. The generated start command invokes it + instead of `eval "exec java $JAVA_OPTS ..."` so that `JAVA_OPTS` is + tokenized without shell interpretation. For source/git buildpack usage + `bin/finalize` builds `javaexec` on the fly and passes its path via + `JAVAEXEC_BINARY_PATH` (see [DEVELOPING.md](DEVELOPING.md#sourcegit-buildpack-usage)). Components can: - Read installed dependencies from `$DEPS_DIR//` diff --git a/docs/framework-java_opts.md b/docs/framework-java_opts.md index a4067582f9..1b83d3555b 100644 --- a/docs/framework-java_opts.md +++ b/docs/framework-java_opts.md @@ -26,37 +26,83 @@ The framework can be configured by creating or modifying the [`config/java_opts. Any `JAVA_OPTS` from either the config file or environment variables will be specified in the start command after any Java Opts added by other frameworks. -## Escaping strings +## Runtime variable expansion -Java options will have special characters escaped when used in the shell command that starts the Java application but the `$` and `\` characters will not be escaped. This is to allow Java options to include environment variables when the application starts. +Java options are assembled at container start by the buildpack's `profile.d` script +(`00_java_opts.sh`), then passed to the JVM by the shell-free `javaexec` launcher. +Because `javaexec` tokenizes `JAVA_OPTS` without invoking a shell, characters such as +`*`, `&`, `;`, `|`, and `>` are treated as literals — they reach the JVM exactly as +written. -```bash -cf set-env my-application JAVA_OPTS '-Dexample.port=$PORT' -``` - -If an escaped `$` or `\` character is needed in the Java options they will have to be escaped manually. For example, to obtain this output in the start command. +### Environment variable references -```bash --Dexample.other=something.\$dollar.\\slash -``` +`$VARNAME` and `${VARNAME}` references in **both** `JAVA_OPTS` (env) and `java_opts` +(config) are expanded at container start against the runtime environment: -From the command line use; ```bash -cf set-env my-application JAVA_OPTS '-Dexample.other=something.\\\\\$dollar.\\\\\\\slash' +# $PWD, $HOME, $PORT, and any CF-injected variable all work +cf set-env my-application JAVA_OPTS '-Dapp.config=$PWD/config/app.properties' +cf set-env my-application JAVA_OPTS '-Dserver.port=$PORT' ``` -From the [`config/java_opts.yml`][] file use; ```yaml -from_environment: true -java_opts: '-Dexample.other=something.\\$dollar.\\\\slash' +# config/java_opts.yml +java_opts: '-Xloggc:$PWD/beacon_gc.log -verbose:gc' ``` -Finally, from the applications manifest use; -```yaml - env: - JAVA_OPTS: '-Dexample.other=something.\\\\\$dollar.\\\\\\\slash' +### Command substitutions are never executed + +`$(...)` and backtick command substitutions are **not** executed. A value such as +`-Dinject=$(hostname)` reaches the JVM as the literal string `-Dinject=$(hostname)`. +This is intentional: executing arbitrary commands from a user-supplied option string +would be a security vulnerability. + +### Processor count: `$(nproc)` + +The one exception is `-XX:ActiveProcessorCount=$(nproc)`, which the buildpack itself +emits for JRE vendors that need it. The profile.d script resolves this single known +token to the actual CPU count before passing the option to the JVM. Any other +`$(...)` expression passes to the JVM literally. + +### Special characters and quoting + +Characters that were shell-special under the old `eval`-based launcher (`*`, `&`, +`;`, `|`, `>`) are now passed to the JVM as literals — no quoting tricks required. + +POSIX quoting in the assembled `JAVA_OPTS` string is respected by `javaexec`'s +tokenizer: a quoted value such as `"-Dfoo=bar baz"` is delivered as the single +argument `-Dfoo=bar baz`. + +| Want to pass to JVM | Write in `JAVA_OPTS` / `java_opts` | +|---------------------|-------------------------------------| +| Literal `$PORT` (no expansion) | `\$PORT` | +| Literal `\` backslash | `\\` | +| Literal `\\` two backslashes | `\\\\` | +| Value of `$PORT` at runtime | `$PORT` | +| Cron expression `0 */7 * * *` | `0 */7 * * *` (no quoting needed) | +| Space inside one JVM arg | `"-Dfoo=bar baz"` (quote the arg) | + +```bash +# Expand $PORT at runtime +cf set-env my-application JAVA_OPTS '-Dserver.port=$PORT' + +# Literal $PORT — not expanded +cf set-env my-application JAVA_OPTS '-Dexample.literal=\$PORT' + +# Windows-style path — \\ becomes one backslash +cf set-env my-application JAVA_OPTS '-Dapp.data=C:\\data\\app' + +# Cron expression — * is not glob-expanded +cf set-env my-application JAVA_OPTS '-DcronExpr=0 */7 * * *' ``` +> **Note:** `$` followed by a digit or non-identifier character (e.g. `$1`, `$.`) +> is left as-is. Undefined variables expand to an empty string. + +> **Migrating from the Ruby buildpack?** See +> [Migrating JAVA_OPTS escaping from the Ruby buildpack](java_opts-ruby-migration.md) +> for a comparison of the escaping rules. + ## Examples ### Configuration File Example diff --git a/docs/framework-ordering.md b/docs/framework-ordering.md index 16537a1628..d6158fffbe 100644 --- a/docs/framework-ordering.md +++ b/docs/framework-ordering.md @@ -94,6 +94,16 @@ This ensures: 2. **Container Security Provider runs BEFORE JRebel** (07 < 20) 3. **User JAVA_OPTS override everything** (99 runs last) +> **Note (safe expansion):** the snippet above is simplified. The real +> `00_java_opts.sh` does **not** use `eval`. It expands only `$VAR` / `${VAR}` +> references in `.opts` content via a pure-bash expander, so embedded command +> substitutions (`$(...)`, backticks) are never executed. The one trusted +> substitution the buildpack emits, `-XX:ActiveProcessorCount=$(nproc)`, is +> resolved explicitly at runtime; any other surviving `$(...)` triggers a +> warning. At launch the JVM is started through the shell-free `javaexec` +> launcher (`$DEPS_DIR//bin/javaexec`), which tokenizes `JAVA_OPTS` +> without re-invoking a shell, rather than `eval "exec java $JAVA_OPTS"`. + ## Critical Ordering Dependencies ### Container Security Provider (Priority 17, Line 51) diff --git a/docs/java_opts-ruby-migration.md b/docs/java_opts-ruby-migration.md new file mode 100644 index 0000000000..3bb60baf8f --- /dev/null +++ b/docs/java_opts-ruby-migration.md @@ -0,0 +1,87 @@ +# Migrating JAVA_OPTS escaping from the Ruby buildpack + +The Go rewrite of the Java buildpack changed how `JAVA_OPTS` is assembled and +passed to the JVM. If you are migrating configs written for the Ruby buildpack, +the escaping rules are different. + +--- + +## What changed + +| Mechanism | Ruby buildpack | Go buildpack | +|-----------|---------------|-------------| +| Launch | `eval exec java $JAVA_OPTS ...` | `javaexec` (shell-free tokenizer) | +| `$VAR` in opts | expanded by shell at eval | expanded by `profile.d` at container start | +| `$(cmd)` in opts | **executed** by shell | **never executed** (security fix, #1301) | +| `\` handling | eval consumed one level of backslashes | `javaexec` POSIX: `\\`→`\`, `\"` → `"` | +| `*` glob | expanded against filesystem | literal | + +--- + +## Escaping comparison + +### Dollar sign before a variable name + +Both buildpacks expand `$VAR` references at runtime. No escaping needed or supported. + +```bash +# Works the same in both buildpacks +cf set-env my-app JAVA_OPTS '-Dserver.port=$PORT' +``` + +To prevent expansion, `\$` works in both buildpacks: `\$VAR` delivers the +literal text `$VAR` to the JVM without expanding it. + +### Backslash + +```bash +# Ruby buildpack: \\\\ in the manifest/env → \\ after eval → \ to JVM +# Go buildpack: \\ in the manifest/env → \ to JVM (POSIX tokenizer, one level) +``` + +| Want to deliver to JVM | Ruby buildpack (env) | Go buildpack (env) | +|------------------------|----------------------|--------------------| +| one `\` | `\\\\` | `\\` | +| two `\\` | `\\\\\\\\` | `\\\\` | +| literal `\$PORT` | `\\\\\$PORT` | not supported — `$PORT` expands | + +### Cron expressions and glob characters (`*`) + +```bash +# Ruby buildpack: must be quoted carefully to survive eval and glob expansion +# Go buildpack: write literally — * never globs, no eval +cf set-env my-app JAVA_OPTS '-DcronExpr=0 */7 * * *' +``` + +### Command substitution + +```bash +# Ruby buildpack: $(hostname) in JAVA_OPTS was EXECUTED and replaced with output +# Go buildpack: $(hostname) reaches the JVM as the literal string $(hostname) +# This is intentional — executing user-supplied commands is unsafe +``` + +--- + +## Quick migration checklist + +1. **Remove extra backslashes.** Replace `\\\\` with `\\` — the old pattern + survived two shell parse layers (eval) which no longer exist. + +2. **`\$VAR` still works.** Keep any `\$VAR` escapes you have — they are + honoured and pass the literal `$VAR` text to the JVM in both buildpacks. + +3. **Cron / glob expressions.** Remove any protective quoting that was needed + to survive `eval` — write the expression directly. + +4. **Command substitutions.** If you relied on `$(cmd)` being executed in + `JAVA_OPTS` (e.g. `$(hostname)`, `$(cat /etc/myconfig)`), that no longer + works. Compute the value before the app starts and set it as a separate + environment variable, then reference it via `$MYVAR` in `JAVA_OPTS`. + +--- + +## References + +- [Java Options Framework](framework-java_opts.md) +- Issue [#1301](https://github.com/cloudfoundry/java-buildpack/issues/1301) — remove `eval` from start command diff --git a/manifest.yml b/manifest.yml index 16e19274c5..eb72ec5372 100644 --- a/manifest.yml +++ b/manifest.yml @@ -9,6 +9,7 @@ include_files: - bin/compile - bin/detect - bin/finalize +- bin/javaexec - bin/release - bin/supply - manifest.yml diff --git a/scripts/test-javaexec-source-path.sh b/scripts/test-javaexec-source-path.sh new file mode 100755 index 0000000000..cb261e1c5c --- /dev/null +++ b/scripts/test-javaexec-source-path.sh @@ -0,0 +1,64 @@ +#!/bin/bash +# Manual smoke test for the bin/finalize source/git buildpack path. +# Simulates what bin/finalize does: build javaexec into a temp dir and pass +# the path via JAVAEXEC_BINARY_PATH. Verifies the binary builds, that +# InstallJavaexecLauncher picks up the override, and that javaexec tokenizes +# JAVA_OPTS correctly when actually invoked. +set -euo pipefail + +BUILDPACK_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +cd "$BUILDPACK_DIR" + +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +echo "==> [1/4] Build javaexec from source (as bin/finalize now does)" +go build -mod=vendor -o "$tmpdir/javaexec" ./src/java/javaexec/cli +echo " OK: $tmpdir/javaexec ($(wc -c < "$tmpdir/javaexec") bytes)" + +echo "" +echo "==> [2/4] Build finalize from source" +go build -mod=vendor -o "$tmpdir/finalize" ./src/java/finalize/cli +echo " OK: $tmpdir/finalize" + +echo "" +echo "==> [3/4] Unit tests: InstallJavaexecLauncher with JAVAEXEC_BINARY_PATH override" +go test ./src/java/finalize/ -count=1 -v -run "javaexec launcher" 2>&1 | grep -E "PASS|FAIL|RUN|---" + +echo "" +echo "==> [4/4] Tokenization smoke test: run javaexec with a fake java binary" + +# Fake java: prints each received argument on its own line. +cat > "$tmpdir/fake-java" << 'EOF' +#!/bin/bash +printf '%s\n' "$@" +EOF +chmod +x "$tmpdir/fake-java" + +# Quoted value with spaces → one token; cron expr with * → literal; $(...) → not executed. +JAVA_OPTS='-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dwhere=$(hostname)' \ + "$tmpdir/javaexec" "$tmpdir/fake-java" -jar app.jar 2>/dev/null > "$tmpdir/actual.txt" + +expected="-Dfoo=bar baz +-DcronSched=0 */7 * * * * +-Dwhere=\$(hostname) +-jar +app.jar" + +actual=$(cat "$tmpdir/actual.txt") + +if [ "$actual" = "$expected" ]; then + echo " OK: all tokens correct" +else + echo " FAIL: unexpected output" + echo " expected:" + printf '%s\n' "$expected" | sed 's/^/ /' + echo " got:" + printf '%s\n' "$actual" | sed 's/^/ /' + exit 1 +fi + +echo "" +echo "PASS: source/git buildpack path works." +echo " bin/finalize builds javaexec and passes it via JAVAEXEC_BINARY_PATH." +echo " javaexec tokenizes JAVA_OPTS correctly without shell execution." diff --git a/src/integration/README.md b/src/integration/README.md index 9ecfd7d384..e4580ba095 100644 --- a/src/integration/README.md +++ b/src/integration/README.md @@ -124,6 +124,39 @@ The integration tests cover: - Cached buildpack deployment - No internet access scenarios +### Note: JAVA_OPTS character-fidelity is tested at the unit level, not here (#1301) + +The `#1301` work removed `eval` from the start command: `JAVA_OPTS` is now assembled +by a pure-bash expander and tokenized at launch by the shell-free `javaexec` launcher. +The exact-character behaviour this guarantees — shell metacharacters (`; & | > `), +glob/cron stars, quotes-with-spaces, and backslashes all reaching the JVM as literal +text — is verified **deterministically at the unit level** in +[`../java/frameworks/java_opts_writer_test.go`](../java/frameworks/java_opts_writer_test.go), +which runs the **real** generated assembly script and the **real** `javaexec` +tokenizer end-to-end. + +Do **not** re-assert those exact characters through a docker deployment here. The +switchblade docker harness passes env vars into the container in a way that mangles +some metacharacters (e.g. a user `JAVA_OPTS` `&` was observed arriving as `\&` inside +the container) — that is a **harness artifact, not buildpack behaviour** (the unit +test above confirms the buildpack delivers a clean `&`). A docker integration test +asserting the literal received value would fail for reasons unrelated to the product. + +What this directory **does** cover for `#1301`, because it is a genuine end-to-end +property that must hold in a real container launch: + +- **Command substitution is never executed** — a `$(...)` in user `JAVA_OPTS` reaches + the JVM as literal text (`SpringBoot` suite, asserted via the fixture's `/jvm-args` + endpoint). +- **The `javaexec` launcher actually starts the JVM** for the affected containers, + including `Play` (previously `eval exec java $JAVA_OPTS ...`). + +**Source/git buildpack path** (`bin/finalize` building `javaexec` on the fly) is +**not covered** by integration tests here: they use a packaged zip where `bin/javaexec` +is pre-built by `scripts/build.sh`. The source/git path is covered by unit tests in +[`../java/finalize/finalize_test.go`](../java/finalize/finalize_test.go) and can be +verified locally with `bash scripts/test-javaexec-source-path.sh`. + ## Writing New Tests To add a new test: diff --git a/src/integration/java_main_test.go b/src/integration/java_main_test.go index 954ddd83fe..0173930500 100644 --- a/src/integration/java_main_test.go +++ b/src/integration/java_main_test.go @@ -126,7 +126,7 @@ func testJavaMain(platform switchblade.Platform, fixtures string) func(*testing. // Verify SAPMachine JRE was installed from manifest Expect(logs.String()).To(ContainSubstring("Java Buildpack")) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine")) Expect(logs.String()).To(ContainSubstring("17.")) }) }) diff --git a/src/integration/play_test.go b/src/integration/play_test.go index baeb04acb1..54dd7c9076 100644 --- a/src/integration/play_test.go +++ b/src/integration/play_test.go @@ -100,6 +100,23 @@ func testPlay(platform switchblade.Platform, fixtures string) func(*testing.T, s Eventually(deployment).Should(matchers.Serve(Not(BeEmpty()))) }) + // Regression test for #1301: the Play start command now launches the JVM via + // the shell-free javaexec launcher (previously `eval exec java $JAVA_OPTS ...`). + // A command substitution and cron/glob characters in user JAVA_OPTS must not be + // executed or expanded, and must not break launch — the app must still boot. + it("starts via the javaexec launcher with unsafe JAVA_OPTS (#1301)", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_JAVA_VERSION": "11", + "JAVA_OPTS": `-DcronExpr="0 */7 * * * *" -Dinject=$(hostname)`, + }). + Execute(name, filepath.Join(fixtures, "containers", "play_2.2_staged")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + Expect(logs.String()).To(ContainSubstring("Java Buildpack")) + Eventually(deployment).Should(matchers.Serve(Not(BeEmpty()))) + }) + it("handles Play 2.2 application without bat file", func() { deployment, logs, err := platform.Deploy. WithEnv(map[string]string{ diff --git a/src/integration/spring_boot_test.go b/src/integration/spring_boot_test.go index 7ad542342f..fc691a718a 100644 --- a/src/integration/spring_boot_test.go +++ b/src/integration/spring_boot_test.go @@ -156,6 +156,43 @@ func testSpringBoot(platform switchblade.Platform, fixtures string) func(*testin Eventually(deployment).Should(matchers.Serve(ContainSubstring("Hello from Spring Boot"))) }) + // Regression tests for #1301: the start command no longer uses `eval`, and + // JAVA_OPTS is assembled by a pure-bash expander and tokenized at launch by + // the shell-free javaexec launcher. A user-supplied JAVA_OPTS value must + // therefore reach the JVM as literal text — command substitutions are never + // executed, and globs/cron stars/shell metacharacters are never expanded or + // interpreted as operators. These drive the value through the user JAVA_OPTS + // env path (from_environment: true; memory comes from the configured opts), + // then read the JVM's actual received value back via the fixture's /jvm-args + // endpoint (System.getProperty("userProperty")). + memoryOpts := `java_opts: ["-Xmx256M", "-Xms128M", "-Xss512k", "-XX:ReservedCodeCacheSize=120M", "-XX:MetaspaceSize=78643K", "-XX:MaxMetaspaceSize=157286K"]` + + it("does not execute command substitution in JAVA_OPTS (#1301, no eval)", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_JAVA_VERSION": "17", + "JBP_CONFIG_JAVA_OPTS": `{from_environment: true, ` + memoryOpts + `}`, + // $(hostname) would run under the old eval start command. It must + // instead arrive at the JVM verbatim. + "JAVA_OPTS": `-DuserProperty=$(hostname)`, + }). + Execute(name, filepath.Join(fixtures, "containers", "spring_boot_staged")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + Eventually(deployment).Should(matchers.Serve( + ContainSubstring("userProperty=$(hostname)"), // literal, not the executed hostname + ).WithEndpoint("/jvm-args")) + }) + + // NOTE: glob/cron preservation and shell-metacharacter/ampersand/backslash + // fidelity are covered deterministically at the unit level in + // frameworks/java_opts_writer_test.go, which runs the real assembly script + // and the real javaexec tokenizer. They are intentionally not duplicated as + // docker integration tests (the switchblade docker harness mangles some + // metacharacters when passing env vars, which is a harness artifact, not + // buildpack behaviour). The command-substitution case above stays here + // because non-execution is the security property worth proving end-to-end. + it("applies framework opts without any configured JAVA_OPTS", func() { deployment, logs, err := platform.Deploy. WithEnv(map[string]string{ diff --git a/src/integration/tomcat_test.go b/src/integration/tomcat_test.go index 6e0aa362ae..2952e3be12 100644 --- a/src/integration/tomcat_test.go +++ b/src/integration/tomcat_test.go @@ -171,7 +171,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (8.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -185,7 +185,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -198,7 +198,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -224,7 +224,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -237,7 +237,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) @@ -258,7 +258,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10.1.")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.1.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) @@ -371,7 +371,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (17.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (17.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -384,7 +384,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (21.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (21.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -397,7 +397,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (25.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (25.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) diff --git a/src/java/containers/container.go b/src/java/containers/container.go index e16fefe05d..9f40bb800d 100644 --- a/src/java/containers/container.go +++ b/src/java/containers/container.go @@ -127,6 +127,24 @@ func (r *Registry) RegisterStandardContainers() { r.Register(NewJavaMainContainer(r.context)) } +// JavaExecCommand builds a start command of the form: +// +// exec $DEPS_DIR//bin/javaexec "$JAVA_HOME/bin/java" +// +// The javaexec launcher reads JAVA_OPTS from the environment and tokenizes it +// without a shell, so glob characters, quotes, and shell metacharacters in +// user-provided JAVA_OPTS are never expanded or executed. This replaces the +// previous `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS "` form, which +// let the shell glob-expand $JAVA_OPTS and execute embedded command +// substitutions. +// +// javaArgs are buildpack-generated (jar paths, Main-Class, classpath, and +// shell expansions like ${CLASSPATH} or BOOT-INF/lib/*) and are placed on a +// normal command line, where the shell expands them exactly as before. +func JavaExecCommand(depsIdx, javaArgs string) string { + return `exec $DEPS_DIR/` + depsIdx + `/bin/javaexec "$JAVA_HOME/bin/java" ` + javaArgs +} + // This script is used to process the CLASSPATH assembled from various framework scripts sourced from profile.d // to further create symlinks to the corresponding framework dependencies in WEB-INF/lib, BOOT-INF/lib and where ever // needed thus they are available for application classloading diff --git a/src/java/containers/java_main.go b/src/java/containers/java_main.go index 99570f3394..bd46f4dfc8 100644 --- a/src/java/containers/java_main.go +++ b/src/java/containers/java_main.go @@ -286,19 +286,20 @@ func (j *JavaMainContainer) Release() (string, error) { args := "" if cfg.Arguments != "" { + // Passed through as-is onto the start command line. The command is parsed + // once by the shell at launch (no eval), so any quoting in the arguments is + // applied exactly once, like a normal shell command line. args = " " + cfg.Arguments } // JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class. // Use classpath mode so the configured class is actually invoked (not the manifest's). if cfg.JavaMainClass != "" { - 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 + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args)), nil } if j.jarFile != "" { - // JAR has its own Main-Class in the manifest — java -jar handles it - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -jar %s%s", j.jarFile, args), nil + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-jar %s%s", j.jarFile, args)), nil } // Classpath mode: need an explicit main class @@ -311,6 +312,5 @@ func (j *JavaMainContainer) Release() (string, error) { j.context.Log.Debug("Main Class %s found in JAVA_MAIN_CLASS", mainClass) } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args), nil + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args)), nil } diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index 2a0d0f0cd9..3835ea657b 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -154,6 +154,13 @@ var _ = Describe("Java Main Container", func() { }) Describe("Release", func() { + expectSafeLaunch := func(cmd string) { + Expect(cmd).To(ContainSubstring("/bin/javaexec")) + Expect(cmd).To(ContainSubstring(`"$JAVA_HOME/bin/java"`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) + } + Context("with JAR file", func() { BeforeEach(func() { Expect(createJar( @@ -278,6 +285,78 @@ var _ = Describe("Java Main Container", func() { Expect(err).NotTo(HaveOccurred()) Expect(cmd).To(ContainSubstring("--foo=bar")) }) + + It("passes double quotes in arguments through unescaped", func() { + os.Setenv("JBP_CONFIG_JAVA_MAIN", `{arguments: "--spring.datasource.url=\"jdbc:h2:mem:test\""}`) + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + // No eval: the start command is parsed once by the shell, which + // consumes the quotes, so they must NOT be backslash-escaped. + Expect(cmd).To(ContainSubstring(`--spring.datasource.url="jdbc:h2:mem:test"`)) + }) + }) + + // Regression tests for issue #1301: start command must use the javaexec + // launcher and keep $JAVA_OPTS out of the command, so glob chars in + // JAVA_OPTS are never expanded by the shell. + Context("with JAR file (javaexec launch)", func() { + BeforeEach(func() { + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectSafeLaunch(cmd) + }) + }) + + Context("with JAVA_MAIN_CLASS env variable (javaexec launch)", func() { + BeforeEach(func() { + os.Setenv("JAVA_MAIN_CLASS", "com.example.Main") + os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JAVA_MAIN_CLASS") + }) + + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectSafeLaunch(cmd) + }) + }) + + Context("with JBP_CONFIG_JAVA_MAIN java_main_class (javaexec launch)", func() { + BeforeEach(func() { + os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}") + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JBP_CONFIG_JAVA_MAIN") + }) + + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectSafeLaunch(cmd) + }) }) Context("without main class or JAR", func() { @@ -351,6 +430,7 @@ var _ = Describe("Java Main Container", func() { Expect(cmd).To(ContainSubstring(".")) }) }) + }) Describe("Finalize", func() { diff --git a/src/java/containers/play.go b/src/java/containers/play.go index 2dc37717ad..c2860158d2 100644 --- a/src/java/containers/play.go +++ b/src/java/containers/play.go @@ -548,8 +548,9 @@ func (p *PlayContainer) Release() (string, error) { libPath = filepath.ToSlash(relPath) } } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - cmd = fmt.Sprintf("eval exec java $JAVA_OPTS -cp $HOME/%s/* play.core.server.NettyServer $HOME", libPath) + // JAVA_OPTS is applied by the javaexec launcher, which tokenizes it + // without a shell (no glob/word-splitting/command execution). + 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) } p.context.Log.Debug("Play Framework release command: %s", cmd) diff --git a/src/java/containers/play_test.go b/src/java/containers/play_test.go index d4d225434d..29dc76e5e7 100644 --- a/src/java/containers/play_test.go +++ b/src/java/containers/play_test.go @@ -185,7 +185,9 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp")) + Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) @@ -201,7 +203,9 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp")) + Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) diff --git a/src/java/containers/spring_boot.go b/src/java/containers/spring_boot.go index ac6119c1c0..b584973a7f 100644 --- a/src/java/containers/spring_boot.go +++ b/src/java/containers/spring_boot.go @@ -256,21 +256,16 @@ func (s *SpringBootContainer) Release() (string, error) { // Verify this is actually a Spring Boot application if s.isSpringBootExplodedJar(buildDir) { - // True Spring Boot exploded JAR - use main class from manifest or fallback to JarLauncher based on spring-boot version launcherClass := s.getLauncherClass(buildDir) - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass), nil + return JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("-cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass)), nil } - // Exploded JAR but NOT Spring Boot - use Main-Class from MANIFEST.MF mainClass, err := s.readMainClassFromManifest(buildDir) if err != nil { s.context.Log.Debug("Could not read MANIFEST.MF: %s", err.Error()) } if mainClass != "" { - // Use classpath from BOOT-INF/classes and BOOT-INF/lib - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass), nil + return JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("-cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass)), nil } return "", fmt.Errorf("exploded JAR found but no Main-Class in MANIFEST.MF") @@ -292,8 +287,8 @@ func (s *SpringBootContainer) Release() (string, error) { jarFile = jar } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - cmd := fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS ${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile) + // JAVA_OPTS is applied by the javaexec launcher (see JavaExecCommand) (#1301) + cmd := JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile)) return cmd, nil } diff --git a/src/java/containers/spring_boot_test.go b/src/java/containers/spring_boot_test.go index 752dd1ec74..9b650cd937 100644 --- a/src/java/containers/spring_boot_test.go +++ b/src/java/containers/spring_boot_test.go @@ -122,6 +122,13 @@ var _ = Describe("Spring Boot Container", func() { }) Describe("Release", func() { + expectSafeLaunch := func(cmd string) { + Expect(cmd).To(ContainSubstring("/bin/javaexec")) + Expect(cmd).To(ContainSubstring(`"$JAVA_HOME/bin/java"`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) + } + Context("with exploded JAR (BOOT-INF)", func() { BeforeEach(func() { os.MkdirAll(filepath.Join(buildDir, "BOOT-INF"), 0755) @@ -136,6 +143,12 @@ var _ = Describe("Spring Boot Container", func() { Expect(err).NotTo(HaveOccurred()) Expect(cmd).To(ContainSubstring("JarLauncher")) }) + + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectSafeLaunch(cmd) + }) }) Context("with Spring Boot JAR", func() { @@ -151,6 +164,12 @@ var _ = Describe("Spring Boot Container", func() { Expect(cmd).To(ContainSubstring("java")) Expect(cmd).To(ContainSubstring("app-boot.jar")) }) + + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectSafeLaunch(cmd) + }) }) Context("with no Spring Boot JAR found", func() { diff --git a/src/java/finalize/cli/main.go b/src/java/finalize/cli/main.go index 3b26be4ef9..770dbd78b6 100644 --- a/src/java/finalize/cli/main.go +++ b/src/java/finalize/cli/main.go @@ -52,6 +52,7 @@ func main() { logger.Error("Unable to initialize finalizer from supply config: %s", err.Error()) os.Exit(11) } + f.BuildpackDir = buildpackDir if err = finalize.Run(f); err != nil { os.Exit(12) diff --git a/src/java/finalize/finalize.go b/src/java/finalize/finalize.go index 9cb7d444c1..16b50598f8 100644 --- a/src/java/finalize/finalize.go +++ b/src/java/finalize/finalize.go @@ -24,6 +24,7 @@ type Finalizer struct { JRE jres.JRE ContainerName string JREName string + BuildpackDir string } // SupplyConfig holds the values written to config.yml by the supply phase. @@ -113,6 +114,12 @@ func Run(f *Finalizer) error { return err } + // Install the javaexec launcher binary used by the start command. + if err := f.InstallJavaexecLauncher(); err != nil { + f.Log.Error("Failed to install javaexec launcher: %s", err.Error()) + return err + } + // Write release YAML configuration if err := f.writeReleaseYaml(container); err != nil { f.Log.Error("Failed to write release YAML: %s", err.Error()) @@ -191,6 +198,60 @@ func (f *Finalizer) finalizeFrameworks(ctx *common.Context) error { return nil } +// InstallJavaexecLauncher copies the javaexec launcher binary into the +// dependency directory so it is present in the droplet at runtime. The start +// command (see containers.JavaExecCommand) invokes it as +// $DEPS_DIR//bin/javaexec to tokenize JAVA_OPTS without a shell. +// +// Source path resolution (first match wins): +// 1. JAVAEXEC_BINARY_PATH env var — set by bin/finalize for source/git usage +// where javaexec is built into a temp dir alongside the finalize binary. +// 2. /bin/javaexec — present in packaged buildpacks built by +// scripts/build.sh. +// +// The binary is required: missing it means the generated start command would +// fail at launch, so this returns an error rather than continuing. +func (f *Finalizer) InstallJavaexecLauncher() error { + buildpackDir := f.BuildpackDir + if buildpackDir == "" { + var err error + buildpackDir, err = libbuildpack.GetBuildpackDir() + if err != nil { + return fmt.Errorf("unable to determine buildpack directory for javaexec launcher: %w", err) + } + } + + src := filepath.Join(buildpackDir, "bin", "javaexec") + // For source/git buildpack usage the bin/finalize wrapper builds javaexec + // into a temp directory and passes the path here so the source checkout is + // not mutated. Prefer the override; fall back to /bin/javaexec + // for packaged buildpacks where scripts/build.sh provides the binary. + if override := os.Getenv("JAVAEXEC_BINARY_PATH"); override != "" { + src = override + } + data, err := os.ReadFile(src) + if err != nil { + return fmt.Errorf("javaexec launcher binary not found at %s (required by the start command): %w", src, err) + } + + dstDir := filepath.Join(f.Stager.DepDir(), "bin") + if err := os.MkdirAll(dstDir, 0755); err != nil { + return fmt.Errorf("failed to create bin directory: %w", err) + } + + dst := filepath.Join(dstDir, "javaexec") + if err := os.WriteFile(dst, data, 0755); err != nil { + return fmt.Errorf("failed to write javaexec launcher: %w", err) + } + // WriteFile does not change the mode of an existing file; ensure it is executable. + if err := os.Chmod(dst, 0755); err != nil { + return fmt.Errorf("failed to make javaexec launcher executable: %w", err) + } + + f.Log.Debug("Installed javaexec launcher: %s", dst) + return nil +} + // writeReleaseYaml writes the release configuration to a YAML file func (f *Finalizer) writeReleaseYaml(container containers.Container) error { f.Log.BeginStep("Writing release configuration") diff --git a/src/java/finalize/finalize_test.go b/src/java/finalize/finalize_test.go index b13a192fe5..d756f2c8d0 100644 --- a/src/java/finalize/finalize_test.go +++ b/src/java/finalize/finalize_test.go @@ -55,6 +55,11 @@ dependencies: [] ` Expect(os.WriteFile(manifestFile, []byte(manifestContent), 0644)).To(Succeed()) + // The finalizer installs the javaexec launcher from /bin. + binDir := filepath.Join(buildpackDir, "bin") + Expect(os.MkdirAll(binDir, 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(binDir, "javaexec"), []byte("#!/bin/sh\n"), 0755)).To(Succeed()) + logger = libbuildpack.NewLogger(GinkgoWriter) mockCtrl = gomock.NewController(GinkgoT()) @@ -67,11 +72,12 @@ dependencies: [] stager = libbuildpack.NewStager([]string{buildDir, cacheDir, depsDir, depsIdx}, logger, manifest) finalizer = &finalize.Finalizer{ - Stager: stager, - Manifest: mockManifest, - Installer: mockInstaller, - Log: logger, - Command: &libbuildpack.Command{}, + Stager: stager, + Manifest: mockManifest, + Installer: mockInstaller, + Log: logger, + Command: &libbuildpack.Command{}, + BuildpackDir: buildpackDir, } }) @@ -236,4 +242,43 @@ dependencies: [] Expect(depDir).To(ContainSubstring(depsDir)) }) }) + + Describe("javaexec launcher installation", func() { + It("installs launcher from buildpack bin/ for packaged buildpack usage", func() { + // Default path: /bin/javaexec already written in BeforeEach. + Expect(finalizer.InstallJavaexecLauncher()).To(Succeed()) + installed := filepath.Join(stager.DepDir(), "bin", "javaexec") + Expect(installed).To(BeARegularFile()) + }) + + It("installs launcher from JAVAEXEC_BINARY_PATH for source/git buildpack usage", func() { + // Simulate bin/finalize wrapper: javaexec built into a temp dir, path passed via env var. + // /bin/javaexec is still present from BeforeEach, but the env var must take precedence. + altBinary, err := os.CreateTemp("", "javaexec-alt-*") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(altBinary.Name()) + _, err = altBinary.WriteString("#!/bin/sh\n# alt\n") + Expect(err).NotTo(HaveOccurred()) + Expect(altBinary.Close()).To(Succeed()) + + os.Setenv("JAVAEXEC_BINARY_PATH", altBinary.Name()) + defer os.Unsetenv("JAVAEXEC_BINARY_PATH") + + Expect(finalizer.InstallJavaexecLauncher()).To(Succeed()) + installed := filepath.Join(stager.DepDir(), "bin", "javaexec") + Expect(installed).To(BeARegularFile()) + content, err := os.ReadFile(installed) + Expect(err).NotTo(HaveOccurred()) + Expect(string(content)).To(ContainSubstring("# alt")) + }) + + It("fails when launcher is missing and JAVAEXEC_BINARY_PATH is not set", func() { + // Point BuildpackDir at an empty dir so bin/javaexec doesn't exist. + emptyDir, err := os.MkdirTemp("", "finalize-no-javaexec") + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(emptyDir) + finalizer.BuildpackDir = emptyDir + Expect(finalizer.InstallJavaexecLauncher()).NotTo(Succeed()) + }) + }) }) diff --git a/src/java/frameworks/azure_application_insights_agent.go b/src/java/frameworks/azure_application_insights_agent.go index 914dc14b6a..00f29610f7 100644 --- a/src/java/frameworks/azure_application_insights_agent.go +++ b/src/java/frameworks/azure_application_insights_agent.go @@ -168,7 +168,8 @@ func (a *AzureApplicationInsightsAgentFramework) Finalize() error { // Set connection string if available if credentials.ConnectionString != "" { - // Single-quote the value so eval exec java $JAVA_OPTS does not split on ; in the connection string + // Single-quote the value so the javaexec launcher keeps it as one token + // (the connection string contains ; and spaces). opts = append(opts, fmt.Sprintf("-Dapplicationinsights.connection.string='%s'", credentials.ConnectionString)) } else if credentials.InstrumentationKey != "" { // Fallback to instrumentation key diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 4bfd0a6d12..77b9a0d287 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -73,31 +73,129 @@ func CreateJavaOptsAssemblyScript(ctx *common.Context) error { # Expands runtime variables like $DEPS_DIR, $HOME, $JAVA_OPTS, and all other environment variables # Save original JAVA_OPTS from environment (user-provided) -# Normalize to single line: YAML block scalars (>) may introduce newlines -# xargs trims leading/trailing whitespace and collapses internal spaces -USER_JAVA_OPTS=$(echo "$JAVA_OPTS" | tr '\n' ' ' | tr -s ' ' | xargs) +# Normalize to single line: YAML block scalars (>) may introduce newlines; Windows-edited +# manifests may use CRLF (\r\n). Strip \r first, then convert \n to spaces. +# Do not use xargs — it strips quotes and backslashes. +USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr -d '\r' | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" +# Expand $VAR and ${VAR} references in a string using only bash builtins. +# Unlike eval, this NEVER executes command substitutions ($(...) or backticks); +# only environment-variable references are expanded. It is also dependency-free +# (envsubst from gettext-base is not available on the cflinuxfs stacks). +# Expansion is single-pass: substituted values are not re-scanned for further +# references, matching the previous eval-based behavior. +_expand_env_vars() { + local s="$1" out="" name + while [[ "$s" =~ ^([^$]*)\$(\{([A-Za-z_][A-Za-z0-9_]*)\}|([A-Za-z_][A-Za-z0-9_]*))? ]]; do + out+="${BASH_REMATCH[1]}" + name="${BASH_REMATCH[3]}${BASH_REMATCH[4]}" + if [ -n "$name" ]; then + out+="${!name}" + else + out+='$' + fi + s="${s#"${BASH_REMATCH[0]}"}" + done + out+="$s" + printf '%%s' "$out" +} + +# The buildpack itself emits the literal command substitution $(nproc) in its +# base JAVA_OPTS (e.g. -XX:ActiveProcessorCount=$(nproc)). The pure-bash +# expander deliberately does not execute command substitutions, so resolve this +# single known, trusted token here by computing the processor count once and +# substituting it literally below. All other command substitutions remain +# literal (and therefore inert). +_nproc_count="$(nproc 2>/dev/null || echo 1)" +# Backtick char, computed here because this script lives inside a Go raw string. +_backtick="$(printf '\140')" +# Placeholder for \$ escape sequences (Ruby buildpack parity: \$VAR → literal $VAR). +_escaped_dollar_placeholder='__JAVA_OPTS_ESCAPED_DOLLAR__' +_dollar='$' + +# Expand $VAR / ${VAR} references in user JAVA_OPTS (e.g. $PWD, $HOME), matching +# pre-eval behaviour. \$VAR is treated as a literal $ (not expanded), matching the +# Ruby buildpack's eval-based behaviour where \$ suppressed variable expansion. +# Command substitutions are still not executed. +USER_JAVA_OPTS="${USER_JAVA_OPTS//\\\$/$_escaped_dollar_placeholder}" +USER_JAVA_OPTS=$(_expand_env_vars "$USER_JAVA_OPTS") +USER_JAVA_OPTS="${USER_JAVA_OPTS//$_escaped_dollar_placeholder/$_dollar}" + +# Warn if any command substitution remains in user JAVA_OPTS. It will be +# passed literally to the JVM — command substitutions are never executed. +# Show only the offending token, not the full value (which may be long or contain secrets). +case "$USER_JAVA_OPTS" in + *'$('*) + _warn_match="${USER_JAVA_OPTS#*'$('}" + _warn_match="\$(${_warn_match%%%%')'*})" + echo "WARNING: JAVA_OPTS contains command substitution; it will NOT be executed and will be passed literally to the JVM. Matching: ${_warn_match}" >&2 + ;; + *"$_backtick"*) + _warn_match="${USER_JAVA_OPTS#*"$_backtick"}" + _warn_match="${_backtick}${_warn_match%%%%"$_backtick"*}${_backtick}" + echo "WARNING: JAVA_OPTS contains command substitution (backtick); it will NOT be executed and will be passed literally to the JVM. Matching: ${_warn_match}" >&2 + ;; +esac + +# Escape replacement-special chars once; these values are loop-invariant. +_escaped_deps_dir="${DEPS_DIR//\\/\\\\}" +_escaped_deps_dir="${_escaped_deps_dir//&/\\&}" +_escaped_home="${HOME//\\/\\\\}" +_escaped_home="${_escaped_home//&/\\&}" +_user_java_opts_placeholder='__JAVA_OPTS_BUILDPACK_PLACEHOLDER__' +_escaped_user_java_opts="${USER_JAVA_OPTS//\\/\\\\}" +_escaped_user_java_opts="${_escaped_user_java_opts//&/\\&}" + if [ -d "$DEPS_DIR/%s/java_opts" ]; then for opts_file in "$DEPS_DIR/%s/java_opts"/*.opts; do if [ -f "$opts_file" ]; then # Read content and expand runtime variables - opts_content=$(cat "$opts_file") - - # Expand $DEPS_DIR, $HOME, $JAVA_OPTS using bash parameter expansion. - # sed-based substitution breaks when these values contain the sed delimiter (|), - # backslashes, ampersands, or newlines — all valid in JAVA_OPTS and paths. - opts_content="${opts_content//\$DEPS_DIR/$DEPS_DIR}" - opts_content="${opts_content//\$HOME/$HOME}" - opts_content="${opts_content//\$JAVA_OPTS/$USER_JAVA_OPTS}" - - # Expand any remaining environment variables in opts content via eval. - # Note: eval executes commands, but .opts files are written by the buildpack - # at staging time and run within the container context. - # This matches how the Ruby buildpack naturally expanded variables via shell. - opts_content=$(eval "echo \"$opts_content\"") + opts_content=$(< "$opts_file") + + # Shield \$ from expansion so buildpack authors can write \$VAR to pass + # a literal $VAR to the JVM (Ruby buildpack parity). + opts_content="${opts_content//\\\$/$_escaped_dollar_placeholder}" + + # Expand $DEPS_DIR and $HOME using bash parameter expansion. + # In ${var//pattern/repl}, '&' and '\' are special in replacement strings, + # so escape them first to preserve literal path contents. + opts_content="${opts_content//\$DEPS_DIR/$_escaped_deps_dir}" + opts_content="${opts_content//\$HOME/$_escaped_home}" + + # Resolve the trusted $(nproc) token to the computed processor count. + opts_content="${opts_content//\$\(nproc\)/$_nproc_count}" + + # Shield $JAVA_OPTS from expansion: replace with a placeholder first, + # then substitute the actual value AFTER expansion so that quotes and + # backslashes in the user-provided JAVA_OPTS are never reinterpreted. + opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" + + # Expand any remaining environment variables in opts content. + # Use a pure-bash expander instead of eval so that command + # substitutions like $(...) or backticks in .opts content are + # NOT executed — only $VAR / ${VAR} references are expanded. + opts_content=$(_expand_env_vars "$opts_content") + + # Restore \$ escapes to literal $ now that expansion is done. + opts_content="${opts_content//$_escaped_dollar_placeholder/$_dollar}" + + # Defense-in-depth: the buildpack resolves its only known command + # substitution ($(nproc)) above. Any remaining $(...) or backtick at + # this point is an unresolved buildpack-emitted substitution (the + # user's JAVA_OPTS is still a placeholder here) and would reach the + # JVM literally. Warn so such a regression is caught instead of + # silently producing a broken argument. + case "$opts_content" in + *'$('*|*"$_backtick"*) + echo "WARNING: unresolved command substitution in $opts_file; it will be passed to the JVM literally: $opts_content" >&2 + ;; + esac + + # Now safely substitute JAVA_OPTS after expansion (preserves quotes, backslashes, and ampersands) + opts_content="${opts_content//$_user_java_opts_placeholder/$_escaped_user_java_opts}" if [ -n "$opts_content" ]; then JAVA_OPTS="$JAVA_OPTS $opts_content" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index e092ec5df7..ad10bb5fb1 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -1,15 +1,18 @@ package frameworks_test import ( + "bytes" "os" "os/exec" "path/filepath" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/cloudfoundry/java-buildpack/src/java/common" "github.com/cloudfoundry/java-buildpack/src/java/frameworks" + "github.com/cloudfoundry/java-buildpack/src/java/javaexec" "github.com/cloudfoundry/libbuildpack" ) @@ -53,17 +56,8 @@ var _ = Describe("Java Opts Writer", func() { os.RemoveAll(depsDir) }) - Describe("Basic options", func() { - It("writes JAVA_OPTS correctly", func() { - javaOpts := "-Xmx512M -Xms256M" - os.Setenv("JAVA_OPTS", javaOpts) - - Expect(os.Getenv("JAVA_OPTS")).To(Equal(javaOpts)) - }) - }) - Describe("CreateJavaOptsAssemblyScript", func() { - runScript := func(javaOpts string, optsFileContent string) (string, error) { + setupScript := func(_ string, optsFileContent string) string { err := frameworks.CreateJavaOptsAssemblyScript(ctx) Expect(err).NotTo(HaveOccurred()) @@ -71,8 +65,11 @@ var _ = Describe("Java Opts Writer", func() { Expect(os.MkdirAll(optsDir, 0755)).To(Succeed()) Expect(os.WriteFile(filepath.Join(optsDir, "42_agent.opts"), []byte(optsFileContent), 0644)).To(Succeed()) - scriptPath := filepath.Join(depsDir, "0", "profile.d", "00_java_opts.sh") - cmd := exec.Command("bash", "-c", "source "+scriptPath+" && echo \"$JAVA_OPTS\"") + return filepath.Join(depsDir, "0", "profile.d", "00_java_opts.sh") + } + + runWithEnv := func(scriptPath, javaOpts, bashExpr string) (string, error) { + cmd := exec.Command("bash", "-c", "source "+scriptPath+" && "+bashExpr) cmd.Env = append(os.Environ(), "JAVA_OPTS="+javaOpts, "DEPS_DIR="+depsDir, @@ -82,6 +79,50 @@ var _ = Describe("Java Opts Writer", func() { return string(output), err } + runWithCustomRuntimeEnv := func(scriptPath, javaOpts, bashExpr, runtimeDepsDir, runtimeHome string) (string, error) { + cmd := exec.Command("bash", "-c", "source "+scriptPath+" && "+bashExpr) + cmd.Env = append(os.Environ(), + "JAVA_OPTS="+javaOpts, + "DEPS_DIR="+runtimeDepsDir, + "HOME="+runtimeHome, + ) + output, err := cmd.CombinedOutput() + return string(output), err + } + + runScript := func(javaOpts string, optsFileContent string) (string, error) { + scriptPath := setupScript(javaOpts, optsFileContent) + return runWithEnv(scriptPath, javaOpts, `printf '%s\n' "$JAVA_OPTS"`) + } + + // runStartCommand simulates the actual JVM invocation. At launch the + // start command is: + // exec $DEPS_DIR//bin/javaexec "$JAVA_HOME/bin/java" + // where the javaexec launcher tokenizes $JAVA_OPTS without a shell. + // This sources the profile.d assembly script to build $JAVA_OPTS, then + // tokenizes it with the real launcher, returning the argument list java + // would receive (one arg per line). + // Stderr is captured separately so that WARNING messages from the + // assembly script do not pollute the JAVA_OPTS value passed to the + // tokenizer (warnings are diagnostic, not part of the value). + runStartCommand := func(javaOpts string, optsFileContent string) (string, error) { + scriptPath := setupScript(javaOpts, optsFileContent) + cmd := exec.Command("bash", "-c", "source "+scriptPath+` && printf '%s' "$JAVA_OPTS"`) + cmd.Env = append(os.Environ(), + "JAVA_OPTS="+javaOpts, + "DEPS_DIR="+depsDir, + "HOME=/home/vcap/app", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return stderr.String() + stdout.String(), err + } + tokens := javaexec.TokenizeJavaOpts(stdout.String()) + return strings.Join(tokens, "\n") + "\n", nil + } + It("handles multiline JAVA_OPTS from YAML block scalar without sed error", func() { // Reproduce the manifest pattern: // JAVA_OPTS: > @@ -93,6 +134,20 @@ var _ = Describe("Java Opts Writer", func() { output, err := runScript(multilineJavaOpts, "-javaagent:somepath.jar $JAVA_OPTS") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) Expect(output).To(ContainSubstring("-XX:+UseZGC")) + // $HOME must expand even when JAVA_OPTS contains newlines + Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) + Expect(output).NotTo(ContainSubstring("$HOME")) + }) + + It("strips carriage returns (CRLF) from multiline JAVA_OPTS", func() { + // Windows-edited manifests may deliver JAVA_OPTS with \r\n line endings. + // tr '\n' ' ' strips \n but leaves \r, corrupting JVM args (e.g. -Dfoo=bar\r). + crlfJavaOpts := "-Dfoo=bar\r\n-Dbaz=qux\r\n" + output, err := runScript(crlfJavaOpts, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dfoo=bar")) + Expect(output).To(ContainSubstring("-Dbaz=qux")) + Expect(output).NotTo(ContainSubstring("\r")) }) It("handles pipe character in JAVA_OPTS (e.g. javaagent options) without sed error", func() { @@ -112,10 +167,321 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) }) + It("preserves ampersand and backslash in $HOME replacement", func() { + scriptPath := setupScript("", "-javaagent:$HOME/BOOT-INF/lib/agent.jar") + customHome := `/tmp/home&dir\sub` + output, err := runWithCustomRuntimeEnv(scriptPath, "", `printf '%s\n' "$JAVA_OPTS"`, depsDir, customHome) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-javaagent:" + customHome + "/BOOT-INF/lib/agent.jar")) + }) + It("expands $DEPS_DIR in opts file content", func() { output, err := runScript("", "-Djava.security.properties=$DEPS_DIR/0/security.properties") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) Expect(output).To(ContainSubstring("-Djava.security.properties=" + depsDir + "/0/security.properties")) }) + + It("expands arbitrary runtime environment variables in opts file content", func() { + os.Setenv("MY_RUNTIME_VAR", "512m") + defer os.Unsetenv("MY_RUNTIME_VAR") + output, err := runScript("", "-Dmem=$MY_RUNTIME_VAR -Dother=${MY_RUNTIME_VAR}") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dmem=512m -Dother=512m")) + }) + + It("resolves the trusted $(nproc) token to a processor count", func() { + // The buildpack emits -XX:ActiveProcessorCount=$(nproc); it must be + // resolved to a number even though arbitrary command substitutions + // are not executed. + output, err := runScript("", "-XX:ActiveProcessorCount=$(nproc)") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(MatchRegexp(`-XX:ActiveProcessorCount=[0-9]+`)) + Expect(output).NotTo(ContainSubstring("nproc")) + }) + + It("warns when an unresolved command substitution survives in opts content", func() { + // A buildpack-emitted $(...) or backtick other than $(nproc) is not + // executed and would reach the JVM literally; the script must warn. + output, err := runScript("", "-Dwhen=$(date)") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING: unresolved command substitution")) + + output, err = runScript("", "-Dwhen=`date`") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING: unresolved command substitution")) + }) + + It("warning for user JAVA_OPTS command substitution shows matching token, not full value", func() { + // Warning must identify the offending $(...) fragment without dumping + // the entire JAVA_OPTS string (which may be long or contain secrets). + // Use `true` as bashExpr so only the WARNING (stderr) appears in combined output. + scriptPath := setupScript("", "$JAVA_OPTS") + output, err := runWithEnv(scriptPath, + `-Dsafe=before $( hostname | wc -l ) -Dsafe=after`, + `true`, + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING")) + // Warning shows the $(...) fragment + Expect(output).To(ContainSubstring("$(")) + Expect(output).To(ContainSubstring("hostname")) + // Warning must NOT dump the full JAVA_OPTS value + Expect(output).NotTo(ContainSubstring("-Dsafe=before")) + Expect(output).NotTo(ContainSubstring("-Dsafe=after")) + }) + + It("does not execute command substitutions embedded in opts file content", func() { + marker := filepath.Join(cacheDir, "pwned_marker") + Expect(os.Remove(marker)).To(Or(Succeed(), MatchError(os.ErrNotExist))) + output, err := runScript("", "-Dx=$(touch "+marker+")") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + _, statErr := os.Stat(marker) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "command substitution was executed; marker file created") + Expect(output).To(ContainSubstring("$(touch")) + }) + + It("preserves literal -n from opts file content", func() { + output, err := runScript("", "-n") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(strings.TrimSpace(output)).To(Equal("-n")) + }) + + It("does not execute command substitution in user JAVA_OPTS (#1301, no eval)", func() { + // The #1301 user-facing scenario: a command substitution in the + // user-provided JAVA_OPTS env must reach java as a literal argument, + // never be executed. Proven end-to-end through the real assembly script + // and the real javaexec tokenizer, with a marker file as a second guard. + marker := filepath.Join(cacheDir, "user_pwned_marker") + Expect(os.Remove(marker)).To(Or(Succeed(), MatchError(os.ErrNotExist))) + // Quoted so the value (which contains a space) stays a single argument. + output, err := runStartCommand(`-Dx="$(touch `+marker+`)"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + _, statErr := os.Stat(marker) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "command substitution was executed; marker file created") + // java receives the single literal argument, $(...) intact. + Expect(strings.TrimSpace(output)).To(Equal("-Dx=$(touch " + marker + ")")) + }) + + It(`treats \$VAR as a literal $ in user JAVA_OPTS (Ruby buildpack parity)`, func() { + // Ruby buildpack: eval treated \$VAR as literal $VAR (not expanded). + // Users migrating with \$HOME should get $HOME literally, not the expanded path. + // The test helper sets HOME=/home/vcap/app so we can confirm non-expansion. + output, err := runScript(`-Dfoo=\$HOME`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo=$HOME`)) + Expect(output).NotTo(ContainSubstring("-Dfoo=/home/vcap/app")) + }) + + It(`treats \$VAR as a literal $ in opts file content (Ruby buildpack parity)`, func() { + output, err := runScript("", `-Dexample=\$MY_VAR`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dexample=$MY_VAR`)) + }) + + It("expands $VAR references in user JAVA_OPTS", func() { + // $PWD in user JAVA_OPTS must expand to the working directory, + // matching pre-eval behaviour. Command substitutions must still not execute. + wd, _ := os.Getwd() + output, err := runScript( + `-Dapplicationinsights.configuration.file=$PWD/BOOT-INF/classes/applicationinsights.json`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dapplicationinsights.configuration.file=" + wd + "/BOOT-INF/classes/applicationinsights.json")) + Expect(output).NotTo(ContainSubstring("$PWD")) + }) + + It("expands $HOME in user JAVA_OPTS (javaagent path use case)", func() { + output, err := runScript( + `-javaagent:$HOME/BOOT-INF/lib/agent.jar`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) + Expect(output).NotTo(ContainSubstring("$HOME")) + }) + + It("expands ${VAR} braces form in user JAVA_OPTS", func() { + wd, _ := os.Getwd() + output, err := runScript( + `-Dconfig.file=${PWD}/config/app.properties`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dconfig.file=" + wd + "/config/app.properties")) + Expect(output).NotTo(ContainSubstring("${PWD}")) + }) + + // Regression tests for issue #1301: xargs strips quotes, breaking quoted JVM args + It("preserves quoted value with spaces in JAVA_OPTS", func() { + // JAVA_OPTS='-Dfoo="bar baz"' — xargs removes the quotes from USER_JAVA_OPTS, + // so when eval exec java $JAVA_OPTS is called, -Dfoo=bar and baz become separate args + output, err := runScript(`-Dfoo="bar baz"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo="bar baz"`)) + }) + + It("preserves cron expression with glob characters in JAVA_OPTS", func() { + // JAVA_OPTS='-DcronSched="0 */7 * * * *"' — xargs strips quotes, then * expands via glob + // when eval exec java $JAVA_OPTS is invoked, corrupting the cron expression + output, err := runScript(`-DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-DcronSched="0 */7 * * * *"`)) + }) + + It("preserves multiple quoted args in JAVA_OPTS", func() { + // Multiple quoted values — xargs strips all quotes, each space-containing value splits + output, err := runScript(`-Dfoo="bar baz" -Dother="qux quux"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo="bar baz"`)) + Expect(output).To(ContainSubstring(`-Dother="qux quux"`)) + }) + + It("preserves backslashes in JAVA_OPTS values", func() { + // xargs treats backslash as escape char: C:\path\to\app -> C:pathtoapp + // Affects regex patterns and any path using backslash notation + output, err := runScript(`-DregEx="[a-z]+(.*)" -Dpattern=foo\|bar`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-DregEx="[a-z]+(.*)"`)) + Expect(output).To(ContainSubstring(`foo\|bar`)) + }) + + It("preserves leading -n argument in JAVA_OPTS", func() { + output, err := runScript(`-n -Dfoo=bar`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-n -Dfoo=bar`)) + }) + + It("preserves ampersand in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dfoo=a&b`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo=a&b`)) + }) + + It("preserves backslashes in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dpath=C:\tmp\app`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpath=C:\tmp\app`)) + }) + + It("preserves double backslashes in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dpath=C:\\double`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpath=C:\\double`)) + }) + + // Full invocation cycle tests for issue #1301: + // Verify that javaexec tokenizes $JAVA_OPTS without a shell, so glob chars, + // pipes, and shell metacharacters are never expanded or executed. + It("does not glob-expand * in cron expression when invoking java", func() { + output, err := runStartCommand(`-DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + // Java receives exactly one arg: -DcronSched=0 */7 * * * * + Expect(strings.TrimSpace(output)).To(Equal("-DcronSched=0 */7 * * * *")) + }) + + It("delivers exact issue-#1301 reproducer: quoted value + cron as separate args", func() { + // Exact reproducer from the bug report: + // JAVA_OPTS='-Dfoo="bar baz" -DcronSched="0 */7 * * * *"' + // Old xargs path: quotes stripped → "bar baz" splits → * globs → ClassNotFoundException + output, err := runStartCommand(`-Dfoo="bar baz" -DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dfoo=bar baz")) + Expect(lines[1]).To(Equal("-DcronSched=0 */7 * * * *")) + }) + + It("passes pipe character in JAVA_OPTS through to java without shell interpretation", func() { + // Old sed-based assembly (5.0.2) used | as sed delimiter, so a | in JAVA_OPTS + // caused a sed syntax error and dropped options. javaexec never invokes a shell. + output, err := runStartCommand(`-Dpattern=foo|bar -Dother=baz`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dpattern=foo|bar")) + Expect(lines[1]).To(Equal("-Dother=baz")) + }) + + It("passes shell metacharacters (&, ;, >) through to java as literals", func() { + output, err := runStartCommand(`-Da=x&y -Db=a;b -Dc=a>b`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(3)) + Expect(lines[0]).To(Equal("-Da=x&y")) + Expect(lines[1]).To(Equal("-Db=a;b")) + Expect(lines[2]).To(Equal("-Dc=a>b")) + }) + + It("handles full manifest JAVA_OPTS with all edge cases (issue #1301)", func() { + // Reproduces the exact manifest scenario reported by users: + // JAVA_OPTS: >- + // -Dfoo="bar baz" + // -DcronSched="0 */7 * * * *" + // -Dbar=$HOME + // -Dwhere=$( hostname | tr '\n' | curl -v 'https://testasdkjfhakl.me') + // -Dmyfile=c:\\first\\second\\file.txt;ext + // YAML >- folds newlines to spaces, delivering this as one line. + javaOpts := `-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=$HOME -Dwhere=$( hostname | tr '\n' | curl -v 'https://testasdkjfhakl.me') -Dmyfile=c:\\first\\second\\file.txt;ext` + output, err := runStartCommand(javaOpts, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(5)) + Expect(lines[0]).To(Equal("-Dfoo=bar baz")) + Expect(lines[1]).To(Equal("-DcronSched=0 */7 * * * *")) + Expect(lines[2]).To(Equal("-Dbar=/home/vcap/app")) // $HOME expanded by profile.d + // $( hostname | ...) passes literally as one arg — not executed, not split + Expect(lines[3]).To(ContainSubstring("-Dwhere=$(")) + Expect(lines[3]).To(ContainSubstring("hostname")) + // \\ → \ by javaexec; ; is literal + Expect(lines[4]).To(Equal(`-Dmyfile=c:\first\second\file.txt;ext`)) + }) + + // Regression test: eval mangles .opts content containing literal double quotes. + // e.g. Datadog writes -Ddd.service="myapp" into its .opts file; the inner " + // terminates the outer eval "..." string, stripping the value. + It("preserves double-quoted values from .opts file content", func() { + output, err := runScript("", `-Ddd.service="myapp"`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Ddd.service="myapp"`)) + }) + + It("preserves backslashes in .opts file content", func() { + output, err := runScript("", `-Dpattern=foo\|bar`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpattern=foo\|bar`)) + }) + + // Expanded variable values with spaces: same POSIX rules as old eval. + // Unquoted $VAR whose value contains spaces → split (javaexec treats spaces as + // word separators). Double-quoted "$VAR" → one token. Quote your references. + It("splits unquoted expanded $VAR with spaces into separate tokens (POSIX word-split)", func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand(`-Dfoo=$MY_SPACED_VAR`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + // Unquoted: space splits into two JVM arguments, same as old eval. + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dfoo=hello")) + Expect(lines[1]).To(Equal("world")) + }) + + It(`keeps double-quoted "$VAR" with spaces as one JVM argument`, func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand(`-Dfoo="$MY_SPACED_VAR"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + // Double-quoted: javaexec treats the quoted region as one token. + Expect(strings.TrimSpace(output)).To(Equal("-Dfoo=hello world")) + }) + + It(`keeps double-quoted "$VAR" with spaces in .opts content as one JVM argument`, func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand("", `-Dfoo="$MY_SPACED_VAR"`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(strings.TrimSpace(output)).To(Equal("-Dfoo=hello world")) + }) }) }) diff --git a/src/java/javaexec/args.go b/src/java/javaexec/args.go new file mode 100644 index 0000000000..1086b92c12 --- /dev/null +++ b/src/java/javaexec/args.go @@ -0,0 +1,15 @@ +package javaexec + +// BuildArgs assembles the full argv for execing the JVM. The resulting slice is +// [java, , ], mirroring the order of +// the previous `exec java $JAVA_OPTS ` start command. trusted args are +// produced by the buildpack (classpath, -jar/-cp, main class) and are passed +// through unchanged; only JAVA_OPTS is tokenized. +func BuildArgs(java, javaOpts string, trusted []string) []string { + opts := TokenizeJavaOpts(javaOpts) + argv := make([]string, 0, 1+len(opts)+len(trusted)) + argv = append(argv, java) + argv = append(argv, opts...) + argv = append(argv, trusted...) + return argv +} diff --git a/src/java/javaexec/args_test.go b/src/java/javaexec/args_test.go new file mode 100644 index 0000000000..11891e6e03 --- /dev/null +++ b/src/java/javaexec/args_test.go @@ -0,0 +1,23 @@ +package javaexec + +import ( + "reflect" + "testing" +) + +func TestBuildArgs(t *testing.T) { + got := BuildArgs("/jdk/bin/java", `-Xmx1g -Dfoo="bar baz"`, + []string{"-jar", "app.jar"}) + want := []string{"/jdk/bin/java", "-Xmx1g", "-Dfoo=bar baz", "-jar", "app.jar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("BuildArgs = %#v, want %#v", got, want) + } +} + +func TestBuildArgsEmptyOptsHasNoEmptyArg(t *testing.T) { + got := BuildArgs("/jdk/bin/java", "", []string{"-jar", "app.jar"}) + want := []string{"/jdk/bin/java", "-jar", "app.jar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("BuildArgs with empty JAVA_OPTS = %#v, want %#v", got, want) + } +} diff --git a/src/java/javaexec/cli/main.go b/src/java/javaexec/cli/main.go new file mode 100644 index 0000000000..428512f226 --- /dev/null +++ b/src/java/javaexec/cli/main.go @@ -0,0 +1,48 @@ +package main + +import ( + "fmt" + "os" + "os/exec" + "strings" + "syscall" + + "github.com/cloudfoundry/java-buildpack/src/java/javaexec" +) + +// The javaexec launcher replaces the runtime `eval "exec java $JAVA_OPTS "` +// start command. It is invoked as: +// +// javaexec [trusted args...] +// +// It reads JAVA_OPTS from the environment, tokenizes it without a shell (no +// expansion, no command substitution, no globbing), and execs the JVM with +// [java, , ]. Because it execs, the JVM +// replaces this process: no extra wrapper process remains. +func main() { + if len(os.Args) < 2 { + fmt.Fprintln(os.Stderr, "javaexec: usage: javaexec [args...]") + os.Exit(2) + } + + java := os.Args[1] + trusted := os.Args[2:] + + // syscall.Exec does not search PATH; resolve a bare command name (e.g. the + // Play container's "java") to an absolute path. + if !strings.ContainsRune(java, '/') { + resolved, err := exec.LookPath(java) + if err != nil { + fmt.Fprintf(os.Stderr, "javaexec: cannot find %q on PATH: %v\n", java, err) + os.Exit(127) + } + java = resolved + } + + argv := javaexec.BuildArgs(java, os.Getenv("JAVA_OPTS"), trusted) + + if err := syscall.Exec(java, argv, os.Environ()); err != nil { + fmt.Fprintf(os.Stderr, "javaexec: failed to exec %s: %v\n", java, err) + os.Exit(126) + } +} diff --git a/src/java/javaexec/tokenize.go b/src/java/javaexec/tokenize.go new file mode 100644 index 0000000000..63d3a9d6b4 --- /dev/null +++ b/src/java/javaexec/tokenize.go @@ -0,0 +1,122 @@ +// Package launcher tokenizes JAVA_OPTS and execs the JVM without invoking a +// shell. It exists to remove the runtime `eval "exec java $JAVA_OPTS ..."` +// start command, which lets the shell execute command substitutions and treat +// metacharacters (&, |, ;, newlines) in user-provided JAVA_OPTS as operators. +package javaexec + +import "strings" + +// TokenizeJavaOpts splits a JAVA_OPTS string into individual arguments using +// POSIX shell word-splitting and quote-removal rules, but WITHOUT performing +// any expansion: variable references ($VAR), command substitutions ($(...) and +// backticks), globbing, and operators (&, |, ;, <, >) are all treated as +// ordinary literal text. This yields the same arguments the previous +// `eval`-based start command produced for normal and quoted inputs, while +// never executing embedded commands. +// +// An empty or whitespace-only input yields no arguments (nil), so the JVM is +// never handed a spurious empty argument. +func TokenizeJavaOpts(s string) []string { + var tokens []string + var b strings.Builder + started := false + + flush := func() { + if started { + tokens = append(tokens, b.String()) + b.Reset() + started = false + } + } + + runes := []rune(s) + for i := 0; i < len(runes); i++ { + c := runes[i] + switch c { + case ' ', '\t', '\n', '\r', '\v', '\f': + flush() + case '\'': + started = true + // Single quotes: everything literal until the next single quote. + for i++; i < len(runes) && runes[i] != '\''; i++ { + b.WriteRune(runes[i]) + } + case '"': + started = true + // Double quotes: literal, except backslash escapes a small set. + for i++; i < len(runes) && runes[i] != '"'; i++ { + if runes[i] == '\\' && i+1 < len(runes) { + next := runes[i+1] + switch next { + case '"', '\\', '$', '`': + b.WriteRune(next) + i++ + continue + case '\n': + // Line continuation: drop backslash and newline. + i++ + continue + } + } + b.WriteRune(runes[i]) + } + case '$': + started = true + b.WriteRune(c) + // Keep $(...) and ${...} as unbreakable units so that spaces inside + // do not split the enclosing token. Neither is executed; both pass + // literally to the JVM. + if i+1 < len(runes) { + next := runes[i+1] + var open, close rune + if next == '(' { + open, close = '(', ')' + } else if next == '{' { + open, close = '{', '}' + } + if open != 0 { + i++ + b.WriteRune(open) + depth := 1 + for i++; i < len(runes) && depth > 0; i++ { + ch := runes[i] + if ch == open { + depth++ + } else if ch == close { + depth-- + } + b.WriteRune(ch) + if depth == 0 { + break + } + } + } + } + case '`': + started = true + b.WriteRune(c) + // Keep `...` as one unbreakable unit (same reasoning as $(...)). + for i++; i < len(runes) && runes[i] != '`'; i++ { + b.WriteRune(runes[i]) + } + if i < len(runes) { + b.WriteRune('`') // closing backtick + } + case '\\': + started = true + // Unquoted backslash escapes the next character. + if i+1 < len(runes) { + i++ + b.WriteRune(runes[i]) + } else { + b.WriteRune('\\') + } + default: + started = true + b.WriteRune(c) + } + } + flush() + + return tokens +} diff --git a/src/java/javaexec/tokenize_test.go b/src/java/javaexec/tokenize_test.go new file mode 100644 index 0000000000..a46cc827b3 --- /dev/null +++ b/src/java/javaexec/tokenize_test.go @@ -0,0 +1,90 @@ +package javaexec + +import ( + "reflect" + "testing" +) + +func TestTokenizeJavaOpts(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + {"empty", "", nil}, + {"whitespace only", " \t\n ", nil}, + {"simple", "-Xmx1g", []string{"-Xmx1g"}}, + {"two simple", "-Xmx1g -Xms512m", []string{"-Xmx1g", "-Xms512m"}}, + {"leading -n", "-n -Dfoo=bar", []string{"-n", "-Dfoo=bar"}}, + + // Quoting: #1301 — quoted value with spaces stays a single argument. + {"double-quoted spaces", `-Dfoo="bar baz"`, []string{"-Dfoo=bar baz"}}, + {"single-quoted spaces", `-Dfoo='bar baz'`, []string{"-Dfoo=bar baz"}}, + {"two quoted args", `-Dfoo="bar baz" -Dother="qux quux"`, + []string{"-Dfoo=bar baz", "-Dother=qux quux"}}, + + // #1301 — cron with glob characters: one argument, '*' NOT expanded. + {"cron quoted", `-DcronSched="0 */7 * * * *"`, []string{"-DcronSched=0 */7 * * * *"}}, + + // Newlines (YAML block scalar) act as whitespace separators (issue #1259). + {"newline separated", "-Xmx1g\n-Xms512m", []string{"-Xmx1g", "-Xms512m"}}, + + // Shell metacharacters are treated as ordinary literal characters, + // NOT operators (the launcher is not a shell). This is safe by design. + {"ampersand literal", "-Dfoo=a&b", []string{"-Dfoo=a&b"}}, + {"pipe literal", "-Dx=a|b", []string{"-Dx=a|b"}}, + {"semicolon literal", "-Dx=a;b", []string{"-Dx=a;b"}}, + {"redirect literal", "-Dx=a>b", []string{"-Dx=a>b"}}, + + // Security: command substitution is NOT executed; passed through literally + // as a single token so the app does not crash with "class not found". + {"command substitution unquoted", "-Dx=$(echo HACK)", []string{"-Dx=$(echo HACK)"}}, + {"command substitution with pipe", "-Dx=$(hostname | tr a b)", []string{"-Dx=$(hostname | tr a b)"}}, + {"command substitution nested parens", "-Dx=$(foo (bar))", []string{"-Dx=$(foo (bar))"}}, + // Quoted, it stays a single literal argument and is never executed. + {"command substitution quoted", `-Dx="$(echo danger)"`, []string{"-Dx=$(echo danger)"}}, + {"backtick literal", "-Dx=`id`", []string{"-Dx=`id`"}}, + {"backtick with spaces", "-Dx=`hostname -f`", []string{"-Dx=`hostname -f`"}}, + + // ${...} extended bash forms with spaces — pass literally to JVM as one token. + {"brace default with spaces", "-Dx=${MY_VAR:-hello world}", []string{"-Dx=${MY_VAR:-hello world}"}}, + {"brace replacement with spaces", "-Dx=${MY_VAR//foo/bar baz}", []string{"-Dx=${MY_VAR//foo/bar baz}"}}, + {"brace simple no spaces", "-Dx=${MY_VAR}", []string{"-Dx=${MY_VAR}"}}, + {"brace nested braces in pattern", "-Dx=${MY_VAR//foo{bar}/baz qux}", []string{"-Dx=${MY_VAR//foo{bar}/baz qux}"}}, + {"brace assign default with spaces", "-Dx=${MY_VAR:=hello world}", []string{"-Dx=${MY_VAR:=hello world}"}}, + {"dollar var literal", "-Dx=$HOME", []string{"-Dx=$HOME"}}, + + // Full manifest scenario (issue #1301 reproducer with all edge cases): + // -Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=$HOME + // -Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me') + // -Dmyfile=c:\\first\\second\\file.txt;ext + // $HOME is already expanded by profile.d before javaexec sees it. + {"full manifest scenario", `-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=/home/vcap/app -Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me') -Dmyfile=c:\\first\\second\\file.txt;ext`, + []string{ + "-Dfoo=bar baz", + "-DcronSched=0 */7 * * * *", + "-Dbar=/home/vcap/app", + `-Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me')`, + `-Dmyfile=c:\first\second\file.txt;ext`, + }}, + + // Backslash follows POSIX shell rules (parity with previous eval form): + // unquoted backslash escapes the next character. + {"escaped space joins", `a\ b`, []string{"a b"}}, + {"double backslash", `-Dpath=C:\\double`, []string{`-Dpath=C:\double`}}, + {"single-quoted backslashes literal", `-Dpath='C:\tmp\app'`, []string{`-Dpath=C:\tmp\app`}}, + + // Empty quoted string yields an explicit empty argument. + {"empty double quotes", `-Dfoo=""`, []string{"-Dfoo="}}, + {"standalone empty quotes", `'' x`, []string{"", "x"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := TokenizeJavaOpts(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("TokenizeJavaOpts(%q) = %#v, want %#v", tc.in, got, tc.want) + } + }) + } +} diff --git a/src/java/jres/jvmkill.go b/src/java/jres/jvmkill.go index 277207bd4a..223e7f6901 100644 --- a/src/java/jres/jvmkill.go +++ b/src/java/jres/jvmkill.go @@ -176,20 +176,13 @@ func (j *JVMKillAgent) Finalize() error { return nil } -// convertToRuntimePath converts absolute staging path to runtime absolute path -// Example: /tmp/contents.../deps//jre/bin/jvmkill-1.16.0.so -> /home/vcap/deps//jre/bin/jvmkill-1.16.0.so -// Note: We use absolute path instead of $DEPS_DIR because startup scripts run before .profile.d scripts -// are sourced, so $DEPS_DIR is not yet available at runtime. +// convertToRuntimePath converts a staging path to a runtime path using $DEPS_DIR. +// The path ends up in a .opts file that is read by the profile.d assembly script, +// which expands $DEPS_DIR before passing the value to the JVM. func (j *JVMKillAgent) convertToRuntimePath(stagingPath string) string { - // Extract filename and build runtime path - // We know the structure: /deps//jre/bin/jvmkill-VERSION.so - // Runtime path: /home/vcap/deps//jre/bin/jvmkill-VERSION.so - depsIdx := j.ctx.Stager.DepsIdx() filename := filepath.Base(stagingPath) - - // Build absolute runtime path (Cloud Foundry standard location) - return fmt.Sprintf("/home/vcap/deps/%s/jre/bin/%s", depsIdx, filename) + return fmt.Sprintf("$DEPS_DIR/%s/jre/bin/%s", depsIdx, filename) } // getHeapDumpPath checks for volume service with heap-dump tag and returns path diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index 1d7199c527..2f3bf3452e 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -355,24 +355,18 @@ func (m *MemoryCalculator) GetCalculatorCommand() string { return fmt.Sprintf(`CALCULATED_MEMORY=$(%s) && echo JVM Memory Configuration: $CALCULATED_MEMORY && JAVA_OPTS="$JAVA_OPTS $CALCULATED_MEMORY" && MALLOC_ARENA_MAX=2`, calcCmd) } -// convertToRuntimePath converts a staging path to a runtime path -// Example: /tmp/staging/deps/0/jre/bin/calculator -> /home/vcap/deps/0/jre/bin/calculator +// convertToRuntimePath converts a staging path to a runtime path using $DEPS_DIR. +// Example: /tmp/staging/deps/0/jre/bin/calculator -> $DEPS_DIR/0/jre/bin/calculator func (m *MemoryCalculator) convertToRuntimePath(stagingPath string) string { depsIdx := m.ctx.Stager.DepsIdx() - // Extract the relative path from deps// onwards - // stagingPath: /tmp/.../deps//jre/bin/java-buildpack-memory-calculator-X.X.X - // We want: /home/vcap/deps//jre/bin/java-buildpack-memory-calculator-X.X.X - - // Find "jre/bin/" in the path if idx := strings.Index(stagingPath, "jre/bin/"); idx != -1 { relativePath := stagingPath[idx:] - return fmt.Sprintf("/home/vcap/deps/%s/%s", depsIdx, relativePath) + return fmt.Sprintf("$DEPS_DIR/%s/%s", depsIdx, relativePath) } - // Fallback: just use the filename filename := filepath.Base(stagingPath) - return fmt.Sprintf("/home/vcap/deps/%s/jre/bin/%s", depsIdx, filename) + return fmt.Sprintf("$DEPS_DIR/%s/jre/bin/%s", depsIdx, filename) } // openJDKJREConfig mirrors the memory_calculator section of JBP_CONFIG_OPEN_JDK_JRE.