diff --git a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/ExecutorHelper.java b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/ExecutorHelper.java index 14a5e2abd2d4..c6dc27feb0ae 100644 --- a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/ExecutorHelper.java +++ b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/ExecutorHelper.java @@ -26,7 +26,7 @@ /** * Helper class for routing Maven execution based on preferences and/or issued execution requests. */ -public interface ExecutorHelper extends ExecutorTool { +public interface ExecutorHelper { /** * The modes of execution. */ diff --git a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/HelperImpl.java b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/HelperImpl.java index 4304f6d87195..9a94ddc2ddfd 100644 --- a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/HelperImpl.java +++ b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/HelperImpl.java @@ -21,7 +21,6 @@ import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.api.annotations.Nullable; @@ -29,7 +28,6 @@ import org.apache.maven.api.cli.ExecutorException; import org.apache.maven.api.cli.ExecutorRequest; import org.apache.maven.cling.executor.ExecutorHelper; -import org.apache.maven.cling.executor.ExecutorTool; import static java.util.Objects.requireNonNull; @@ -40,7 +38,6 @@ public class HelperImpl implements ExecutorHelper { private final Mode defaultMode; private final Path installationDirectory; private final Path userHomeDirectory; - private final ExecutorTool executorTool; private final HashMap executors; private final ConcurrentHashMap cache; @@ -58,7 +55,6 @@ public HelperImpl( this.userHomeDirectory = userHomeDirectory != null ? ExecutorRequest.getCanonicalPath(userHomeDirectory) : ExecutorRequest.discoverUserHomeDirectory(); - this.executorTool = new ToolboxTool(this); this.executors = new HashMap<>(); this.executors.put(Mode.EMBEDDED, requireNonNull(embedded, "embedded")); @@ -89,28 +85,6 @@ public String mavenVersion() { }); } - @Override - public Map dump(ExecutorRequest.Builder request) throws ExecutorException { - return executorTool.dump(request); - } - - @Override - public String localRepository(ExecutorRequest.Builder request) throws ExecutorException { - return executorTool.localRepository(request); - } - - @Override - public String artifactPath(ExecutorRequest.Builder request, String gav, String repositoryId) - throws ExecutorException { - return executorTool.artifactPath(request, gav, repositoryId); - } - - @Override - public String metadataPath(ExecutorRequest.Builder request, String gav, String repositoryId) - throws ExecutorException { - return executorTool.metadataPath(request, gav, repositoryId); - } - protected Executor getExecutor(Mode mode, ExecutorRequest request) throws ExecutorException { return switch (mode) { case AUTO -> getExecutorByRequest(request); diff --git a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java index b23a4948a3ea..2c08093ce6d7 100644 --- a/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java +++ b/impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java @@ -59,7 +59,8 @@ public Map dump(ExecutorRequest.Builder executorRequest) throws doExecute(builder); try { Properties properties = new Properties(); - properties.load(new ByteArrayInputStream(stdout.toByteArray())); + properties.load(new ByteArrayInputStream( + validateOutput(false, stdout, stderr).getBytes())); return properties.entrySet().stream() .collect(Collectors.toMap( e -> String.valueOf(e.getKey()), @@ -79,7 +80,7 @@ public String localRepository(ExecutorRequest.Builder executorRequest) throws Ex .stdOut(stdout) .stdErr(stderr); doExecute(builder); - return shaveStdout(stdout); + return validateOutput(true, stdout, stderr); } @Override @@ -95,7 +96,7 @@ public String artifactPath(ExecutorRequest.Builder executorRequest, String gav, builder.argument("-Drepository=" + repositoryId + "::unimportant"); } doExecute(builder); - return shaveStdout(stdout); + return validateOutput(true, stdout, stderr); } @Override @@ -111,7 +112,7 @@ public String metadataPath(ExecutorRequest.Builder executorRequest, String gav, builder.argument("-Drepository=" + repositoryId + "::unimportant"); } doExecute(builder); - return shaveStdout(stdout); + return validateOutput(true, stdout, stderr); } private ExecutorRequest.Builder mojo(ExecutorRequest.Builder builder, String mojo) { @@ -131,7 +132,19 @@ private void doExecute(ExecutorRequest.Builder builder) { } } - private String shaveStdout(ByteArrayOutputStream stdout) { - return stdout.toString().replace("\n", "").replace("\r", ""); + /** + * Performs "sanity check" for output, making sure no insane values like empty strings are returned. + */ + private String validateOutput(boolean shave, ByteArrayOutputStream stdout, ByteArrayOutputStream stderr) { + String result = stdout.toString(); + if (shave) { + result = result.replace("\n", "").replace("\r", ""); + } + // sanity checks: stderr has any OR result is empty string (no method should emit empty string) + if (stderr.size() > 0 || result.trim().isEmpty()) { + throw new ExecutorException( + "Unexpected stdout[" + stdout.size() + "]=" + stdout + "; stderr[" + stderr.size() + "]=" + stderr); + } + return result; } } diff --git a/impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/HelperImplTest.java b/impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/ToolboxToolTest.java similarity index 88% rename from impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/HelperImplTest.java rename to impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/ToolboxToolTest.java index ee4c70bd204d..d34fc00f0801 100644 --- a/impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/HelperImplTest.java +++ b/impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/ToolboxToolTest.java @@ -27,6 +27,7 @@ import org.apache.maven.cling.executor.ExecutorHelper; import org.apache.maven.cling.executor.MavenExecutorTestSupport; import org.apache.maven.cling.executor.internal.HelperImpl; +import org.apache.maven.cling.executor.internal.ToolboxTool; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; @@ -38,7 +39,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -public class HelperImplTest { +public class ToolboxToolTest { @TempDir private static Path userHome; @@ -52,7 +53,7 @@ void dump3(ExecutorHelper.Mode mode) throws Exception { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - Map dump = helper.dump(helper.executorRequest()); + Map dump = new ToolboxTool(helper).dump(helper.executorRequest()); assertEquals(System.getProperty("maven3version"), dump.get("maven.version")); } @@ -66,7 +67,7 @@ void dump4(ExecutorHelper.Mode mode) throws Exception { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - Map dump = helper.dump(helper.executorRequest()); + Map dump = new ToolboxTool(helper).dump(helper.executorRequest()); assertEquals(System.getProperty("maven4version"), dump.get("maven.version")); } @@ -106,7 +107,7 @@ void localRepository3(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String localRepository = helper.localRepository(helper.executorRequest()); + String localRepository = new ToolboxTool(helper).localRepository(helper.executorRequest()); Path local = Paths.get(localRepository); assertTrue(Files.isDirectory(local)); } @@ -122,7 +123,7 @@ void localRepository4(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String localRepository = helper.localRepository(helper.executorRequest()); + String localRepository = new ToolboxTool(helper).localRepository(helper.executorRequest()); Path local = Paths.get(localRepository); assertTrue(Files.isDirectory(local)); } @@ -137,7 +138,8 @@ void artifactPath3(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String path = helper.artifactPath(helper.executorRequest(), "aopalliance:aopalliance:1.0", "central"); + String path = new ToolboxTool(helper) + .artifactPath(helper.executorRequest(), "aopalliance:aopalliance:1.0", "central"); // split repository: assert "ends with" as split may introduce prefixes assertTrue( path.endsWith("aopalliance" + File.separator + "aopalliance" + File.separator + "1.0" + File.separator @@ -155,7 +157,8 @@ void artifactPath4(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String path = helper.artifactPath(helper.executorRequest(), "aopalliance:aopalliance:1.0", "central"); + String path = new ToolboxTool(helper) + .artifactPath(helper.executorRequest(), "aopalliance:aopalliance:1.0", "central"); // split repository: assert "ends with" as split may introduce prefixes assertTrue( path.endsWith("aopalliance" + File.separator + "aopalliance" + File.separator + "1.0" + File.separator @@ -173,7 +176,7 @@ void metadataPath3(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String path = helper.metadataPath(helper.executorRequest(), "aopalliance", "someremote"); + String path = new ToolboxTool(helper).metadataPath(helper.executorRequest(), "aopalliance", "someremote"); // split repository: assert "ends with" as split may introduce prefixes assertTrue(path.endsWith("aopalliance" + File.separator + "maven-metadata-someremote.xml"), "path=" + path); } @@ -188,7 +191,7 @@ void metadataPath4(ExecutorHelper.Mode mode) { userHome, MavenExecutorTestSupport.EMBEDDED_MAVEN_EXECUTOR, MavenExecutorTestSupport.FORKED_MAVEN_EXECUTOR); - String path = helper.metadataPath(helper.executorRequest(), "aopalliance", "someremote"); + String path = new ToolboxTool(helper).metadataPath(helper.executorRequest(), "aopalliance", "someremote"); // split repository: assert "ends with" as split may introduce prefixes assertTrue(path.endsWith("aopalliance" + File.separator + "maven-metadata-someremote.xml"), "path=" + path); } diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3477DependencyResolutionErrorMessageTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3477DependencyResolutionErrorMessageTest.java index c61f02303afb..5ec5fa8c0f42 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3477DependencyResolutionErrorMessageTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3477DependencyResolutionErrorMessageTest.java @@ -96,7 +96,7 @@ void connectionProblemsPlugin() throws Exception { new String[] { ".*The following artifacts could not be resolved: org.apache.maven.its.plugins:maven-it-plugin-not-exists:pom:1.2.3 \\(absent\\): " + "Could not transfer artifact org.apache.maven.its.plugins:maven-it-plugin-not-exists:pom:1.2.3 from/to " - + "maven-core-it \\(http://localhost:.*/repo\\): Connection to http://localhost:.*2/repo/ refused.*" + + "central \\(http://localhost:.*/repo\\): Connection to http://localhost:.*2/repo/ refused.*" }, "pom-plugin.xml"); } diff --git a/its/core-it-suite/src/test/resources-filtered/settings-remote.xml b/its/core-it-suite/src/test/resources-filtered/settings-remote.xml index d0d696e1f37f..7f714f84b59f 100644 --- a/its/core-it-suite/src/test/resources-filtered/settings-remote.xml +++ b/its/core-it-suite/src/test/resources-filtered/settings-remote.xml @@ -49,7 +49,7 @@ plugins/artifacts from test repos so use of these settings should be the excepti true - true + false @@ -62,7 +62,7 @@ plugins/artifacts from test repos so use of these settings should be the excepti true - true + false diff --git a/its/core-it-suite/src/test/resources-filtered/settings.xml b/its/core-it-suite/src/test/resources-filtered/settings.xml index 4a456c25076e..809f16331c10 100644 --- a/its/core-it-suite/src/test/resources-filtered/settings.xml +++ b/its/core-it-suite/src/test/resources-filtered/settings.xml @@ -51,7 +51,7 @@ own test repos or can employ the local repository (after bootstrapping). This co true - true + false diff --git a/its/core-it-suite/src/test/resources/mng-5175/settings-template.xml b/its/core-it-suite/src/test/resources/mng-5175/settings-template.xml index c6fc8614c87d..2dfd86fa3f75 100644 --- a/its/core-it-suite/src/test/resources/mng-5175/settings-template.xml +++ b/its/core-it-suite/src/test/resources/mng-5175/settings-template.xml @@ -20,5 +20,46 @@ http://localhost:@port@/archiva/repository/internal/ + + + + maven-core-it-repo + + + maven-core-it + http://unimportant + + true + ignore + + + true + ignore + + + + + + maven-core-it + http://localhost:@port@/repo + + true + ignore + + + true + ignore + + + + + + + maven-core-it-repo + diff --git a/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java b/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java index 1c9406fcc129..04be2c6dbfa8 100644 --- a/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java +++ b/its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java @@ -48,9 +48,11 @@ import org.apache.maven.api.cli.ExecutorException; import org.apache.maven.api.cli.ExecutorRequest; import org.apache.maven.cling.executor.ExecutorHelper; +import org.apache.maven.cling.executor.ExecutorTool; import org.apache.maven.cling.executor.embedded.EmbeddedMavenExecutor; import org.apache.maven.cling.executor.forked.ForkedMavenExecutor; import org.apache.maven.cling.executor.internal.HelperImpl; +import org.apache.maven.cling.executor.internal.ToolboxTool; import org.codehaus.plexus.util.FileUtils; import org.codehaus.plexus.util.StringUtils; @@ -88,6 +90,8 @@ public class Verifier { private final ExecutorHelper executorHelper; + private final ExecutorTool executorTool; + private final Path basedir; // the basedir of IT private final Path tempBasedir; // empty basedir for queries @@ -145,6 +149,7 @@ public Verifier(String basedir, List defaultCliArguments) throws Verific this.userHomeDirectory, EMBEDDED_MAVEN_EXECUTOR, FORKED_MAVEN_EXECUTOR); + this.executorTool = new ToolboxTool(executorHelper); this.defaultCliArguments = new ArrayList<>(defaultCliArguments != null ? defaultCliArguments : DEFAULT_CLI_ARGUMENTS); this.logFile = this.basedir.resolve(logFileName); @@ -235,7 +240,7 @@ public void execute() throws VerificationException { if (ret > 0) { String dump; try { - dump = executorHelper.dump(request.toBuilder()).toString(); + dump = executorTool.dump(request.toBuilder()).toString(); } catch (Exception e) { dump = "FAILED: " + e.getMessage(); } @@ -340,14 +345,14 @@ public String getLocalRepositoryWithSettings(String settingsXml) { if (!Files.isRegularFile(settingsFile)) { throw new IllegalArgumentException("settings xml does not exist: " + settingsXml); } - return executorHelper.localRepository(executorHelper + return executorTool.localRepository(executorHelper .executorRequest() .cwd(tempBasedir) .userHomeDirectory(userHomeDirectory) .argument("-s") .argument(settingsFile.toString())); } else { - return executorHelper.localRepository( + return executorTool.localRepository( executorHelper.executorRequest().cwd(tempBasedir).userHomeDirectory(userHomeDirectory)); } } @@ -644,7 +649,7 @@ public String getArtifactPath(String gid, String aid, String version, String ext } return getLocalRepository() + File.separator - + executorHelper.artifactPath(executorHelper.executorRequest(), gav, null); + + executorTool.artifactPath(executorHelper.executorRequest(), gav, null); } private String getSupportArtifactPath(String artifact) { @@ -701,7 +706,7 @@ public String getSupportArtifactPath(String gid, String aid, String version, Str gav = gid + ":" + aid + ":" + ext + ":" + version; } return outerLocalRepository - .resolve(executorHelper.artifactPath( + .resolve(executorTool.artifactPath( executorHelper.executorRequest().argument("-Dmaven.repo.local=" + outerLocalRepository), gav, null)) @@ -776,7 +781,7 @@ public String getArtifactMetadataPath(String gid, String aid, String version, St gav += filename; return getLocalRepository() + File.separator - + executorHelper.metadataPath(executorHelper.executorRequest(), gav, repoId); + + executorTool.metadataPath(executorHelper.executorRequest(), gav, repoId); } /** @@ -806,7 +811,7 @@ public void deleteArtifact(String org, String name, String version, String ext) * @since 1.2 */ public void deleteArtifacts(String gid) throws IOException { - String mdPath = executorHelper.metadataPath(executorHelper.executorRequest(), gid, null); + String mdPath = executorTool.metadataPath(executorHelper.executorRequest(), gid, null); Path dir = Paths.get(getLocalRepository()).resolve(mdPath).getParent(); FileUtils.deleteDirectory(dir.toFile()); } @@ -826,7 +831,7 @@ public void deleteArtifacts(String gid, String aid, String version) throws IOExc requireNonNull(version, "version is null"); String mdPath = - executorHelper.metadataPath(executorHelper.executorRequest(), gid + ":" + aid + ":" + version, null); + executorTool.metadataPath(executorHelper.executorRequest(), gid + ":" + aid + ":" + version, null); Path dir = Paths.get(getLocalRepository()).resolve(mdPath).getParent(); FileUtils.deleteDirectory(dir.toFile()); }