From 978818983042dcad1e997cc81c8a98c9724bfb80 Mon Sep 17 00:00:00 2001 From: Dimitris Kavvathas Date: Wed, 7 Dec 2022 11:10:47 +0100 Subject: [PATCH] SONAR-17518 Ignore AccessDeniedException of directory, if it is excluded. --- .../api/batch/fs/internal/PathPattern.java | 7 +- .../filesystem/AbstractExclusionFilters.java | 27 ++- .../scan/filesystem/ProjectFileIndexer.java | 54 +++++- .../mediumtest/fs/FileSystemMediumTest.java | 174 ++++++++++++++++++ 4 files changed, 251 insertions(+), 11 deletions(-) diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/PathPattern.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/PathPattern.java index fb5f77f3967..eff388bb44a 100644 --- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/PathPattern.java +++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/PathPattern.java @@ -20,6 +20,7 @@ package org.sonar.api.batch.fs.internal; import java.nio.file.Path; +import java.util.Arrays; import javax.annotation.concurrent.ThreadSafe; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang.StringUtils; @@ -58,11 +59,7 @@ public abstract class PathPattern { } public static PathPattern[] create(String[] s) { - PathPattern[] result = new PathPattern[s.length]; - for (int i = 0; i < s.length; i++) { - result[i] = create(s[i]); - } - return result; + return Arrays.stream(s).map(PathPattern::create).toArray(PathPattern[]::new); } /** diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/AbstractExclusionFilters.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/AbstractExclusionFilters.java index 2d808175dd0..545d653fbf1 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/AbstractExclusionFilters.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/AbstractExclusionFilters.java @@ -45,7 +45,7 @@ public abstract class AbstractExclusionFilters { private PathPattern[] testInclusionsPattern; private PathPattern[] testExclusionsPattern; - public AbstractExclusionFilters(Function configProvider) { + protected AbstractExclusionFilters(Function configProvider) { this.sourceInclusions = inclusions(configProvider, CoreProperties.PROJECT_INCLUSIONS_PROPERTY); this.testInclusions = inclusions(configProvider, CoreProperties.PROJECT_TEST_INCLUSIONS_PROPERTY); this.sourceExclusions = exclusions(configProvider, CoreProperties.GLOBAL_EXCLUSIONS_PROPERTY, CoreProperties.PROJECT_EXCLUSIONS_PROPERTY); @@ -138,10 +138,6 @@ public abstract class AbstractExclusionFilters { public boolean isExcluded(Path absolutePath, Path relativePath, InputFile.Type type) { PathPattern[] exclusionPatterns = InputFile.Type.MAIN == type ? mainExclusionsPattern : testExclusionsPattern; - if (exclusionPatterns.length == 0) { - return false; - } - for (PathPattern pattern : exclusionPatterns) { if (pattern.match(absolutePath, relativePath)) { return true; @@ -150,4 +146,25 @@ public abstract class AbstractExclusionFilters { return false; } + + /** + *

Checks if the file should be excluded as a parent directory of excluded files and subdirectories.

+ * + * @param absolutePath The full path of the file. + * @param relativePath The relative path of the file. + * @param baseDir The base directory of the project. + * @param type The file type. + * @return True if the file should be excluded, false otherwise. + */ + public boolean isExcludedAsParentDirectoryOfExcludedChildren(Path absolutePath, Path relativePath, Path baseDir, InputFile.Type type) { + PathPattern[] exclusionPatterns = InputFile.Type.MAIN == type ? mainExclusionsPattern : testExclusionsPattern; + + return Stream.of(exclusionPatterns) + .map(PathPattern::toString) + .filter(ps -> ps.endsWith("/**/*")) + .map(ps -> ps.substring(0, ps.length() - 5)) + .map(baseDir::resolve) + .anyMatch(exclusionRootPath -> absolutePath.startsWith(exclusionRootPath) + || PathPattern.create(exclusionRootPath.toString()).match(absolutePath, relativePath)); + } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFileIndexer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFileIndexer.java index 1219428a305..0c7b24d235c 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFileIndexer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFileIndexer.java @@ -20,6 +20,7 @@ package org.sonar.scanner.scan.filesystem; import java.io.IOException; +import java.nio.file.AccessDeniedException; import java.nio.file.FileSystemLoopException; import java.nio.file.FileVisitOption; import java.nio.file.FileVisitResult; @@ -38,6 +39,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.SystemUtils; +import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.InputFile.Type; import org.sonar.api.batch.fs.internal.DefaultInputModule; import org.sonar.api.batch.scm.IgnoreCommand; @@ -216,6 +218,27 @@ public class ProjectFileIndexer { new IndexFileVisitor(module, moduleExclusionFilters, moduleCoverageAndDuplicationExclusions, type, exclusionCounter)); } + + /** + *

Checks if the path is a directory that is excluded.

+ * + *

Exclusions patterns are checked both at project and module level.

+ * + * @param moduleExclusionFilters The exclusion filters. + * @param realAbsoluteFile The path to be checked. + * @param projectBaseDir The project base directory. + * @param moduleBaseDir The module base directory. + * @param type The input file type. + * @return True if path is an excluded directory, false otherwise. + */ + private static boolean isExcludedDirectory(ModuleExclusionFilters moduleExclusionFilters, Path realAbsoluteFile, Path projectBaseDir, Path moduleBaseDir, + InputFile.Type type) { + Path projectRelativePath = projectBaseDir.relativize(realAbsoluteFile); + Path moduleRelativePath = moduleBaseDir.relativize(realAbsoluteFile); + return moduleExclusionFilters.isExcludedAsParentDirectoryOfExcludedChildren(realAbsoluteFile, projectRelativePath, projectBaseDir, type) + || moduleExclusionFilters.isExcludedAsParentDirectoryOfExcludedChildren(realAbsoluteFile, moduleRelativePath, moduleBaseDir, type); + } + private class IndexFileVisitor implements FileVisitor { private final DefaultInputModule module; private final ModuleExclusionFilters moduleExclusionFilters; @@ -249,16 +272,45 @@ public class ProjectFileIndexer { return FileVisitResult.CONTINUE; } + /** + *

Overridden method to handle exceptions while visiting files in the analysis.

+ * + *

+ *

    + *
  • FileSystemLoopException - We show a warning that a symlink loop exists and we skip the file.
    • + *
  • AccessDeniedException for excluded files/directories - We skip the file, as files excluded from the analysis, shouldn't throw access exceptions.
    • + *
+ *

+ * + * @param file a reference to the file + * @param exc the I/O exception that prevented the file from being visited + * + * @throws IOException + */ @Override public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { if (exc instanceof FileSystemLoopException) { LOG.warn("Not indexing due to symlink loop: {}", file.toFile()); return FileVisitResult.CONTINUE; + } else if (exc instanceof AccessDeniedException && isExcluded(file)) { + return FileVisitResult.CONTINUE; } - throw exc; } + /** + *

Checks if the directory is excluded in the analysis or not. Only the exclusions are checked.

+ * + *

The inclusions cannot be checked for directories, since the current implementation of pattern matching is intended only for files.

+ * + * @param path The file or directory. + * @return True if file/directory is excluded from the analysis, false otherwise. + */ + private boolean isExcluded(Path path) throws IOException { + Path realAbsoluteFile = path.toRealPath(LinkOption.NOFOLLOW_LINKS).toAbsolutePath().normalize(); + return isExcludedDirectory(moduleExclusionFilters, realAbsoluteFile, inputModuleHierarchy.root().getBaseDir(), module.getBaseDir(), type); + } + @Override public FileVisitResult postVisitDirectory(Path dir, IOException exc) { return FileVisitResult.CONTINUE; diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java index 59d6ff9b60d..a178d56f9fc 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java @@ -29,9 +29,11 @@ import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Random; +import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -1084,6 +1086,178 @@ public class FileSystemMediumTest { assertThat(result.inputFile("sample.xoo")).isNotNull(); } + @Test + public void givenExclusionEndingWithOneWildcardWhenAnalysedThenOnlyDirectChildrenFilesShouldBeExcluded() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", true); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + // src/srcSubDir/srcSubSubDir/subSubSrc.xoo + File srcSubSubDir = createDir(srcSubDir, "srcSubSubDir", true); + writeFile(srcSubSubDir, "subSubSrc.xoo", "Sample xoo\ncontent"); + + AnalysisResult result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.exclusions", "src/srcSubDir/*") + .build()) + .execute(); + + assertAnalysedFiles(result, "src/src.xoo", "src/srcSubDir/srcSubSubDir/subSubSrc.xoo"); + } + + @Test + public void givenPathsWithoutReadPermissionWhenAllChildrenAreExcludedThenScannerShouldSkipIt() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", false); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + // src/srcSubDir2/srcSub2.xoo + File srcSubDir2 = createDir(srcDir, "srcSubDir2", true); + writeFile(srcSubDir2, "srcSub2.xoo", "Sample 2 xoo\ncontent").setReadable(false); + + // src/srcSubDir2/srcSubSubDir2/srcSubSub2.xoo + File srcSubSubDir2 = createDir(srcSubDir2, "srcSubSubDir2", false); + writeFile(srcSubSubDir2, "srcSubSub2.xoo", "Sample xoo\ncontent"); + + AnalysisResult result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.exclusions", "src/srcSubDir/**/*,src/srcSubDir2/**/*") + .build()) + .execute(); + + assertAnalysedFiles(result, "src/src.xoo"); + assertThat(logTester.logs()).contains("1 file ignored because of inclusion/exclusion patterns"); + } + + @Test + public void givenFileWithoutAccessWhenChildrenAreExcludedThenThenScanShouldFail() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo\ncontent").setReadable(false); + + AnalysisBuilder result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.exclusions", "src/src.xoo/**/*") // incorrect pattern, but still the scan should fail if src.xoo is not accessible + .build()); + + assertThatThrownBy(result::execute) + .isExactlyInstanceOf(IllegalStateException.class) + .hasMessageStartingWith(format("java.lang.IllegalStateException: Unable to read file")); + } + + @Test + public void givenDirectoryWithoutReadPermissionWhenIncludedThenScanShouldFail() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", false); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + AnalysisBuilder result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.exclusions", "src/srcSubDir/*") // srcSubDir should not be excluded unless all children are excluded (src/srcSubDir/**/*) + .build()); + + assertThatThrownBy(result::execute) + .isExactlyInstanceOf(IllegalStateException.class) + .hasMessageEndingWith(format("Failed to index files")); + } + + @Test + public void givenDirectoryWhenAllChildrenAreExcludedThenSkippedFilesShouldBeReported() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", true); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + // src/srcSubDir2/srcSub2.xoo + File srcSubDir2 = createDir(srcDir, "srcSubDir2", true); + writeFile(srcSubDir2, "srcSub2.xoo", "Sample 2 xoo\ncontent").setReadable(false); + + AnalysisResult result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.exclusions", "src/srcSubDir/**/*,src/srcSubDir2/*") + .build()) + .execute(); + + assertAnalysedFiles(result, "src/src.xoo"); + assertThat(logTester.logs()).contains("2 files ignored because of inclusion/exclusion patterns"); + } + + @Ignore("Fails until we can pattern match inclusions to directories, not only files.") + @Test + public void givenDirectoryWithoutReadPermissionWhenNotIncludedThenScanShouldSkipIt() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", true); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + // src/srcSubDir2/srcSub2.xoo + File srcSubDir2 = createDir(srcDir, "srcSubDir2", false); + writeFile(srcSubDir2, "srcSub2.xoo", "Sample xoo\ncontent"); + + AnalysisResult result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .put("sonar.inclusions", "src/srcSubDir/**/*") + .build()) + .execute(); + + assertAnalysedFiles(result, "src/srcSubDir/srcSub.xoo"); + } + + @Test + public void givenDirectoryWithoutReadPermissionUnderSourcesWhenAnalysedThenShouldFail() throws IOException { + // src/src.xoo + File srcDir = createDir(baseDir, "src", true); + writeFile(srcDir, "src.xoo", "Sample xoo 2\ncontent"); + + // src/srcSubDir/srcSub.xoo + File srcSubDir = createDir(srcDir, "srcSubDir", false); + writeFile(srcSubDir, "srcSub.xoo", "Sample xoo\ncontent"); + + AnalysisBuilder result = tester.newAnalysis() + .properties(builder + .put("sonar.sources", "src") + .build()); + + assertThatThrownBy(result::execute) + .isExactlyInstanceOf(IllegalStateException.class) + .hasMessageEndingWith(format("Failed to index files")); + } + + private static void assertAnalysedFiles(AnalysisResult result, String... files) { + assertThat(result.inputFiles().stream().map(InputFile::toString).collect(Collectors.toList())).contains(files); + } + + private File createDir(File parentDir, String name, boolean isReadable) { + File dir = new File(parentDir, name); + dir.mkdir(); + dir.setReadable(isReadable); + return dir; + } + private File writeFile(File parent, String name, String content) throws IOException { File file = new File(parent, name); FileUtils.write(file, content, StandardCharsets.UTF_8); -- 2.39.5