Merge pull request #4655 from dwnusbaum/JENKINS-61841-rebased

[JENKINS-61841] Limit the number of exceptions stored by CompositeIOException

core/src/main/java/jenkins/util/io/CompositeIOException.java

@@ -30,19 +30,50 @@ import org.kohsuke.accmod.restrictions.NoExternalUse;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
 @Restricted(NoExternalUse.class)
 public class CompositeIOException extends IOException {
+    private static final long serialVersionUID = 121943141387608148L;
+
+    /**
+     * The maximum number of exceptions that can be reported by a single
+     * {@code CompositeIOException}.
+     * <p>
+     * The number of exceptions is limited to avoid pathological cases where
+     * a huge number of exceptions could lead to excessive memory usage.
+     * For example, if the number of exceptions was unlimited, a call to
+     * {@code Util.deleteRecursive} could fail with a
+     * {@code CompositeIOException} that contains an exception for every
+     * single file inside of the directory.
+     */
+    public static final int EXCEPTION_LIMIT = 10;
+
     private final List<IOException> exceptions;
 
+    /**
+     * Construct a new {@code CompositeIOException} where the given list of
+     * exceptions are added as suppressed exceptions to the new exception.
+     * <p>
+     * If the given list of exceptions is longer than {@link #EXCEPTION_LIMIT},
+     * the list will be truncated to that length, and a message indicating the
+     * number of discarded exceptions will be appended to the original message.
+     */
     public CompositeIOException(String message, @NonNull List<IOException> exceptions) {
-        super(message);
-        this.exceptions = exceptions;
-        exceptions.forEach(this::addSuppressed);
+        super(message + getDiscardedExceptionsMessage(exceptions));
+        if (exceptions.size() > EXCEPTION_LIMIT) {
+            this.exceptions = new ArrayList<>(exceptions.subList(0, EXCEPTION_LIMIT));
+        } else {
+            this.exceptions = exceptions;
+        }
+        this.exceptions.forEach(this::addSuppressed);
     }
 
+    /**
+     * @see CompositeIOException(String, List)
+     */
     public CompositeIOException(String message, IOException... exceptions) {
         this(message, Arrays.asList(exceptions));
     }
@@ -54,4 +85,12 @@ public class CompositeIOException extends IOException {
     public UncheckedIOException asUncheckedIOException() {
         return new UncheckedIOException(this);
     }
+
+    private static String getDiscardedExceptionsMessage(List<IOException> exceptions) {
+        if (exceptions.size() > EXCEPTION_LIMIT) {
+            return " (Discarded " + (exceptions.size() - EXCEPTION_LIMIT) + " additional exceptions)";
+        } else {
+            return "";
+        }
+    }
 }

core/src/test/java/jenkins/util/io/PathRemoverTest.java

@@ -51,10 +51,17 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeTrue;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
@@ -418,6 +425,33 @@ public class PathRemoverTest {
         assertFalse(d1.exists());
     }
 
+    @Test
+    @Issue("JENKINS-55448")
+    public void testForceRemoveRecursive_TruncatesNumberOfExceptions() throws IOException {
+        assumeTrue(Functions.isWindows());
+        final int maxExceptions = CompositeIOException.EXCEPTION_LIMIT;
+        final int lockedFiles = maxExceptions + 5;
+        final int totalFiles = lockedFiles + 5;
+        File dir = tmp.newFolder();
+        File[] files = new File[totalFiles];
+        for (int i = 0; i < totalFiles; i++) {
+            files[i] = new File(dir, "f" + i);
+        }
+        touchWithFileName(files);
+        for (int i = 0; i < lockedFiles; i++) {
+            locker.acquireLock(files[i]);
+        }
+        try {
+            PathRemover.newSimpleRemover().forceRemoveRecursive(dir.toPath());
+            fail("Deletion should have failed");
+        } catch (CompositeIOException e) {
+            assertThat(e.getSuppressed(), arrayWithSize(maxExceptions));
+            assertThat(e.getMessage(), endsWith("(Discarded " + (lockedFiles + 1 - maxExceptions) + " additional exceptions)"));
+        }
+        assertTrue(dir.exists());
+        assertThat(dir.listFiles().length, equalTo(lockedFiles));
+    }
+
     private static void mkdirs(File... dirs) {
         for (File dir : dirs) {
             assertTrue("Could not mkdir " + dir, dir.mkdir());