Issue 1259 java opts breaks pre start script#1260
Open
stokpop wants to merge 3 commits intocloudfoundry:mainfrom
Open
Issue 1259 java opts breaks pre start script#1260stokpop wants to merge 3 commits intocloudfoundry:mainfrom
stokpop wants to merge 3 commits intocloudfoundry:mainfrom
Conversation
JAVA_OPTS set via YAML block scalar (>) in manifest.yml may contain literal newlines when delivered to the shell. The profile.d script used sed to substitute $JAVA_OPTS into .opts file content, which fails with "unterminated 's' command" when the value spans multiple lines. Fix: normalize USER_JAVA_OPTS to a single line at capture time using tr to convert newlines to spaces and collapse multiple spaces. Added test reproducing the exact failure with multiline JAVA_OPTS containing -javaagent and -Xms/-Xmx/-XX flags.
…script
Using sed to substitute $DEPS_DIR, $HOME and $JAVA_OPTS in opts file
content breaks when those values contain the sed delimiter (|),
backslashes, ampersands, or newlines. All are valid in JAVA_OPTS,
e.g. javaagent options using pipe syntax:
-javaagent:agent.jar=enableExecutorMBeans|disableMyFeature
Bash parameter expansion (${var//find/replace}) has no special-character
restrictions and replaces sed for these three substitutions.
Adds regression test covering pipe character in JAVA_OPTS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1259
The
profile.d/00_java_opts.shscript usedsedto substitute$JAVA_OPTS,$HOMEand$DEPS_DIRinto.optsfile content. This breaks at runtime whenJAVA_OPTScontains:>) in CF manifest|) — e.g. javaagent options likeagent.jar=featureA|featureB&or\— interpreted as sed replacement metacharacters, causing silent corruptionWhen
sedfails, the script continues withoutset -e, silently dropping all opts file content. The JVM starts with an emptyJAVA_OPTS: no memory limits, no agents.Changes
JAVA_OPTSto single line at capture time (trnewlines to spaces) before any substitutionsedwith bash parameter expansion (${var//find/replace}) for$JAVA_OPTS,$HOMEand$DEPS_DIRsubstitutions — no special-character restrictionsJAVA_OPTS, pipe characters, and$HOME/$DEPS_DIRexpansionWhy
set -ewas not addedprofile.dscripts are sourced by the CF launcher, not run in a subshell —set -ewould leak to the parent shell. Akill -TERM $$guard was also considered, but after analysis it provides little value: withsedreplaced by bash parameter expansion, there are no remaining commands in the assembly loop that can fail (the substitutions are pure bash builtins). A guard checking for emptyJAVA_OPTSwould also be imprecise, since the memory calculator appends toJAVA_OPTSvia its own separatejava.shscript after00_java_opts.shruns.