From 2963769b77500553322bcc6c805abb9547ae3b87 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 19 Jul 2023 17:42:02 +0200 Subject: [PATCH] [MNG-7846] Break out of endless loop (#1206) This is most probably "just" about broken Throwable implementations. Some override getCause but does not perform checks to what it was inited as "this" means "not yet inited" actually. Backport of 36db1e35cac5e8c72bf0c795dd08756e50079e05 --- https://issues.apache.org/jira/browse/MNG-7846 --- .../exception/DefaultExceptionHandler.java | 3 +- .../DefaultExceptionHandlerTest.java | 29 +++++++++++++++++++ .../apache/maven/cli/CLIReportingUtils.java | 4 ++- .../org/slf4j/impl/MavenSimpleLogger.java | 2 +- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/exception/DefaultExceptionHandler.java b/maven-core/src/main/java/org/apache/maven/exception/DefaultExceptionHandler.java index b79396ab6a61..32c289c6699a 100644 --- a/maven-core/src/main/java/org/apache/maven/exception/DefaultExceptionHandler.java +++ b/maven-core/src/main/java/org/apache/maven/exception/DefaultExceptionHandler.java @@ -222,7 +222,8 @@ private boolean isNoteworthyException(Throwable exception) { private String getMessage(String message, Throwable exception) { String fullMessage = (message != null) ? message : ""; - for (Throwable t = exception; t != null; t = t.getCause()) { + // To break out of possible endless loop when getCause returns "this" + for (Throwable t = exception; t != null && t != t.getCause(); t = t.getCause()) { String exceptionMessage = t.getMessage(); if (t instanceof AbstractMojoExecutionException) { diff --git a/maven-core/src/test/java/org/apache/maven/exception/DefaultExceptionHandlerTest.java b/maven-core/src/test/java/org/apache/maven/exception/DefaultExceptionHandlerTest.java index 5bb40419c808..33fe83c21d6b 100644 --- a/maven-core/src/test/java/org/apache/maven/exception/DefaultExceptionHandlerTest.java +++ b/maven-core/src/test/java/org/apache/maven/exception/DefaultExceptionHandlerTest.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.net.ConnectException; +import java.util.concurrent.atomic.AtomicReference; import org.apache.maven.model.Plugin; import org.apache.maven.plugin.MojoExecution; @@ -95,4 +96,32 @@ public void testHandleExceptionNoClassDefFoundErrorNull() { String expectedReference = "http://cwiki.apache.org/confluence/display/MAVEN/PluginContainerException"; assertEquals(expectedReference, summary.getReference()); } + + @Test + public void testHandleExceptionLoopInCause() { + // Some broken exception that does return "this" as getCause + AtomicReference causeRef = new AtomicReference<>(null); + Exception cause2 = new RuntimeException("loop") { + @Override + public synchronized Throwable getCause() { + return causeRef.get(); + } + }; + causeRef.set(cause2); + + Plugin plugin = new Plugin(); + Exception cause = new PluginContainerException(plugin, null, null, cause2); + cause2.initCause(cause); + PluginDescriptor pluginDescriptor = new PluginDescriptor(); + MojoDescriptor mojoDescriptor = new MojoDescriptor(); + mojoDescriptor.setPluginDescriptor(pluginDescriptor); + MojoExecution mojoExecution = new MojoExecution(mojoDescriptor); + Throwable exception = new PluginExecutionException(mojoExecution, null, cause); + + DefaultExceptionHandler handler = new DefaultExceptionHandler(); + ExceptionSummary summary = handler.handleException(exception); + + String expectedReference = "http://cwiki.apache.org/confluence/display/MAVEN/PluginContainerException"; + assertEquals(expectedReference, summary.getReference()); + } } diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java b/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java index 49eb1512d28c..33d94f370406 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java @@ -144,7 +144,9 @@ public static void showError(Logger logger, String message, Throwable e, boolean if (e != null) { logger.error(e.getMessage()); - for (Throwable cause = e.getCause(); cause != null; cause = cause.getCause()) { + for (Throwable cause = e.getCause(); + cause != null && cause != cause.getCause(); + cause = cause.getCause()) { logger.error("Caused by: {}", cause.getMessage()); } } diff --git a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenSimpleLogger.java b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenSimpleLogger.java index 009311621409..0d9b9c390e50 100644 --- a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenSimpleLogger.java +++ b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenSimpleLogger.java @@ -78,7 +78,7 @@ private void printStackTrace(Throwable t, PrintStream stream, String prefix) { writeThrowable(se, stream, "Suppressed", prefix + " "); } Throwable cause = t.getCause(); - if (cause != null) { + if (cause != null && t != cause) { writeThrowable(cause, stream, "Caused by", prefix); } }