diff --git a/plugin/src/functionalTest/kotlin/com/ryandens/javaagent/JavaagentPluginFunctionalTest.kt b/plugin/src/functionalTest/kotlin/com/ryandens/javaagent/JavaagentPluginFunctionalTest.kt index ef8adb94..d0bc56ce 100644 --- a/plugin/src/functionalTest/kotlin/com/ryandens/javaagent/JavaagentPluginFunctionalTest.kt +++ b/plugin/src/functionalTest/kotlin/com/ryandens/javaagent/JavaagentPluginFunctionalTest.kt @@ -163,8 +163,10 @@ class JavaagentPluginFunctionalTest { val expectedWindowsDefaultJvmOpts = """set DEFAULT_JVM_OPTS="-javaagent:%APP_HOME%\agent-libs\simple-agent.jar" "-Xmx256m"""" assertTrue(applicationDistributionScript.readText().contains(expectedWindowsDefaultJvmOpts)) } else { + // Each option is its own backslash-escaped quoted token inside the outer-quoted value, so the + // launcher never word-splits an option (including ones whose value has a space) -- see issue #95. val expectedDefaultJavaOpts = """ -DEFAULT_JVM_OPTS="-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar -Xmx256m" +DEFAULT_JVM_OPTS="\"-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar\" \"-Xmx256m\"" """ assertTrue(applicationDistributionScript.readText().contains(expectedDefaultJavaOpts)) } @@ -186,6 +188,29 @@ DEFAULT_JVM_OPTS="-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar -Xmx256m assertTrue(ccResult.output.contains("Reusing configuration cache.")) } + @Test fun `preserves applicationDefaultJvmArgs containing spaces in application distribution`() { + // Regression test for https://github.com/ryandens/javaagent-gradle-plugin/issues/95: applying the + // plugin must not drop or mangle the JVM args a consumer configures via `applicationDefaultJvmArgs`. + // A value containing a space (`-Dgreeting=hello world`) is the sharp edge: the generated start script + // must keep each option as its own quoted token so the shell does not word-split "hello world" into + // two arguments. HelloWorld echoes the `greeting` system property back so we can assert end-to-end + // that the argument actually reaches the JVM intact. + val dependencies = """ + javaagent project(':simple-agent') + """ + + createJavaagentProject(dependencies, applicationDefaultJvmArgs = "['-Dgreeting=hello world', '-Xmx256m']") + val result = runBuild(listOf("build", "installDist", "execStartScript")) + + // the agent is still attached + assertTrue(result.output.contains("Hello from my simple agent!")) + // and the space-containing default JVM arg survived intact rather than being word-split + assertTrue( + result.output.contains("greeting=[hello world]"), + "expected the space-containing applicationDefaultJvmArg to reach the JVM as a single argument, but the run output was:\n${result.output}", + ) + } + @Test fun `can handle upgrade of agent with build cache`() { createJavaagentProject( """ @@ -224,7 +249,9 @@ DEFAULT_JVM_OPTS="-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar -Xmx256m if (isWindows) { assertTrue(applicationDistributionScript.readText().contains("""set DEFAULT_JVM_OPTS="-Xmx256m"""")) } else { - assertTrue(applicationDistributionScript.readText().contains("""DEFAULT_JVM_OPTS="-Xmx256m""")) + // With no agents, the placeholder token is stripped but the remaining option keeps its own + // backslash-escaped quotes inside the outer-quoted value: DEFAULT_JVM_OPTS="\"-Xmx256m\"" + assertTrue(applicationDistributionScript.readText().contains("DEFAULT_JVM_OPTS=\"\\\"-Xmx256m\\\"\"")) } assertFalse( @@ -236,7 +263,10 @@ DEFAULT_JVM_OPTS="-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar -Xmx256m assertTrue(result.output.contains("Hello World!")) } - private fun createJavaagentProject(dependencies: String) { + private fun createJavaagentProject( + dependencies: String, + applicationDefaultJvmArgs: String = "['-Xmx256m']", + ) { val helloWorldDir = File(functionalTestDir, "hello-world") Paths.get("src", "functionalTest", "resources", "hello-world-project").toFile().copyRecursively(helloWorldDir) val simpleAgentTestDir = File(functionalTestDir, "simple-agent") @@ -288,7 +318,7 @@ DEFAULT_JVM_OPTS="-javaagent:${"$"}APP_HOME/agent-libs/simple-agent.jar -Xmx256m application { mainClass = 'com.ryandens.HelloWorld' - applicationDefaultJvmArgs = ['-Xmx256m'] + applicationDefaultJvmArgs = $applicationDefaultJvmArgs } run { diff --git a/plugin/src/functionalTest/resources/hello-world-project/src/main/java/com/ryandens/HelloWorld.java b/plugin/src/functionalTest/resources/hello-world-project/src/main/java/com/ryandens/HelloWorld.java index e1a04c65..b37e9d0a 100644 --- a/plugin/src/functionalTest/resources/hello-world-project/src/main/java/com/ryandens/HelloWorld.java +++ b/plugin/src/functionalTest/resources/hello-world-project/src/main/java/com/ryandens/HelloWorld.java @@ -5,5 +5,11 @@ public final class HelloWorld { public static void main(final String[] args) { System.out.println("Hello World!"); + // Echo a system property so functional tests can verify that JVM args configured via + // `applicationDefaultJvmArgs` are actually applied at runtime and not dropped/mangled by the plugin. + final String greeting = System.getProperty("greeting"); + if (greeting != null) { + System.out.println("greeting=[" + greeting + "]"); + } } -} \ No newline at end of file +} diff --git a/plugin/src/main/kotlin/com/ryandens/javaagent/JavaagentAwareStartScriptGenerator.kt b/plugin/src/main/kotlin/com/ryandens/javaagent/JavaagentAwareStartScriptGenerator.kt index 5f517d7c..8a51619a 100644 --- a/plugin/src/main/kotlin/com/ryandens/javaagent/JavaagentAwareStartScriptGenerator.kt +++ b/plugin/src/main/kotlin/com/ryandens/javaagent/JavaagentAwareStartScriptGenerator.kt @@ -82,12 +82,16 @@ class JavaagentAwareStartScriptGenerator( val files = javaagentFiles.get() val replace = if (files.isEmpty()) { - // Remove the placeholder opt when there are no agents. On Windows each opt is its own quoted token - // (`"-javaagent:...PLACEHOLDER.jar" "-Xmx256m"`), so the surrounding quotes and trailing space must - // be stripped too; on Unix the opts are collapsed into a single quoted string - // (`"-javaagent:...PLACEHOLDER.jar -Xmx256m"`), so only the unquoted placeholder and its trailing - // space are removed. The trailing-space variants run first to avoid leaving a dangling separator. + // Remove the placeholder opt when there are no agents. Each opt is its own quoted token: on Unix + // the quotes are backslash-escaped inside the outer-quoted value + // (`"\"-javaagent:...PLACEHOLDER.jar\" \"-Xmx256m\""`), on Windows they are literal + // (`"-javaagent:...PLACEHOLDER.jar" "-Xmx256m"`). Strip the placeholder token together with its + // surrounding quotes and trailing separator. The trailing-space variants run first to avoid + // leaving a dangling separator; the escaped variants run before the bare ones so the leading `\` + // is not orphaned. str + .replace("\\\"-javaagent:COM_RYANDENS_JAVAAGENTS_PLACEHOLDER.jar\\\" ", "") + .replace("\\\"-javaagent:COM_RYANDENS_JAVAAGENTS_PLACEHOLDER.jar\\\"", "") .replace("\"-javaagent:COM_RYANDENS_JAVAAGENTS_PLACEHOLDER.jar\" ", "") .replace("\"-javaagent:COM_RYANDENS_JAVAAGENTS_PLACEHOLDER.jar\"", "") .replace("-javaagent:COM_RYANDENS_JAVAAGENTS_PLACEHOLDER.jar ", "") diff --git a/plugin/src/main/kotlin/com/ryandens/javaagent/Platform.kt b/plugin/src/main/kotlin/com/ryandens/javaagent/Platform.kt index 7d69dac0..81289960 100644 --- a/plugin/src/main/kotlin/com/ryandens/javaagent/Platform.kt +++ b/plugin/src/main/kotlin/com/ryandens/javaagent/Platform.kt @@ -12,16 +12,22 @@ enum class Platform( val templateBindingFactory: StartScriptTemplateBindingFactory, val template: TextResource, /** - * Separator placed between multiple `-javaagent` options. On Windows each option must remain its own - * quoted token (`" "` = close quote, space, open quote); on Unix the options share a single quoted string, - * so a plain space suffices. + * Separator placed between multiple `-javaagent` options. On both platforms each option must remain its + * own quoted token so the launcher never word-splits a path (or any other option) on internal spaces. On + * Windows the tokens are literally double-quoted (`" "` = close quote, space, open quote); on Unix the + * whole `DEFAULT_JVM_OPTS` value is itself wrapped in double quotes so `$APP_HOME` expands, so the inner + * per-option quotes are backslash-escaped (`\" \"` = escaped close quote, space, escaped open quote). */ val agentArgSeparator: String, /** - * Normalizes the template's rendered `defaultJvmOpts`. On Unix the individually quoted options are - * collapsed into a single space-separated string that the shell word-splits back apart when the unquoted - * `$DEFAULT_JVM_OPTS` is expanded; on Windows the batch script does not word-split inside quotes, so each - * option must stay its own quoted token and the value is left unchanged. + * Normalizes the template's rendered `defaultJvmOpts`. On Windows the batch script does not word-split + * inside quotes, so each option stays its own literally-quoted token and the value is left unchanged. On + * Unix the value is wrapped in an outer pair of double quotes -- required so the embedded `$APP_HOME` in a + * `-javaagent:` path is expanded when the assignment runs -- and each option's own double quotes are + * backslash-escaped so they survive into the value. That keeps every option a distinct quoted token that + * the launcher's `xargs` pass parses back out intact, including options whose value contains spaces (e.g. + * `-Dgreeting=hello world`). Collapsing the per-option quotes into a single quoted string would let the + * shell word-split those values apart -- see issue #95. */ val defaultJvmOptsMapper: (String) -> String, ) { @@ -31,8 +37,8 @@ enum class Platform( "\$APP_HOME", StartScriptTemplateBindingFactory.unix(), UnixStartScriptGenerator().template, - " ", - { it.replace("\" \"", " ") }, + "\\\" \\\"", + { "\"" + it.replace("\"", "\\\"") + "\"" }, ), WINDOWS( "\r\n",