From ef36df6b817ff510c46a7515b754d80b29fdd2c1 Mon Sep 17 00:00:00 2001
From: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
Date: Fri, 3 Jan 2025 23:04:28 +0000
Subject: [PATCH] Respect environment variables in jvm.options (#16834)

JvmOptionsParser adds support for ${VAR:default} syntax when parsing jvm.options
- allow dynamic resolution of environment variables in the jvm.options file
- enables fallback to default value when the environment variable is not set
---
 config/jvm.options                            |  2 +-
 .../logstash/launchers/JvmOptionsParser.java  | 26 ++++++-
 .../launchers/JvmOptionsParserTest.java       | 74 +++++++++++++++++++
 3 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/config/jvm.options b/config/jvm.options
index f84e904ce..18ccd2f83 100644
--- a/config/jvm.options
+++ b/config/jvm.options
@@ -30,7 +30,7 @@
 ## basic
 
 # set the I/O temp directory
-#-Djava.io.tmpdir=$HOME
+#-Djava.io.tmpdir=${HOME}
 
 # set to headless, just in case
 -Djava.awt.headless=true
diff --git a/tools/jvm-options-parser/src/main/java/org/logstash/launchers/JvmOptionsParser.java b/tools/jvm-options-parser/src/main/java/org/logstash/launchers/JvmOptionsParser.java
index d19b5f547..24edfec73 100644
--- a/tools/jvm-options-parser/src/main/java/org/logstash/launchers/JvmOptionsParser.java
+++ b/tools/jvm-options-parser/src/main/java/org/logstash/launchers/JvmOptionsParser.java
@@ -32,7 +32,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
@@ -283,7 +282,6 @@ public class JvmOptionsParser {
     }
 
     private static final Pattern OPTION_DEFINITION = Pattern.compile("((?<start>\\d+)(?<range>-)?(?<end>\\d+)?:)?(?<option>-.*)$");
-
     /**
      *
      * If the version syntax specified on a line matches the specified JVM options, the JVM option callback will be invoked with the JVM
@@ -372,13 +370,16 @@ public class JvmOptionsParser {
             // Skip comments and blank lines
             return Optional.empty();
         }
-        final Matcher matcher = OPTION_DEFINITION.matcher(line);
+
+        String subbedLine = resolveEnvVar(line, System.getenv());
+
+        final Matcher matcher = OPTION_DEFINITION.matcher(subbedLine);
         if (matcher.matches()) {
             final String start = matcher.group("start");
             final String end = matcher.group("end");
             if (start == null) {
                 // no range present, unconditionally apply the JVM option
-                return Optional.of(line);
+                return Optional.of(subbedLine);
             } else {
                 final int lower = Integer.parseInt(start);
                 final int upper;
@@ -404,6 +405,23 @@ public class JvmOptionsParser {
         return Optional.empty();
     }
 
+    private static final Pattern ENV_VAR_PATTERN = Pattern.compile("\\$\\{([a-zA-Z_.][a-zA-Z0-9_.]*)(?::([^}]*))?\\}");
+
+    static String resolveEnvVar(String line, Map<String,String> env) {
+        Matcher matcher = ENV_VAR_PATTERN.matcher(line);
+        StringBuilder sb = new StringBuilder();
+
+        while (matcher.find()) {
+            String varName = matcher.group(1);
+            String defaultValue = Optional.ofNullable(matcher.group(2)).orElse("");
+            String replacement = env.getOrDefault(varName, defaultValue);
+            matcher.appendReplacement(sb,  Matcher.quoteReplacement(replacement));
+        }
+        matcher.appendTail(sb);
+
+        return sb.toString();
+    }
+
     private static final Pattern JAVA_VERSION = Pattern.compile("^(?:1\\.)?(?<javaMajorVersion>\\d+)(?:\\.\\d+)?$");
 
     private static int javaMajorVersion() {
diff --git a/tools/jvm-options-parser/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java b/tools/jvm-options-parser/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java
index 86964abc5..49e28df7f 100644
--- a/tools/jvm-options-parser/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java
+++ b/tools/jvm-options-parser/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java
@@ -198,4 +198,78 @@ public class JvmOptionsParserTest {
         return new BufferedReader(new StringReader(s));
     }
 
+
+    @Test
+    public void testSingleEnvSub() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${LOGSTASH_HOME}/heapdump.hprof",
+                Map.of("LOGSTASH_HOME", "/path/to/ls_home"));
+        assertEquals("-XX:HeapDumpPath=/path/to/ls_home/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testMultipleEnvSub() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${LOGSTASH_HOME}/${DATA}/heapdump.hprof",
+                Map.of("LOGSTASH_HOME", "/path/to/ls_home", "DATA", "data"));
+        assertEquals("-XX:HeapDumpPath=/path/to/ls_home/data/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testPeriodEnvSub() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${.HOME}/heapdump.hprof",
+                Map.of(".HOME", "/path/to/.home"));
+        assertEquals("-XX:HeapDumpPath=/path/to/.home/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testEmptyEnvSub() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${NOT_VALID}/heapdump.hprof", Map.of());
+        assertEquals("-XX:HeapDumpPath=/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testNoSub() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("   ", Map.of());
+        assertEquals("   ", result);
+    }
+
+    @Test
+    public void testEnvSubWithDefault() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${LOGSTASH_HOME:/usr/share/logstash}/${DATA:data}/heapdump.hprof",
+                Map.of());
+        assertEquals("-XX:HeapDumpPath=/usr/share/logstash/data/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testEnvSubWithDefaultSpecialChar() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${LOGSTASH_HOME:/usr/share/logstash}/${DATA:{$crazy!enough?'bless'@[you]}/heapdump.hprof",
+                Map.of());
+        assertEquals("-XX:HeapDumpPath=/usr/share/logstash/{$crazy!enough?'bless'@[you]/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testEnvSubWithDefaultOverwritten() throws IOException {
+        String result = JvmOptionsParser.resolveEnvVar("-XX:HeapDumpPath=${LOGSTASH_HOME:/usr/share/logstash}/${DATA:data}/heapdump.hprof",
+                Map.of("DATA", "data2"));
+        assertEquals("-XX:HeapDumpPath=/usr/share/logstash/data2/heapdump.hprof", result);
+    }
+
+    @Test
+    public void testEnvSubInFile() throws IOException {
+        File optionsFile = writeIntoTempOptionsFile(
+                writer -> writer.println("-Xlog:gc*,gc+age=trace,safepoint:file=${UNKNOWN}:"));
+
+        JvmOptionsParser.handleJvmOptions(new String[] {"/path/to/ls_home", optionsFile.toString()}, "-Dcli.opts=something");
+
+        final String output = outputStreamCaptor.toString();
+        assertTrue("env variable should be substituted ", output.contains("-Xlog:gc*,gc+age=trace,safepoint:file=:"));
+    }
+
+    @Test
+    public void testCommentedEnvSub() throws IOException {
+        final BufferedReader options = asReader("# -Xlog:gc*,gc+age=trace,safepoint:file=${UNKNOWN}:");
+        final JvmOptionsParser.ParseResult res = JvmOptionsParser.parse(11, options);
+
+        assertTrue("no invalid lines can be present", res.getInvalidLines().isEmpty());
+        assertFalse(String.join(System.lineSeparator(), res.getJvmOptions()).contains("-Xlog:gc*,gc+age=trace,safepoint"));
+    }
 }
\ No newline at end of file