From 2b0923f614c04aa16ece0c11ae4e70d5f850e00a Mon Sep 17 00:00:00 2001 From: Adler Fleurant <2609856+AdlerFleurant@users.noreply.github.com> Date: Tue, 20 Sep 2022 10:33:54 -0400 Subject: [PATCH] Distrust maven from system properties Check if maven settings file exists when getting from system properties. Null check on mavenSettings. Fix nearby typos. Fixes #28090 Signed-off-by: Adler Fleurant <2609856+AdlerFleurant@users.noreply.github.com> --- .../io/quarkus/cli/build/MavenRunner.java | 8 +-- .../test/java/io/quarkus/cli/CliDriver.java | 53 +++++++++---------- .../cli/RegistryClientBuilderTestBase.java | 12 ++--- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/devtools/cli/src/main/java/io/quarkus/cli/build/MavenRunner.java b/devtools/cli/src/main/java/io/quarkus/cli/build/MavenRunner.java index d8bcb2b29ab90..2c801a999901a 100644 --- a/devtools/cli/src/main/java/io/quarkus/cli/build/MavenRunner.java +++ b/devtools/cli/src/main/java/io/quarkus/cli/build/MavenRunner.java @@ -1,6 +1,7 @@ package io.quarkus.cli.build; import java.io.File; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayDeque; import java.util.ArrayList; @@ -32,6 +33,7 @@ import picocli.CommandLine; public class MavenRunner implements BuildSystemRunner { + public static String MAVEN_SETTINGS = "maven.settings"; static final String[] windowsWrapper = { "mvnw.cmd", "mvnw.bat" }; static final String otherWrapper = "mvnw"; @@ -238,13 +240,13 @@ void setMavenProperties(ArrayDeque args, boolean batchMode) { args.addFirst("-Dstyle.color=always"); } - String mavenSettings = propertiesOptions.properties.remove("maven.settings"); + String mavenSettings = propertiesOptions.properties.remove(MAVEN_SETTINGS); if (mavenSettings != null && !mavenSettings.isEmpty()) { args.add("-s"); args.add(mavenSettings); } else { - mavenSettings = System.getProperty("maven.settings"); - if (mavenSettings != null && !mavenSettings.isEmpty()) { + mavenSettings = System.getProperty(MAVEN_SETTINGS); + if (mavenSettings != null && !mavenSettings.isEmpty() && Files.exists(Path.of(mavenSettings))) { args.add("-s"); args.add(mavenSettings); } diff --git a/devtools/cli/src/test/java/io/quarkus/cli/CliDriver.java b/devtools/cli/src/test/java/io/quarkus/cli/CliDriver.java index ddf060402c37e..2ee05931bef2b 100644 --- a/devtools/cli/src/test/java/io/quarkus/cli/CliDriver.java +++ b/devtools/cli/src/test/java/io/quarkus/cli/CliDriver.java @@ -1,5 +1,8 @@ package io.quarkus.cli; +import static io.quarkus.cli.build.MavenRunner.MAVEN_SETTINGS; +import static org.apache.maven.cli.MavenCli.LOCAL_REPO_PROPERTY; + import java.io.ByteArrayOutputStream; import java.io.File; import java.io.PrintStream; @@ -12,6 +15,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.function.BinaryOperator; +import java.util.function.UnaryOperator; import org.junit.jupiter.api.Assertions; @@ -20,6 +26,9 @@ public class CliDriver { static final PrintStream stdout = System.out; static final PrintStream stderr = System.err; + private static final BinaryOperator ARG_FORMATTER = (key, value) -> "-D" + key + "=" + value; + private static final UnaryOperator REPO_ARG_FORMATTER = value -> ARG_FORMATTER.apply(LOCAL_REPO_PROPERTY, value); + private static final UnaryOperator SETTINGS_ARG_FORMATTER = value -> ARG_FORMATTER.apply(MAVEN_SETTINGS, value); public static class CliDriverBuilder { @@ -63,8 +72,10 @@ public Result execute() throws Exception { newArgs.subList(index, newArgs.size()).clear(); } - propagateProperty("maven.repo.local", mavenLocalRepo, newArgs); - propagateProperty("maven.settings", mavenSettings, newArgs); + Optional.ofNullable(mavenLocalRepo).or(CliDriver::getMavenLocalRepoProperty).map(REPO_ARG_FORMATTER) + .ifPresent(newArgs::add); + Optional.ofNullable(mavenSettings).or(CliDriver::getMavenSettingsProperty).map(SETTINGS_ARG_FORMATTER) + .ifPresent(newArgs::add); newArgs.add("--cli-test"); newArgs.add("--cli-test-dir"); @@ -81,7 +92,7 @@ public Result execute() throws Exception { PrintStream errPs = new PrintStream(err); System.setErr(errPs); - final Map originalProps = collectOverridenProps(newArgs); + final Map originalProps = collectOverriddenProps(newArgs); Result result = new Result(); QuarkusCli cli = new QuarkusCli(); @@ -109,7 +120,7 @@ protected void resetProperties(Map originalProps) { } } - protected Map collectOverridenProps(List newArgs) { + protected Map collectOverriddenProps(List newArgs) { final Map originalProps = new HashMap<>(); for (String s : newArgs) { if (s.startsWith("-D")) { @@ -121,22 +132,14 @@ protected Map collectOverridenProps(List newArgs) { originalProps.put(propName, origValue); } else if (System.getProperties().contains(propName)) { originalProps.put(propName, "true"); + } else { + originalProps.put(propName, null); } } } } return originalProps; } - - private static void propagateProperty(String propName, String testValue, List args) { - if (testValue == null) { - testValue = System.getProperty(propName); - if (testValue == null) { - return; - } - } - args.add("-D" + propName + "=" + testValue); - } } public static CliDriverBuilder builder() { @@ -144,14 +147,8 @@ public static CliDriverBuilder builder() { } public static void preserveLocalRepoSettings(Collection args) { - String s = convertToProperty("maven.repo.local"); - if (s != null) { - args.add(s); - } - s = convertToProperty("maven.settings"); - if (s != null) { - args.add(s); - } + getMavenLocalRepoProperty().map(REPO_ARG_FORMATTER).ifPresent(args::add); + getMavenSettingsProperty().map(SETTINGS_ARG_FORMATTER).ifPresent(args::add); } public static Result executeArbitraryCommand(Path startingDir, String... args) throws Exception { @@ -439,12 +436,12 @@ public static void validateApplicationProperties(Path projectRoot, List "Properties file should contain " + conf + ". Found:\n" + propertiesFile)); } - private static String convertToProperty(String name) { - String value = System.getProperty(name); - if (value != null) { - return "-D" + name + "=" + value; - } - return null; + private static Optional getMavenLocalRepoProperty() { + return Optional.ofNullable(System.getProperty(LOCAL_REPO_PROPERTY)); + } + + private static Optional getMavenSettingsProperty() { + return Optional.ofNullable(System.getProperty(MAVEN_SETTINGS)).filter(value -> Files.exists(Path.of(value))); } private static void retryDelete(File file) { diff --git a/devtools/cli/src/test/java/io/quarkus/cli/RegistryClientBuilderTestBase.java b/devtools/cli/src/test/java/io/quarkus/cli/RegistryClientBuilderTestBase.java index 00a402bcda5ca..d101dbe001899 100644 --- a/devtools/cli/src/test/java/io/quarkus/cli/RegistryClientBuilderTestBase.java +++ b/devtools/cli/src/test/java/io/quarkus/cli/RegistryClientBuilderTestBase.java @@ -1,7 +1,7 @@ package io.quarkus.cli; -import java.io.BufferedReader; import java.io.BufferedWriter; +import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.nio.file.Files; @@ -59,7 +59,7 @@ static void setup() throws Exception { final BootstrapMavenContext mavenContext = new BootstrapMavenContext( BootstrapMavenContext.config().setWorkspaceDiscovery(false)); - final Settings settings = getBaseMavenSettings(mavenContext.getUserSettings().toPath()); + final Settings settings = getBaseMavenSettings(mavenContext.getUserSettings()); Profile profile = new Profile(); settings.addActiveProfile("qs-test-registry"); @@ -106,11 +106,9 @@ protected static String getCurrentQuarkusVersion() { return v; } - private static Settings getBaseMavenSettings(Path mavenSettings) throws IOException { - if (Files.exists(mavenSettings)) { - try (BufferedReader reader = Files.newBufferedReader(mavenSettings)) { - return new DefaultSettingsReader().read(reader, Map.of()); - } + private static Settings getBaseMavenSettings(File mavenSettings) throws IOException { + if (mavenSettings != null && mavenSettings.exists()) { + return new DefaultSettingsReader().read(mavenSettings, Map.of()); } return new Settings(); }