Skip to content

Commit 53ebe43

Browse files
committed
fix(java_opts): handle multiple \$JAVA_OPTS in one opts file; fix %% in fmt.Sprintf
Two bugs fixed: 1. The if-guard (single replacement) became a bounded for-loop so opts files with $JAVA_OPTS more than once are handled correctly. Scenario: a user sets JBP_CONFIG_JAVA_OPTS: '{java_opts: "$JAVA_OPTS -Xmx512m", from_environment: true}' which produces "$JAVA_OPTS -Xmx512m $JAVA_OPTS" in the opts file — two placeholders. 2. The _before cut used %% in the Go fmt.Sprintf template, which emits a single % in the generated bash (shortest-suffix removal). With two placeholders, % cuts from the wrong side, leaving placeholders in place. Changed to %%%% in Go source → %% in bash (longest-suffix removal) so each for-loop iteration always removes from the first occurrence. Test added (TDD): "replaces all occurrences of $JAVA_OPTS when opts file contains it twice" — was red before both fixes, green after. Also: scripts/unit.sh: use ${BASH_VERSION} builtin instead of bash --version pipe (Copilot review comment 2).
1 parent 86684bc commit 53ebe43

3 files changed

Lines changed: 21 additions & 7 deletions

File tree

scripts/unit.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function main() {
1414
local src
1515
src="$(find "${ROOTDIR}/src" -mindepth 1 -maxdepth 1 -type d )"
1616

17-
echo "bash: $(bash --version | head -1)"
17+
echo "bash: ${BASH_VERSION}"
1818

1919
util::tools::ginkgo::install --directory "${ROOTDIR}/.bin"
2020
util::tools::buildpack-packager::install --directory "${ROOTDIR}/.bin"

src/java/frameworks/java_opts_writer.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,19 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then
196196
197197
# Restore USER_JAVA_OPTS via string-split, not ${//}: bash 4.x and 5.x
198198
# treat '\\' in replacement strings differently, corrupting backslashes.
199-
# %% / # are pure string ops — no special-char interpretation, any bash.
200-
# Guard required: without placeholder present, %% and # return the full
201-
# string, so omitting the guard duplicates opts_content.
202-
if [[ "$opts_content" == *"$_user_java_opts_placeholder"* ]]; then
203-
_before="${opts_content%%"$_user_java_opts_placeholder"*}"
199+
# %%%% / # are pure string ops — no special-char interpretation, any bash.
200+
# Pre-count occurrences so the loop is bounded even if USER_JAVA_OPTS
201+
# itself contained the placeholder string (infinite-loop defence).
202+
# Counting uses empty replacement — no backslash in replacement, safe.
203+
# Handles multiple occurrences (e.g. user config "$JAVA_OPTS -Xmx512m"
204+
# with from_environment:true produces "$JAVA_OPTS -Xmx512m $JAVA_OPTS").
205+
_stripped="${opts_content//"$_user_java_opts_placeholder"/}"
206+
_placeholder_count=$(( (${#opts_content} - ${#_stripped}) / ${#_user_java_opts_placeholder} ))
207+
for (( _i=0; _i<_placeholder_count; _i++ )); do
208+
_before="${opts_content%%%%"$_user_java_opts_placeholder"*}"
204209
_after="${opts_content#*"$_user_java_opts_placeholder"}"
205210
opts_content="${_before}${USER_JAVA_OPTS}${_after}"
206-
fi
211+
done
207212
208213
if [ -n "$opts_content" ]; then
209214
JAVA_OPTS="$JAVA_OPTS $opts_content"

src/java/frameworks/java_opts_writer_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,15 @@ var _ = Describe("Java Opts Writer", func() {
370370
Expect(output).To(ContainSubstring(`-Dpath=C:\\double`))
371371
})
372372

373+
It("replaces all occurrences of $JAVA_OPTS when opts file contains it twice", func() {
374+
// Scenario: JBP_CONFIG_JAVA_OPTS: '{java_opts: "$JAVA_OPTS -Xmx512m", from_environment: true}'
375+
// produces opts file: "$JAVA_OPTS -Xmx512m $JAVA_OPTS" — two placeholders
376+
output, err := runScript(`-Dfoo=bar`, "$JAVA_OPTS -Xmx512m $JAVA_OPTS")
377+
Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
378+
Expect(strings.Count(output, "-Dfoo=bar")).To(Equal(2))
379+
Expect(output).NotTo(ContainSubstring("__JAVA_OPTS_BUILDPACK_PLACEHOLDER__"))
380+
})
381+
373382
// Full invocation cycle tests for issue #1301:
374383
// Verify that javaexec tokenizes $JAVA_OPTS without a shell, so glob chars,
375384
// pipes, and shell metacharacters are never expanded or executed.

0 commit comments

Comments
 (0)