This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 50aecc7432 [MNG-8419][MNG-8424] Too aggressive warning for pre-Maven4
passwords (#1970)
50aecc7432 is described below
commit 50aecc74328d451dd82a92b7e378a217f45a6033
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Thu Dec 12 17:43:08 2024 +0100
[MNG-8419][MNG-8424] Too aggressive warning for pre-Maven4 passwords (#1970)
Toned down the Maven4 sec dispatcher messages. Also, IF maven3 passwords
detected AND there was `.mvn/extensions.xml` the warnings were doubled.
Examples:
Failure to start Maven (due non-decryptable passwords):
```
$ mvn clean
[ERROR] Error executing Maven.
[ERROR] Error building settings
* FATAL: Could not decrypt password (fix the corrupted password or remove
it, if unused) {xL6L/HbmrY++sNkphnq3fguYepTpM04WlIXb8nB1pk=}
* WARNING: Detected 2 pre-Maven 4 legacy encrypted password(s) - configure
password encryption with the help of mvnenc for increased security.
$
```
Warning at start (due Maven3 passwords):
```
$ mvn clean
[INFO]
[INFO] Some problems were encountered while building the effective settings
(use -X to see details)
[INFO]
[INFO] Scanning for projects...
[INFO]
--------------------------------------------------------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Apache Maven
...
```
---
https://issues.apache.org/jira/browse/MNG-8424
https://issues.apache.org/jira/browse/MNG-8419
---
.../maven/api/services/MavenBuilderException.java | 68 ++++++++++++++++++++++
.../api/services/SettingsBuilderException.java | 13 +----
.../api/services/ToolchainsBuilderException.java | 13 +----
.../apache/maven/cling/invoker/LookupInvoker.java | 27 ++++++---
.../invoker/PlexusContainerCapsuleFactory.java | 4 +-
.../internal/impl/DefaultSettingsBuilder.java | 26 +++++----
.../maven/it/MavenITmng3748BadSettingsXmlTest.java | 4 ++
.../it/MavenITmng8379SettingsDecryptTest.java | 6 +-
8 files changed, 118 insertions(+), 43 deletions(-)
diff --git
a/api/maven-api-core/src/main/java/org/apache/maven/api/services/MavenBuilderException.java
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/MavenBuilderException.java
new file mode 100644
index 0000000000..1af8ab3355
--- /dev/null
+++
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/MavenBuilderException.java
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://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.apache.maven.api.services;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+
+import org.apache.maven.api.annotations.Experimental;
+
+/**
+ * Base class for all maven exceptions carrying {@link BuilderProblem}s.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+public abstract class MavenBuilderException extends MavenException {
+
+ private final List<BuilderProblem> problems;
+
+ public MavenBuilderException(String message, Throwable cause) {
+ super(message, cause);
+ problems = List.of();
+ }
+
+ public MavenBuilderException(String message, List<BuilderProblem>
problems) {
+ super(buildMessage(message, problems), null);
+ this.problems = problems;
+ }
+
+ /**
+ * Formats message out of problems: problems are sorted (in natural order
of {@link BuilderProblem.Severity})
+ * and then a list is built. These exceptions are usually thrown in
"fatal" cases (and usually prevent Maven
+ * from starting), and these exceptions may end up very early on output.
+ */
+ protected static String buildMessage(String message, List<BuilderProblem>
problems) {
+ StringBuilder msg = new StringBuilder(message);
+ ArrayList<BuilderProblem> sorted = new ArrayList<>(problems);
+ sorted.sort(Comparator.comparing(BuilderProblem::getSeverity));
+ for (BuilderProblem problem : sorted) {
+ msg.append("\n * ")
+ .append(problem.getSeverity().name())
+ .append(": ")
+ .append(problem.getMessage());
+ }
+ return msg.toString();
+ }
+
+ public List<BuilderProblem> getProblems() {
+ return problems;
+ }
+}
diff --git
a/api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java
index a8576b145e..c324a9250a 100644
---
a/api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java
+++
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java
@@ -20,7 +20,6 @@ package org.apache.maven.api.services;
import java.io.Serial;
import java.util.List;
-import java.util.stream.Collectors;
import org.apache.maven.api.annotations.Experimental;
@@ -30,28 +29,20 @@ import org.apache.maven.api.annotations.Experimental;
* @since 4.0.0
*/
@Experimental
-public class SettingsBuilderException extends MavenException {
+public class SettingsBuilderException extends MavenBuilderException {
@Serial
private static final long serialVersionUID = 4714858598345418083L;
- private final List<BuilderProblem> problems;
-
/**
* @param message the message to give
* @param e the {@link Exception}
*/
public SettingsBuilderException(String message, Exception e) {
super(message, e);
- this.problems = List.of();
}
public SettingsBuilderException(String message, List<BuilderProblem>
problems) {
- super(message + ": " +
problems.stream().map(BuilderProblem::toString).collect(Collectors.joining(",
")), null);
- this.problems = List.copyOf(problems);
- }
-
- public List<BuilderProblem> getProblems() {
- return problems;
+ super(message, problems);
}
}
diff --git
a/api/maven-api-core/src/main/java/org/apache/maven/api/services/ToolchainsBuilderException.java
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/ToolchainsBuilderException.java
index b6e429dfda..89a2c8ffef 100644
---
a/api/maven-api-core/src/main/java/org/apache/maven/api/services/ToolchainsBuilderException.java
+++
b/api/maven-api-core/src/main/java/org/apache/maven/api/services/ToolchainsBuilderException.java
@@ -20,7 +20,6 @@ package org.apache.maven.api.services;
import java.io.Serial;
import java.util.List;
-import java.util.stream.Collectors;
import org.apache.maven.api.annotations.Experimental;
@@ -30,28 +29,20 @@ import org.apache.maven.api.annotations.Experimental;
* @since 4.0.0
*/
@Experimental
-public class ToolchainsBuilderException extends MavenException {
+public class ToolchainsBuilderException extends MavenBuilderException {
@Serial
private static final long serialVersionUID = 7899871809665729349L;
- private final List<BuilderProblem> problems;
-
/**
* @param message the message to give
* @param e the {@link Exception}
*/
public ToolchainsBuilderException(String message, Exception e) {
super(message, e);
- this.problems = List.of();
}
public ToolchainsBuilderException(String message, List<BuilderProblem>
problems) {
- super(message + ": " +
problems.stream().map(BuilderProblem::toString).collect(Collectors.joining(",
")), null);
- this.problems = List.copyOf(problems);
- }
-
- public List<BuilderProblem> getProblems() {
- return problems;
+ super(message, problems);
}
}
diff --git
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
index 48c4c56c0a..348492f069 100644
---
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
+++
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
@@ -465,10 +465,18 @@ public abstract class LookupInvoker<C extends
LookupContext> implements Invoker
}
protected void settings(C context) throws Exception {
- settings(context, context.lookup.lookup(SettingsBuilder.class));
+ settings(context, true, context.lookup.lookup(SettingsBuilder.class));
}
- protected void settings(C context, SettingsBuilder settingsBuilder) throws
Exception {
+ /**
+ * This method is invoked twice during "normal" LookupInvoker level
startup: once when (if present) extensions
+ * are loaded up during Plexus DI creation, and once afterward as "normal"
boot procedure.
+ * <p>
+ * If there are Maven3 passwords presents in settings, this results in
doubled warnings emitted. So Plexus DI
+ * creation call keeps "emitSettingsWarnings" false. If there are fatal
issues, it will anyway "die" at that
+ * spot before warnings would be emitted.
+ */
+ protected void settings(C context, boolean emitSettingsWarnings,
SettingsBuilder settingsBuilder) throws Exception {
Options mavenOptions = context.invokerRequest.options();
Path userSettingsFile = null;
@@ -557,14 +565,17 @@ public abstract class LookupInvoker<C extends
LookupContext> implements Invoker
context.interactive = mayDisableInteractiveMode(context,
context.effectiveSettings.isInteractiveMode());
context.localRepositoryPath = localRepositoryPath(context);
- if (!settingsResult.getProblems().isEmpty()) {
- context.logger.warn("");
- context.logger.warn("Some problems were encountered while building
the effective settings");
+ if (emitSettingsWarnings && !settingsResult.getProblems().isEmpty()) {
+ context.logger.info("");
+ context.logger.info(
+ "Some problems were encountered while building the
effective settings (use -X to see details)");
- for (BuilderProblem problem : settingsResult.getProblems()) {
- context.logger.warn(problem.getMessage() + " @ " +
problem.getLocation());
+ if (context.invokerRequest.options().verbose().orElse(false)) {
+ for (BuilderProblem problem : settingsResult.getProblems()) {
+ context.logger.warn(problem.getMessage() + " @ " +
problem.getLocation());
+ }
}
- context.logger.warn("");
+ context.logger.info("");
}
}
diff --git
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/PlexusContainerCapsuleFactory.java
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/PlexusContainerCapsuleFactory.java
index 9b4d5a40a1..2f38469843 100644
---
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/PlexusContainerCapsuleFactory.java
+++
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/PlexusContainerCapsuleFactory.java
@@ -75,7 +75,7 @@ public class PlexusContainerCapsuleFactory<C extends
LookupContext> implements C
return new PlexusContainerCapsule(
context, Thread.currentThread().getContextClassLoader(),
container(invoker, context));
} catch (Exception e) {
- throw new InvokerException("Failed to create plexus container
capsule", e);
+ throw new InvokerException("Failed to create Plexus DI Container",
e);
}
}
@@ -279,7 +279,7 @@ public class PlexusContainerCapsuleFactory<C extends
LookupContext> implements C
container.getLoggerManager().setThresholds(toPlexusLoggingLevel(context.loggerLevel));
Thread.currentThread().setContextClassLoader(container.getContainerRealm());
- invoker.settings(context, container.lookup(SettingsBuilder.class));
+ invoker.settings(context, false,
container.lookup(SettingsBuilder.class));
MavenExecutionRequest mer = new DefaultMavenExecutionRequest();
invoker.populateRequest(context, new DefaultLookup(container),
mer);
diff --git
a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/DefaultSettingsBuilder.java
b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/DefaultSettingsBuilder.java
index fd1bbfc072..e542689f61 100644
---
a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/DefaultSettingsBuilder.java
+++
b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/DefaultSettingsBuilder.java
@@ -29,6 +29,7 @@ import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -273,18 +274,12 @@ public class DefaultSettingsBuilder implements
SettingsBuilder {
return settings;
}
SecDispatcher secDispatcher = new DefaultSecDispatcher(dispatchers,
getSecuritySettings(request.getSession()));
+ final AtomicInteger preMaven4Passwords = new AtomicInteger(0);
Function<String, String> decryptFunction = str -> {
if (str != null && !str.isEmpty() && !str.contains("${") &&
secDispatcher.isAnyEncryptedString(str)) {
if (secDispatcher.isLegacyEncryptedString(str)) {
// add a problem
- problems.add(new DefaultBuilderProblem(
- settingsSource.getLocation(),
- -1,
- -1,
- null,
- "Pre-Maven 4 legacy encrypted password detected "
- + " - configure password encryption with
the help of mvnenc to be compatible with Maven 4.",
- BuilderProblem.Severity.WARNING));
+ preMaven4Passwords.incrementAndGet();
}
try {
return secDispatcher.decrypt(str);
@@ -295,12 +290,23 @@ public class DefaultSettingsBuilder implements
SettingsBuilder {
-1,
e,
"Could not decrypt password (fix the corrupted
password or remove it, if unused) " + str,
- BuilderProblem.Severity.ERROR));
+ BuilderProblem.Severity.FATAL));
}
}
return str;
};
- return new SettingsTransformer(decryptFunction).visit(settings);
+ Settings result = new
SettingsTransformer(decryptFunction).visit(settings);
+ if (preMaven4Passwords.get() > 0) {
+ problems.add(new DefaultBuilderProblem(
+ settingsSource.getLocation(),
+ -1,
+ -1,
+ null,
+ "Detected " + preMaven4Passwords.get() + " pre-Maven 4
legacy encrypted password(s) "
+ + "- configure password encryption with the help
of mvnenc for increased security.",
+ BuilderProblem.Severity.WARNING));
+ }
+ return result;
}
private Path getSecuritySettings(ProtoSession session) {
diff --git
a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3748BadSettingsXmlTest.java
b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3748BadSettingsXmlTest.java
index 235d634cc6..8dac9578b6 100644
---
a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3748BadSettingsXmlTest.java
+++
b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3748BadSettingsXmlTest.java
@@ -21,6 +21,7 @@ package org.apache.maven.it;
import java.io.File;
import java.util.List;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -33,7 +34,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
*
* @author jdcasey
*
+ * NOTE (cstamas): this IT was written to test that settings.xml is STRICT,
while later changes modified
+ * this very IT into the opposite: to test that parsing is LENIENT.
*/
+@Disabled("This is archaic test; we should strive to make settings.xml parsing
strict again")
public class MavenITmng3748BadSettingsXmlTest extends
AbstractMavenIntegrationTestCase {
public MavenITmng3748BadSettingsXmlTest() {
diff --git
a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java
b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java
index a615d6f2fc..fbc80d4d92 100644
---
a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java
+++
b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java
@@ -47,7 +47,8 @@ class MavenITmng8379SettingsDecryptTest extends
AbstractMavenIntegrationTestCase
verifier.verifyErrorFreeLog();
// there is a warning and all fields decrypted
- verifier.verifyTextInLog("[WARNING] Pre-Maven 4 legacy encrypted
password detected");
+ verifier.verifyTextInLog(
+ "[INFO] Some problems were encountered while building the
effective settings (use -X to see details)");
verifier.verifyTextInLog("<password>testtest</password>");
verifier.verifyTextInLog("<value>testtest</value>");
}
@@ -69,6 +70,9 @@ class MavenITmng8379SettingsDecryptTest extends
AbstractMavenIntegrationTestCase
verifier.verifyErrorFreeLog();
// there is no warning and all fields decrypted
+ verifier.verifyTextNotInLog("[WARNING]");
+ verifier.verifyTextNotInLog(
+ "[INFO] Some problems were encountered while building the
effective settings (use -X to see details)");
verifier.verifyTextInLog("<password>testtest</password>");
verifier.verifyTextInLog("<value>secretHeader</value>");
}
