Merge pull request #1330 from stokpop/fix/java-opts-backslash-bash-compat

fix(java_opts): preserve backslashes in JAVA_OPTS on bash 4.x

Author: stokpop

scripts/unit.sh

@@ -14,6 +14,8 @@ function main() {
   local src
   src="$(find "${ROOTDIR}/src" -mindepth 1 -maxdepth 1 -type d )"
 
+  echo "bash: ${BASH_VERSION}"
+
   util::tools::ginkgo::install --directory "${ROOTDIR}/.bin"
   util::tools::buildpack-packager::install --directory "${ROOTDIR}/.bin"
 

src/java/frameworks/java_opts_writer.go

@@ -141,13 +141,13 @@ case "$USER_JAVA_OPTS" in
 esac
 
 # Escape replacement-special chars once; these values are loop-invariant.
+# USER_JAVA_OPTS is injected via string-split below (not ${//}) — bash 4.x and 5.x
+# treat '\\' in replacement strings differently, corrupting backslashes.
 _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
@@ -194,8 +194,21 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then
                     ;;
             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}"
+            # Restore USER_JAVA_OPTS via string-split, not ${//}: bash 4.x and 5.x
+            # treat '\\' in replacement strings differently, corrupting backslashes.
+            # %%%% / # are pure string ops — no special-char interpretation, any bash.
+            # Pre-count occurrences so the loop is bounded even if USER_JAVA_OPTS
+            # itself contained the placeholder string (infinite-loop defence).
+            # Counting uses empty replacement — no backslash in replacement, safe.
+            # Handles multiple occurrences (e.g. user config "$JAVA_OPTS -Xmx512m"
+            # with from_environment:true produces "$JAVA_OPTS -Xmx512m $JAVA_OPTS").
+            _stripped="${opts_content//"$_user_java_opts_placeholder"/}"
+            _placeholder_count=$(( (${#opts_content} - ${#_stripped}) / ${#_user_java_opts_placeholder} ))
+            for (( _i=0; _i<_placeholder_count; _i++ )); do
+                _before="${opts_content%%%%"$_user_java_opts_placeholder"*}"
+                _after="${opts_content#*"$_user_java_opts_placeholder"}"
+                opts_content="${_before}${USER_JAVA_OPTS}${_after}"
+            done
             
             if [ -n "$opts_content" ]; then
                 JAVA_OPTS="$JAVA_OPTS $opts_content"

src/java/frameworks/java_opts_writer_test.go

@@ -370,6 +370,15 @@ var _ = Describe("Java Opts Writer", func() {
 			Expect(output).To(ContainSubstring(`-Dpath=C:\\double`))
 		})
 
+		It("replaces all occurrences of $JAVA_OPTS when opts file contains it twice", func() {
+			// Scenario: JBP_CONFIG_JAVA_OPTS: '{java_opts: "$JAVA_OPTS -Xmx512m", from_environment: true}'
+			// produces opts file: "$JAVA_OPTS -Xmx512m $JAVA_OPTS" — two placeholders
+			output, err := runScript(`-Dfoo=bar`, "$JAVA_OPTS -Xmx512m $JAVA_OPTS")
+			Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
+			Expect(strings.Count(output, "-Dfoo=bar")).To(Equal(2))
+			Expect(output).NotTo(ContainSubstring("__JAVA_OPTS_BUILDPACK_PLACEHOLDER__"))
+		})
+
 		// 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.