Skip to content

Commit

Permalink
Distrust maven from system properties
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@AdlerFleurant
AdlerFleurant committed Sep 23, 2022
1 parent 1b152c7 commit cba2132
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 38 deletions.
8 changes: 5 additions & 3 deletions devtools/cli/src/main/java/io/quarkus/cli/build/MavenRunner.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -238,13 +240,13 @@ void setMavenProperties(ArrayDeque<String> 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);
}
Expand Down
53 changes: 25 additions & 28 deletions devtools/cli/src/test/java/io/quarkus/cli/CliDriver.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand All @@ -20,6 +26,9 @@
public class CliDriver {
static final PrintStream stdout = System.out;
static final PrintStream stderr = System.err;
private static final BinaryOperator<String> ARG_FORMATTER = (key, value) -> "-D" + key + "=" + value;
private static final UnaryOperator<String> REPO_ARG_FORMATTER = value -> ARG_FORMATTER.apply(LOCAL_REPO_PROPERTY, value);
private static final UnaryOperator<String> SETTINGS_ARG_FORMATTER = value -> ARG_FORMATTER.apply(MAVEN_SETTINGS, value);

public static class CliDriverBuilder {

Expand Down Expand Up @@ -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");
Expand All @@ -81,7 +92,7 @@ public Result execute() throws Exception {
PrintStream errPs = new PrintStream(err);
System.setErr(errPs);

final Map<String, String> originalProps = collectOverridenProps(newArgs);
final Map<String, String> originalProps = collectOverriddenProps(newArgs);

Result result = new Result();
QuarkusCli cli = new QuarkusCli();
Expand Down Expand Up @@ -109,7 +120,7 @@ protected void resetProperties(Map<String, String> originalProps) {
}
}

protected Map<String, String> collectOverridenProps(List<String> newArgs) {
protected Map<String, String> collectOverriddenProps(List<String> newArgs) {
final Map<String, String> originalProps = new HashMap<>();
for (String s : newArgs) {
if (s.startsWith("-D")) {
Expand All @@ -121,37 +132,23 @@ protected Map<String, String> collectOverridenProps(List<String> 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<String> args) {
if (testValue == null) {
testValue = System.getProperty(propName);
if (testValue == null) {
return;
}
}
args.add("-D" + propName + "=" + testValue);
}
}

public static CliDriverBuilder builder() {
return new CliDriverBuilder();
}

public static void preserveLocalRepoSettings(Collection<String> 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 {
Expand Down Expand Up @@ -439,12 +436,12 @@ public static void validateApplicationProperties(Path projectRoot, List<String>
"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<String> getMavenLocalRepoProperty() {
return Optional.ofNullable(System.getProperty(LOCAL_REPO_PROPERTY));
}

private static Optional<String> getMavenSettingsProperty() {
return Optional.ofNullable(System.getProperty(MAVEN_SETTINGS)).filter(value -> Files.exists(Path.of(value)));
}

private static void retryDelete(File file) {
Expand Down
12 changes: 5 additions & 7 deletions devtools/cli/src/test/java/io/quarkus/cli/RegistryClientBuilderTestBase.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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();
}
Expand Down

0 comments on commit cba2132

Please sign in to comment.