From 507f868fe9bf4cfcb620a8128ed87ef7c82f3067 Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Fri, 12 Sep 2025 10:06:07 +0200 Subject: [PATCH] Remove quoting for system properties Before this commit, system properties with a value has been quoted all the time. This is fine as long if the resulting string is passed into the RunArguments(String) constructor, which then parses it again into a String[]. This isn't fine if they are added to ProcessBuilder.command, because then quoting is unnecessary. This commit also fixes the unnecessary Map -> String -> String[] parsing by adding the entries directly into the array. Closes gh-46555 --- .../springframework/boot/maven/AotTests.java | 10 ++++ .../projects/aot-system-properties/pom.xml | 47 +++++++++++++++++++ .../main/java/org/test/SampleApplication.java | 31 ++++++++++++ .../org/test/TestProfileConfiguration.java | 32 +++++++++++++ .../boot/maven/AbstractRunMojo.java | 36 +++++--------- .../boot/maven/CommandLineBuilder.java | 30 ++++-------- .../boot/maven/RunArguments.java | 13 +++-- .../boot/maven/SystemPropertyFormatter.java | 40 ++++++++++++++++ .../boot/maven/CommandLineBuilderTests.java | 2 +- .../maven/SystemPropertyFormatterTests.java | 8 ++-- 10 files changed, 195 insertions(+), 54 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/pom.xml create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/SampleApplication.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/TestProfileConfiguration.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/SystemPropertyFormatter.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AotTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AotTests.java index e9539a1d763..49bb6ec7bca 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AotTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AotTests.java @@ -37,6 +37,7 @@ import static org.assertj.core.api.Assertions.contentOf; * @author Stephane Nicoll * @author Andy Wilkinson * @author Scott Frederick + * @author Moritz Halbritter */ @ExtendWith(MavenBuildExtension.class) class AotTests { @@ -108,6 +109,15 @@ class AotTests { }); } + @TestTemplate + void whenAotRunsWithSystemPropertiesSourcesAreGenerated(MavenBuild mavenBuild) { + mavenBuild.project("aot-system-properties").goals("package").execute((project) -> { + Path aotDirectory = project.toPath().resolve("target/spring-aot/main"); + assertThat(collectRelativePaths(aotDirectory.resolve("sources"))) + .contains(Path.of("org", "test", "TestProfileConfiguration__BeanDefinitions.java")); + }); + } + @TestTemplate void whenAotRunsWithJvmArgumentsSourcesAreGenerated(MavenBuild mavenBuild) { mavenBuild.project("aot-jvm-arguments").goals("package").execute((project) -> { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/pom.xml b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/pom.xml new file mode 100644 index 00000000000..4c88725bece --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/pom.xml @@ -0,0 +1,47 @@ + + + 4.0.0 + org.springframework.boot.maven.it + aot-system-properties + 0.0.1.BUILD-SNAPSHOT + + UTF-8 + @java.version@ + @java.version@ + + + + + @project.groupId@ + @project.artifactId@ + @project.version@ + + + + process-aot + + + + abc + + + + + + + + + + org.springframework.boot + spring-boot + @project.version@ + + + jakarta.servlet + jakarta.servlet-api + @jakarta-servlet.version@ + provided + + + diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/SampleApplication.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/SampleApplication.java new file mode 100644 index 00000000000..e2f2bc36ab6 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/SampleApplication.java @@ -0,0 +1,31 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.test; + +import org.springframework.boot.SpringApplication; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +@Configuration(proxyBeanMethods = false) +@Import(TestProfileConfiguration.class) +public class SampleApplication { + + public static void main(String[] args) { + SpringApplication.run(SampleApplication.class, args); + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/TestProfileConfiguration.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/TestProfileConfiguration.java new file mode 100644 index 00000000000..96922433096 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/TestProfileConfiguration.java @@ -0,0 +1,32 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.test; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Profile; + +@Configuration(proxyBeanMethods = false) +@Profile("abc") +class TestProfileConfiguration { + + @Bean + public String abc() { + return "abc"; + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractRunMojo.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractRunMojo.java index e528301ebac..8cfd502709d 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractRunMojo.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractRunMojo.java @@ -25,6 +25,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.stream.Collectors; @@ -39,6 +40,7 @@ import org.apache.maven.project.MavenProject; import org.apache.maven.toolchain.ToolchainManager; import org.springframework.boot.loader.tools.FileUtils; +import org.springframework.util.StringUtils; /** * Base class to run a Spring Boot application. @@ -300,17 +302,20 @@ public abstract class AbstractRunMojo extends AbstractDependencyFilterMojo { * @return a {@link RunArguments} defining the JVM arguments */ protected RunArguments resolveJvmArguments() { - StringBuilder stringBuilder = new StringBuilder(); + List arguments = new ArrayList<>(); if (this.systemPropertyVariables != null) { - stringBuilder.append(this.systemPropertyVariables.entrySet() - .stream() - .map((e) -> SystemPropertyFormatter.format(e.getKey(), e.getValue())) - .collect(Collectors.joining(" "))); + for (Entry systemProperty : this.systemPropertyVariables.entrySet()) { + String argument = SystemPropertyFormatter.format(systemProperty.getKey(), systemProperty.getValue()); + if (StringUtils.hasText(argument)) { + arguments.add(argument); + } + } } if (this.jvmArguments != null) { - stringBuilder.append(" ").append(this.jvmArguments); + String[] jvmArguments = RunArguments.parseArgs(this.jvmArguments); + arguments.addAll(Arrays.asList(jvmArguments)); } - return new RunArguments(stringBuilder.toString()); + return new RunArguments(arguments); } private void addJvmArgs(List args) { @@ -417,21 +422,4 @@ public abstract class AbstractRunMojo extends AbstractDependencyFilterMojo { } } - /** - * Format System properties. - */ - static class SystemPropertyFormatter { - - static String format(String key, String value) { - if (key == null) { - return ""; - } - if (value == null || value.isEmpty()) { - return String.format("-D%s", key); - } - return String.format("-D%s=\"%s\"", key, value); - } - - } - } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/CommandLineBuilder.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/CommandLineBuilder.java index 8afb7287cd4..2e48c4f8891 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/CommandLineBuilder.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/CommandLineBuilder.java @@ -21,8 +21,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; +import org.springframework.util.StringUtils; + /** * Helper class to build the command-line arguments of a java process. * @@ -55,10 +58,12 @@ final class CommandLineBuilder { CommandLineBuilder withSystemProperties(Map systemProperties) { if (systemProperties != null) { - systemProperties.entrySet() - .stream() - .map((e) -> SystemPropertyFormatter.format(e.getKey(), e.getValue())) - .forEach(this.options::add); + for (Entry systemProperty : systemProperties.entrySet()) { + String option = SystemPropertyFormatter.format(systemProperty.getKey(), systemProperty.getValue()); + if (StringUtils.hasText(option)) { + this.options.add(option); + } + } } return this; } @@ -88,21 +93,4 @@ final class CommandLineBuilder { return commandLine; } - /** - * Format System properties. - */ - private static final class SystemPropertyFormatter { - - static String format(String key, String value) { - if (key == null) { - return ""; - } - if (value == null || value.isEmpty()) { - return String.format("-D%s", key); - } - return String.format("-D%s=\"%s\"", key, value); - } - - } - } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RunArguments.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RunArguments.java index 5c256dcaf27..a6a0712700a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RunArguments.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RunArguments.java @@ -19,7 +19,6 @@ package org.springframework.boot.maven; import java.util.Arrays; import java.util.Deque; import java.util.LinkedList; -import java.util.Objects; import org.codehaus.plexus.util.cli.CommandLineUtils; @@ -39,8 +38,16 @@ class RunArguments { } RunArguments(String[] args) { + this((args != null) ? Arrays.asList(args) : null); + } + + RunArguments(Iterable args) { if (args != null) { - Arrays.stream(args).filter(Objects::nonNull).forEach(this.args::add); + for (String arg : args) { + if (arg != null) { + this.args.add(arg); + } + } } } @@ -52,7 +59,7 @@ class RunArguments { return this.args.toArray(new String[0]); } - private static String[] parseArgs(String arguments) { + static String[] parseArgs(String arguments) { if (arguments == null || arguments.trim().isEmpty()) { return NO_ARGS; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/SystemPropertyFormatter.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/SystemPropertyFormatter.java new file mode 100644 index 00000000000..11e1136651f --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/SystemPropertyFormatter.java @@ -0,0 +1,40 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.maven; + +/** + * Format System properties. + * + * @author Phillip Webb + * @author Stephane Nicoll + */ +final class SystemPropertyFormatter { + + private SystemPropertyFormatter() { + } + + static String format(String key, String value) { + if (key == null) { + return ""; + } + if (value == null || value.isEmpty()) { + return String.format("-D%s", key); + } + return String.format("-D%s=%s", key, value); + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/CommandLineBuilderTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/CommandLineBuilderTests.java index 4e11b213738..7eccf586b37 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/CommandLineBuilderTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/CommandLineBuilderTests.java @@ -76,7 +76,7 @@ class CommandLineBuilderTests { @Test void buildWithSystemProperty() { assertThat(CommandLineBuilder.forMainClass(CLASS_NAME).withSystemProperties(Map.of("flag", "enabled")).build()) - .containsExactly("-Dflag=\"enabled\"", CLASS_NAME); + .containsExactly("-Dflag=enabled", CLASS_NAME); } @Test diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/SystemPropertyFormatterTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/SystemPropertyFormatterTests.java index 0825d129b2c..d10f5923c3d 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/SystemPropertyFormatterTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/SystemPropertyFormatterTests.java @@ -18,12 +18,10 @@ package org.springframework.boot.maven; import org.junit.jupiter.api.Test; -import org.springframework.boot.maven.AbstractRunMojo.SystemPropertyFormatter; - import static org.assertj.core.api.Assertions.assertThat; /** - * Tests for {@link AbstractRunMojo.SystemPropertyFormatter}. + * Tests for {@link SystemPropertyFormatter}. */ class SystemPropertyFormatterTests { @@ -39,7 +37,7 @@ class SystemPropertyFormatterTests { @Test void parseKeyWithValue() { - assertThat(SystemPropertyFormatter.format("key1", "value1")).isEqualTo("-Dkey1=\"value1\""); + assertThat(SystemPropertyFormatter.format("key1", "value1")).isEqualTo("-Dkey1=value1"); } @Test @@ -49,7 +47,7 @@ class SystemPropertyFormatterTests { @Test void parseKeyWithOnlySpaces() { - assertThat(SystemPropertyFormatter.format("key1", " ")).isEqualTo("-Dkey1=\" \""); + assertThat(SystemPropertyFormatter.format("key1", " ")).isEqualTo("-Dkey1= "); } }