Skip to content

Commit 507f868

Browse files
mhalbrittermhalbritter
committed
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
1 parent b5405d2 commit 507f868

File tree

10 files changed

+195
-54
lines changed

10 files changed

+195
-54
lines changed

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AotTests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
* @author Stephane Nicoll
3838
* @author Andy Wilkinson
3939
* @author Scott Frederick
40+
* @author Moritz Halbritter
4041
*/
4142
@ExtendWith(MavenBuildExtension.class)
4243
class AotTests {
@@ -108,6 +109,15 @@ void whenAotRunsWithArgumentsSourcesAreGenerated(MavenBuild mavenBuild) {
108109
});
109110
}
110111

112+
@TestTemplate
113+
void whenAotRunsWithSystemPropertiesSourcesAreGenerated(MavenBuild mavenBuild) {
114+
mavenBuild.project("aot-system-properties").goals("package").execute((project) -> {
115+
Path aotDirectory = project.toPath().resolve("target/spring-aot/main");
116+
assertThat(collectRelativePaths(aotDirectory.resolve("sources")))
117+
.contains(Path.of("org", "test", "TestProfileConfiguration__BeanDefinitions.java"));
118+
});
119+
}
120+
111121
@TestTemplate
112122
void whenAotRunsWithJvmArgumentsSourcesAreGenerated(MavenBuild mavenBuild) {
113123
mavenBuild.project("aot-jvm-arguments").goals("package").execute((project) -> {

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/pom.xml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
4+
<modelVersion>4.0.0</modelVersion>
5+
<groupId>org.springframework.boot.maven.it</groupId>
6+
<artifactId>aot-system-properties</artifactId>
7+
<version>0.0.1.BUILD-SNAPSHOT</version>
8+
<properties>
9+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
10+
<maven.compiler.source>@java.version@</maven.compiler.source>
11+
<maven.compiler.target>@java.version@</maven.compiler.target>
12+
</properties>
13+
<build>
14+
<plugins>
15+
<plugin>
16+
<groupId>@project.groupId@</groupId>
17+
<artifactId>@project.artifactId@</artifactId>
18+
<version>@project.version@</version>
19+
<executions>
20+
<execution>
21+
<goals>
22+
<goal>process-aot</goal>
23+
</goals>
24+
<configuration>
25+
<systemPropertyVariables>
26+
<spring.profiles.active>abc</spring.profiles.active>
27+
</systemPropertyVariables>
28+
</configuration>
29+
</execution>
30+
</executions>
31+
</plugin>
32+
</plugins>
33+
</build>
34+
<dependencies>
35+
<dependency>
36+
<groupId>org.springframework.boot</groupId>
37+
<artifactId>spring-boot</artifactId>
38+
<version>@project.version@</version>
39+
</dependency>
40+
<dependency>
41+
<groupId>jakarta.servlet</groupId>
42+
<artifactId>jakarta.servlet-api</artifactId>
43+
<version>@jakarta-servlet.version@</version>
44+
<scope>provided</scope>
45+
</dependency>
46+
</dependencies>
47+
</project>

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/SampleApplication.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.test;
18+
19+
import org.springframework.boot.SpringApplication;
20+
import org.springframework.context.annotation.Configuration;
21+
import org.springframework.context.annotation.Import;
22+
23+
@Configuration(proxyBeanMethods = false)
24+
@Import(TestProfileConfiguration.class)
25+
public class SampleApplication {
26+
27+
public static void main(String[] args) {
28+
SpringApplication.run(SampleApplication.class, args);
29+
}
30+
31+
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/aot-system-properties/src/main/java/org/test/TestProfileConfiguration.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.test;
18+
19+
import org.springframework.context.annotation.Bean;
20+
import org.springframework.context.annotation.Configuration;
21+
import org.springframework.context.annotation.Profile;
22+
23+
@Configuration(proxyBeanMethods = false)
24+
@Profile("abc")
25+
class TestProfileConfiguration {
26+
27+
@Bean
28+
public String abc() {
29+
return "abc";
30+
}
31+
32+
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractRunMojo.java‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Collections;
2626
import java.util.List;
2727
import java.util.Map;
28+
import java.util.Map.Entry;
2829
import java.util.Set;
2930
import java.util.stream.Collectors;
3031

@@ -39,6 +40,7 @@
3940
import org.apache.maven.toolchain.ToolchainManager;
4041

4142
import org.springframework.boot.loader.tools.FileUtils;
43+
import org.springframework.util.StringUtils;
4244

4345
/**
4446
* Base class to run a Spring Boot application.
@@ -300,17 +302,20 @@ private Map<String, String> determineEnvironmentVariables() {
300302
* @return a {@link RunArguments} defining the JVM arguments
301303
*/
302304
protected RunArguments resolveJvmArguments() {
303-
StringBuilder stringBuilder = new StringBuilder();
305+
List<String> arguments = new ArrayList<>();
304306
if (this.systemPropertyVariables != null) {
305-
stringBuilder.append(this.systemPropertyVariables.entrySet()
306-
.stream()
307-
.map((e) -> SystemPropertyFormatter.format(e.getKey(), e.getValue()))
308-
.collect(Collectors.joining(" ")));
307+
for (Entry<String, String> systemProperty : this.systemPropertyVariables.entrySet()) {
308+
String argument = SystemPropertyFormatter.format(systemProperty.getKey(), systemProperty.getValue());
309+
if (StringUtils.hasText(argument)) {
310+
arguments.add(argument);
311+
}
312+
}
309313
}
310314
if (this.jvmArguments != null) {
311-
stringBuilder.append(" ").append(this.jvmArguments);
315+
String[] jvmArguments = RunArguments.parseArgs(this.jvmArguments);
316+
arguments.addAll(Arrays.asList(jvmArguments));
312317
}
313-
return new RunArguments(stringBuilder.toString());
318+
return new RunArguments(arguments);
314319
}
315320

316321
private void addJvmArgs(List<String> args) {
@@ -417,21 +422,4 @@ private void logArguments(String name, String[] args) {
417422
}
418423
}
419424

420-
/**
421-
* Format System properties.
422-
*/
423-
static class SystemPropertyFormatter {
424-
425-
static String format(String key, String value) {
426-
if (key == null) {
427-
return "";
428-
}
429-
if (value == null || value.isEmpty()) {
430-
return String.format("-D%s", key);
431-
}
432-
return String.format("-D%s=\"%s\"", key, value);
433-
}
434-
435-
}
436-
437425
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/CommandLineBuilder.java‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@
2121
import java.util.Arrays;
2222
import java.util.List;
2323
import java.util.Map;
24+
import java.util.Map.Entry;
2425
import java.util.Objects;
2526

27+
import org.springframework.util.StringUtils;
28+
2629
/**
2730
* Helper class to build the command-line arguments of a java process.
2831
*
@@ -55,10 +58,12 @@ CommandLineBuilder withJvmArguments(String... jvmArguments) {
5558

5659
CommandLineBuilder withSystemProperties(Map<String, String> systemProperties) {
5760
if (systemProperties != null) {
58-
systemProperties.entrySet()
59-
.stream()
60-
.map((e) -> SystemPropertyFormatter.format(e.getKey(), e.getValue()))
61-
.forEach(this.options::add);
61+
for (Entry<String, String> systemProperty : systemProperties.entrySet()) {
62+
String option = SystemPropertyFormatter.format(systemProperty.getKey(), systemProperty.getValue());
63+
if (StringUtils.hasText(option)) {
64+
this.options.add(option);
65+
}
66+
}
6267
}
6368
return this;
6469
}
@@ -88,21 +93,4 @@ List<String> build() {
8893
return commandLine;
8994
}
9095

91-
/**
92-
* Format System properties.
93-
*/
94-
private static final class SystemPropertyFormatter {
95-
96-
static String format(String key, String value) {
97-
if (key == null) {
98-
return "";
99-
}
100-
if (value == null || value.isEmpty()) {
101-
return String.format("-D%s", key);
102-
}
103-
return String.format("-D%s=\"%s\"", key, value);
104-
}
105-
106-
}
107-
10896
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RunArguments.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.util.Arrays;
2020
import java.util.Deque;
2121
import java.util.LinkedList;
22-
import java.util.Objects;
2322

2423
import org.codehaus.plexus.util.cli.CommandLineUtils;
2524

@@ -39,8 +38,16 @@ class RunArguments {
3938
}
4039

4140
RunArguments(String[] args) {
41+
this((args != null) ? Arrays.asList(args) : null);
42+
}
43+
44+
RunArguments(Iterable<String> args) {
4245
if (args != null) {
43-
Arrays.stream(args).filter(Objects::nonNull).forEach(this.args::add);
46+
for (String arg : args) {
47+
if (arg != null) {
48+
this.args.add(arg);
49+
}
50+
}
4451
}
4552
}
4653

@@ -52,7 +59,7 @@ String[] asArray() {
5259
return this.args.toArray(new String[0]);
5360
}
5461

55-
private static String[] parseArgs(String arguments) {
62+
static String[] parseArgs(String arguments) {
5663
if (arguments == null || arguments.trim().isEmpty()) {
5764
return NO_ARGS;
5865
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/SystemPropertyFormatter.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.maven;
18+
19+
/**
20+
* Format System properties.
21+
*
22+
* @author Phillip Webb
23+
* @author Stephane Nicoll
24+
*/
25+
final class SystemPropertyFormatter {
26+
27+
private SystemPropertyFormatter() {
28+
}
29+
30+
static String format(String key, String value) {
31+
if (key == null) {
32+
return "";
33+
}
34+
if (value == null || value.isEmpty()) {
35+
return String.format("-D%s", key);
36+
}
37+
return String.format("-D%s=%s", key, value);
38+
}
39+
40+
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/CommandLineBuilderTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ void buildWithNullSystemPropertyIsIgnored() {
7676
@Test
7777
void buildWithSystemProperty() {
7878
assertThat(CommandLineBuilder.forMainClass(CLASS_NAME).withSystemProperties(Map.of("flag", "enabled")).build())
79-
.containsExactly("-Dflag=\"enabled\"", CLASS_NAME);
79+
.containsExactly("-Dflag=enabled", CLASS_NAME);
8080
}
8181

8282
@Test

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/SystemPropertyFormatterTests.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@
1818

1919
import org.junit.jupiter.api.Test;
2020

21-
import org.springframework.boot.maven.AbstractRunMojo.SystemPropertyFormatter;
22-
2321
import static org.assertj.core.api.Assertions.assertThat;
2422

2523
/**
26-
* Tests for {@link AbstractRunMojo.SystemPropertyFormatter}.
24+
* Tests for {@link SystemPropertyFormatter}.
2725
*/
2826
class SystemPropertyFormatterTests {
2927

@@ -39,7 +37,7 @@ void parseOnlyKey() {
3937

4038
@Test
4139
void parseKeyWithValue() {
42-
assertThat(SystemPropertyFormatter.format("key1", "value1")).isEqualTo("-Dkey1=\"value1\"");
40+
assertThat(SystemPropertyFormatter.format("key1", "value1")).isEqualTo("-Dkey1=value1");
4341
}
4442

4543
@Test
@@ -49,7 +47,7 @@ void parseKeyWithEmptyValue() {
4947

5048
@Test
5149
void parseKeyWithOnlySpaces() {
52-
assertThat(SystemPropertyFormatter.format("key1", " ")).isEqualTo("-Dkey1=\" \"");
50+
assertThat(SystemPropertyFormatter.format("key1", " ")).isEqualTo("-Dkey1= ");
5351
}
5452

5553
}

0 commit comments

Comments
 (0)