Widen restrictive archive traversal, fixes #1925

Includes unit-tests against a handrolled archive containing multiple other archives, each with a single classfile.
spotbugs · Mar 21, 2022 · ec66ae4 · ec66ae4
1 parent 5bdc84e
commit ec66ae4
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 ## Unreleased - 2022-??-??
 ### Fixed
 - Bumped Saxon-HE from 10.6 to 11.2 ([#1955](https://github.com/spotbugs/spotbugs/pull/1955))
+- Fixed traversal of nested archives governed by `-nested:true` ([#1930](https://github.com/spotbugs/spotbugs/pull/1930))
 
 ## 4.6.0 - 2022-03-08
 ### Fixed

diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/classfile/impl/ClassPathBuilderTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/classfile/impl/ClassPathBuilderTest.java
@@ -0,0 +1,36 @@
+package edu.umd.cs.findbugs.classfile.impl;
+
+import edu.umd.cs.findbugs.AppVersion;
+import edu.umd.cs.findbugs.BugCollection;
+import edu.umd.cs.findbugs.test.SpotBugsRule;
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.nio.file.Paths;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class ClassPathBuilderTest {
+
+    @Rule
+    public SpotBugsRule analyzer = new SpotBugsRule();
+
+    @Test
+    public void nestedTraversalDisabled() {
+        BugCollection results = analyzer.performAnalysis((engine) -> {
+            engine.setScanNestedArchives(false);
+            engine.setNoClassOk(true);
+        }, Paths.get("../spotbugsTestCases/archives/nestedArchive.jar"));
+        AppVersion appInformation = results.getCurrentAppVersion();
+        assertThat(appInformation.getNumClasses(), equalTo(0));
+    }
+
+    @Test
+    public void nestedTraversalEnabled() {
+        BugCollection results = analyzer.performAnalysis((engine) -> engine.setScanNestedArchives(true),
+                Paths.get("../spotbugsTestCases/archives/nestedArchive.jar"));
+        AppVersion appInformation = results.getCurrentAppVersion();
+        assertThat(appInformation.getNumClasses(), equalTo(5));
+    }
+}
diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/classfile/impl/ClassPathBuilder.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/classfile/impl/ClassPathBuilder.java
@@ -700,9 +700,9 @@ private void scanCodebase(IClassPath classPath, LinkedList<WorkListItem> workLis
 
             // If resource is a nested archive, add it to the worklist
             if (scanNestedArchives && (codeBase.isApplicationCodeBase() || codeBase instanceof DirectoryCodeBase)
-                    && Archive.isLibraryFileName(entry.getResourceName())) {
+                    && Archive.isArchiveFileName(entry.getResourceName())) {
                 if (VERBOSE) {
-                    System.out.println("Entry is an library!");
+                    System.out.println("Entry is a library!");
                 }
                 ICodeBaseLocator nestedArchiveLocator = classFactory.createNestedArchiveCodeBaseLocator(codeBase,
                         entry.getResourceName());

diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/util/Archive.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/util/Archive.java
@@ -71,6 +71,8 @@ private static String getExtension(String fileName) {
         return extension;
     }
 
+    @Deprecated
+    @SuppressWarnings("unused") // unused within the codebase, but deprecated until next release for public API reasons
     public static boolean isLibraryFileName(String fileName) {
         String extension = getExtension(fileName);
         return ".jar".equals(extension);

diff --git a/spotbugsTestCases/archives/nestedArchive.jar b/spotbugsTestCases/archives/nestedArchive.jar
diff --git a/test-harness-core/src/main/java/edu/umd/cs/findbugs/test/AnalysisRunner.java b/test-harness-core/src/main/java/edu/umd/cs/findbugs/test/AnalysisRunner.java
@@ -18,6 +18,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.Consumer;
 import java.util.jar.JarOutputStream;
 import java.util.zip.ZipEntry;
 
@@ -29,6 +30,7 @@
 import edu.umd.cs.findbugs.BugRanker;
 import edu.umd.cs.findbugs.DetectorFactoryCollection;
 import edu.umd.cs.findbugs.FindBugs2;
+import edu.umd.cs.findbugs.IFindBugsEngine;
 import edu.umd.cs.findbugs.Plugin;
 import edu.umd.cs.findbugs.PluginException;
 import edu.umd.cs.findbugs.Priorities;
@@ -40,7 +42,7 @@
 /**
  * <p>
  * This class runs analysis with SpotBugs. The target class files and
- * auxClasspathEntries should be specified before you invoke {@link #run(Path...)}
+ * auxClasspathEntries should be specified before you invoke {@link #run(Consumer, Path...)}
  * method.
  * </p>
  *
@@ -84,6 +86,12 @@ public AnalysisRunner addAuxClasspathEntry(Path path) {
 
     @Nonnull
     public BugCollectionBugReporter run(Path... files) {
+        return this.run((engine) -> {
+        }, files);
+    }
+
+    @Nonnull
+    public BugCollectionBugReporter run(Consumer<IFindBugsEngine> engineCustomization, Path... files) {
         DetectorFactoryCollection.resetInstance(new DetectorFactoryCollection());
 
         try (FindBugs2 engine = new FindBugs2(); Project project = createProject(files)) {
@@ -102,6 +110,7 @@ public BugCollectionBugReporter run(Path... files) {
             preferences.enableAllDetectors(true);
             engine.setUserPreferences(preferences);
 
+            engineCustomization.accept(engine);
             try {
                 engine.execute();
             } catch (final IOException | InterruptedException e) {

diff --git a/test-harness/src/main/java/edu/umd/cs/findbugs/test/SpotBugsRule.java b/test-harness/src/main/java/edu/umd/cs/findbugs/test/SpotBugsRule.java
@@ -1,10 +1,12 @@
 package edu.umd.cs.findbugs.test;
 
 import java.nio.file.Path;
+import java.util.function.Consumer;
 
 import javax.annotation.Nonnull;
 import javax.annotation.ParametersAreNonnullByDefault;
 
+import edu.umd.cs.findbugs.IFindBugsEngine;
 import org.junit.rules.ExternalResource;
 
 import edu.umd.cs.findbugs.BugCollection;
@@ -76,16 +78,33 @@ public SpotBugsRule addAuxClasspathEntry(Path path) {
      * <p>
      * Run SpotBugs under given condition, and return its result.
      * </p>
+     * @param engineCustomization
+     *      A customization of the engine to apply before running engine#execute
      * @param paths
      *     Paths of target class files
      * @return a {@link BugCollection} which contains all detected bugs.
      */
     // TODO let users specify SlashedClassName, then find its file path automatically
     @Nonnull
-    public BugCollection performAnalysis(Path... paths) {
+    public BugCollection performAnalysis(Consumer<IFindBugsEngine> engineCustomization, Path... paths) {
         if (runner == null) {
             throw new IllegalStateException("Please call this performAnalysis() method in test method");
         }
-        return runner.run(paths).getBugCollection();
+        return runner.run(engineCustomization, paths).getBugCollection();
+    }
+
+    /**
+     * <p>
+     * Run SpotBugs under given condition, and return its result.
+     * </p>
+     * @param paths
+     *     Paths of target class files
+     * @return a {@link BugCollection} which contains all detected bugs.
+     */
+    // TODO let users specify SlashedClassName, then find its file path automatically
+    @Nonnull
+    public BugCollection performAnalysis(Path... paths) {
+        return performAnalysis(e -> {
+        }, paths);
     }
 }