Skip to content

Commit bbbd895

Browse files
ramonskieramonskie
authored
Merge pull request #1260 from stokpop/issue-1259-java-opts-breaks-pre-start-script
Issue 1259 java opts breaks pre start script
2 parents 307fb47 + 4d73403 commit bbbd895

2 files changed

Lines changed: 111 additions & 13 deletions

File tree

‎src/java/frameworks/java_opts_writer.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ func CreateJavaOptsAssemblyScript(ctx *common.Context) error {
7373
# Expands runtime variables like $DEPS_DIR, $HOME, $JAVA_OPTS, and all other environment variables
7474
7575
# Save original JAVA_OPTS from environment (user-provided)
76-
USER_JAVA_OPTS="$JAVA_OPTS"
76+
# Normalize to single line: YAML block scalars (>) may introduce newlines
77+
# xargs trims leading/trailing whitespace and collapses internal spaces
78+
USER_JAVA_OPTS=$(echo "$JAVA_OPTS" | tr '\n' ' ' | tr -s ' ' | xargs)
7779
7880
# Start building new JAVA_OPTS
7981
JAVA_OPTS=""
@@ -84,19 +86,17 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then
8486
# Read content and expand runtime variables
8587
opts_content=$(cat "$opts_file")
8688
87-
# First, expand special variables that need specific handling
88-
# Expand $DEPS_DIR variable
89-
opts_content=$(echo "$opts_content" | sed "s|\$DEPS_DIR|$DEPS_DIR|g")
89+
# Expand $DEPS_DIR, $HOME, $JAVA_OPTS using bash parameter expansion.
90+
# sed-based substitution breaks when these values contain the sed delimiter (|),
91+
# backslashes, ampersands, or newlines — all valid in JAVA_OPTS and paths.
92+
opts_content="${opts_content//\$DEPS_DIR/$DEPS_DIR}"
93+
opts_content="${opts_content//\$HOME/$HOME}"
94+
opts_content="${opts_content//\$JAVA_OPTS/$USER_JAVA_OPTS}"
9095
91-
# Expand $HOME variable (for app-provided JARs like AspectJ)
92-
opts_content=$(echo "$opts_content" | sed "s|\$HOME|$HOME|g")
93-
94-
# Expand $JAVA_OPTS to the saved USER_JAVA_OPTS value (not the loop's current JAVA_OPTS)
95-
opts_content=$(echo "$opts_content" | sed "s|\$JAVA_OPTS|$USER_JAVA_OPTS|g")
96-
97-
# Now expand all remaining environment variables using eval with proper escaping
98-
# This mimics Ruby buildpack behavior where shell naturally expands variables
99-
# Use eval in a subshell to safely expand variables without executing commands
96+
# Expand any remaining environment variables in opts content via eval.
97+
# Note: eval executes commands, but .opts files are written by the buildpack
98+
# at staging time and run within the container context.
99+
# This matches how the Ruby buildpack naturally expanded variables via shell.
100100
opts_content=$(eval "echo \"$opts_content\"")
101101
102102
if [ -n "$opts_content" ]; then

‎src/java/frameworks/java_opts_writer_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,55 @@ package frameworks_test
22

33
import (
44
"os"
5+
"os/exec"
6+
"path/filepath"
57

68
. "github.com/onsi/ginkgo/v2"
79
. "github.com/onsi/gomega"
10+
11+
"github.com/cloudfoundry/java-buildpack/src/java/common"
12+
"github.com/cloudfoundry/java-buildpack/src/java/frameworks"
13+
"github.com/cloudfoundry/libbuildpack"
814
)
915

16+
func newJavaOptsContext(buildDir, cacheDir, depsDir string) *common.Context {
17+
logger := libbuildpack.NewLogger(GinkgoWriter)
18+
manifest := &libbuildpack.Manifest{}
19+
installer := &libbuildpack.Installer{}
20+
stager := libbuildpack.NewStager([]string{buildDir, cacheDir, depsDir, "0"}, logger, manifest)
21+
return &common.Context{
22+
Stager: stager,
23+
Manifest: manifest,
24+
Installer: installer,
25+
Log: logger,
26+
Command: &libbuildpack.Command{},
27+
}
28+
}
29+
1030
var _ = Describe("Java Opts Writer", func() {
31+
var (
32+
buildDir string
33+
cacheDir string
34+
depsDir string
35+
ctx *common.Context
36+
)
37+
38+
BeforeEach(func() {
39+
var err error
40+
buildDir, err = os.MkdirTemp("", "build")
41+
Expect(err).NotTo(HaveOccurred())
42+
cacheDir, err = os.MkdirTemp("", "cache")
43+
Expect(err).NotTo(HaveOccurred())
44+
depsDir, err = os.MkdirTemp("", "deps")
45+
Expect(err).NotTo(HaveOccurred())
46+
ctx = newJavaOptsContext(buildDir, cacheDir, depsDir)
47+
})
48+
1149
AfterEach(func() {
1250
os.Unsetenv("JAVA_OPTS")
51+
os.RemoveAll(buildDir)
52+
os.RemoveAll(cacheDir)
53+
os.RemoveAll(depsDir)
1354
})
1455

1556
Describe("Basic options", func() {
@@ -20,4 +61,61 @@ var _ = Describe("Java Opts Writer", func() {
2061
Expect(os.Getenv("JAVA_OPTS")).To(Equal(javaOpts))
2162
})
2263
})
64+
65+
Describe("CreateJavaOptsAssemblyScript", func() {
66+
runScript := func(javaOpts string, optsFileContent string) (string, error) {
67+
err := frameworks.CreateJavaOptsAssemblyScript(ctx)
68+
Expect(err).NotTo(HaveOccurred())
69+
70+
optsDir := filepath.Join(depsDir, "0", "java_opts")
71+
Expect(os.MkdirAll(optsDir, 0755)).To(Succeed())
72+
Expect(os.WriteFile(filepath.Join(optsDir, "42_agent.opts"), []byte(optsFileContent), 0644)).To(Succeed())
73+
74+
scriptPath := filepath.Join(depsDir, "0", "profile.d", "00_java_opts.sh")
75+
cmd := exec.Command("bash", "-c", "source "+scriptPath+" && echo \"$JAVA_OPTS\"")
76+
cmd.Env = append(os.Environ(),
77+
"JAVA_OPTS="+javaOpts,
78+
"DEPS_DIR="+depsDir,
79+
"HOME=/home/vcap/app",
80+
)
81+
output, err := cmd.CombinedOutput()
82+
return string(output), err
83+
}
84+
85+
It("handles multiline JAVA_OPTS from YAML block scalar without sed error", func() {
86+
// Reproduce the manifest pattern:
87+
// JAVA_OPTS: >
88+
// -javaagent:$HOME/BOOT-INF/lib/agent.jar
89+
// -XX:+UseZGC
90+
// YAML '>' folds newlines to spaces, but CF may deliver them as literal newlines
91+
multilineJavaOpts := "-javaagent:$HOME/BOOT-INF/lib/agent.jar\n-XX:+UseZGC\n-XX:+AlwaysPreTouch"
92+
93+
output, err := runScript(multilineJavaOpts, "-javaagent:somepath.jar $JAVA_OPTS")
94+
Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
95+
Expect(output).To(ContainSubstring("-XX:+UseZGC"))
96+
})
97+
98+
It("handles pipe character in JAVA_OPTS (e.g. javaagent options) without sed error", func() {
99+
// Reproduce the manifest pattern:
100+
// JAVA_OPTS: >
101+
// -javaagent:$HOME/BOOT-INF/lib/jfr-exporter.jar=enableExecutorMBeans|disableMyFeature
102+
pipeJavaOpts := "-javaagent:$HOME/BOOT-INF/lib/jfr-exporter.jar=enableExecutorMBeans|disableMyFeature"
103+
104+
output, err := runScript(pipeJavaOpts, "-javaagent:somepath.jar $JAVA_OPTS")
105+
Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
106+
Expect(output).To(ContainSubstring("enableExecutorMBeans|disableMyFeature"))
107+
})
108+
109+
It("expands $HOME in opts file content", func() {
110+
output, err := runScript("", "-javaagent:$HOME/BOOT-INF/lib/agent.jar")
111+
Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
112+
Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar"))
113+
})
114+
115+
It("expands $DEPS_DIR in opts file content", func() {
116+
output, err := runScript("", "-Djava.security.properties=$DEPS_DIR/0/security.properties")
117+
Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output)
118+
Expect(output).To(ContainSubstring("-Djava.security.properties=" + depsDir + "/0/security.properties"))
119+
})
120+
})
23121
})

0 commit comments

Comments
 (0)