From 20eeed3b2168256d559917f0e6a8b80e9da04bcb Mon Sep 17 00:00:00 2001 From: gtoison Date: Sat, 14 May 2022 09:22:23 +0100 Subject: [PATCH] deps: drop deprecated RulesDefinitionXmlLoader - RulesDefinitionXmlLoader is deprecated and will be dropped in SQ 10 - RulesDefinitionXmlLoader does not populate the OWASP categories and can't be extended (see #392) - Loading/unloading the plugins dynamically from jar files leads to file leaks because SpotBugs does not close the classloaders (solves #128) - The Groovy scripts seem to need some fixes to work with Groovy 4 --- pom.xml | 41 +-- .../findbugs/FindbugsConfiguration.java | 76 +---- .../plugins/findbugs/FindbugsExecutor.java | 90 +++--- .../plugins/findbugs/FindbugsPlugin.java | 24 +- .../plugins/findbugs/FindbugsSensor.java | 2 +- .../findbugs/profiles/FindbugsProfile.java | 148 +++++++++- .../rules/FbContribRulesDefinition.java | 19 +- .../FindSecurityBugsJspRulesDefinition.java | 26 +- .../FindSecurityBugsRulesDefinition.java | 17 +- .../FindSecurityBugsScalaRulesDefinition.java | 16 +- .../plugins/findbugs/rules/FindbugsRules.java | 264 +++++++++++++++++ .../rules/FindbugsRulesDefinition.java | 30 +- .../rules/FindbugsRulesPluginsDefinition.java | 267 ++++++++++++++++++ .../FbContribRulesDefinitionTest.java | 3 +- .../FindSecurityBugsRulesDefinitionTest.java | 3 +- .../findbugs/FindbugsConfigurationTest.java | 39 --- .../findbugs/FindbugsExecutorTest.java | 44 ++- .../plugins/findbugs/FindbugsPluginTest.java | 2 +- .../findbugs/FindbugsRulesDefinitionTest.java | 15 +- .../plugins/findbugs/FindbugsSensorTest.java | 42 ++- .../sonar/plugins/findbugs/it/ScalaIT.java | 8 +- .../profiles/FindbugsContribProfileTest.java | 42 ++- .../profiles/FindbugsProfileTest.java | 9 +- .../profiles/FindbugsScalaProfileTest.java | 8 +- .../FindbugsSecurityAuditProfileTest.java | 8 +- .../FindbugsSecurityJspProfileTest.java | 14 +- .../FindbugsSecurityMinimalProfileTest.java | 8 +- .../plugins/findbugs/rule/FakeRuleFinder.java | 14 +- 28 files changed, 888 insertions(+), 391 deletions(-) create mode 100644 src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRules.java create mode 100644 src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesPluginsDefinition.java diff --git a/pom.xml b/pom.xml index f9319792..8231b607 100644 --- a/pom.xml +++ b/pom.xml @@ -84,19 +84,6 @@ - - - org.sonarsource.sslr-squid-bridge - sslr-squid-bridge - 2.7.0.377 - - - org.codehaus.sonar - sonar-plugin-api - - - - org.slf4j slf4j-api @@ -114,14 +101,12 @@ com.mebigfatguy.sb-contrib sb-contrib ${sbcontrib.version} - provided com.h3xstream.findsecbugs findsecbugs-plugin ${findsecbugs.version} - provided @@ -189,6 +174,13 @@ test + + org.picocontainer + picocontainer + 2.15 + test + + org.sonarsource.orchestrator sonar-orchestrator @@ -359,25 +351,6 @@ - - - org.apache.maven.plugins - maven-dependency-plugin - - - copy-dependencies - process-resources - - copy-dependencies - - - true - ${project.build.outputDirectory} - annotations,jsr305,sb-contrib,findsecbugs-plugin - - - - org.sonarsource.sonar-packaging-maven-plugin sonar-packaging-maven-plugin diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java index c3cdcc8c..5b689d39 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java @@ -27,7 +27,6 @@ import edu.umd.cs.findbugs.Project; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.io.StringWriter; import java.io.Writer; import java.nio.charset.StandardCharsets; @@ -46,7 +45,6 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; import org.sonar.api.PropertyType; -import org.sonar.api.Startable; import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.fs.FilePredicates; import org.sonar.api.batch.fs.FileSystem; @@ -71,7 +69,7 @@ import static java.lang.String.format; @ScannerSide -public class FindbugsConfiguration implements Startable { +public class FindbugsConfiguration { private static final Logger LOG = Loggers.get(FindbugsConfiguration.class); private static final Pattern JSP_FILE_NAME_PATTERN = Pattern.compile(".*_jsp[\\$0-9]*\\.class"); @@ -125,12 +123,6 @@ public void initializeFindbugsProject(Project findbugsProject) throws IOExceptio } } - copyLibs(); - if (annotationsLib != null) { - // Findbugs dependencies are packaged by Maven. They are not available during execution of unit tests. - findbugsProject.addAuxClasspathEntry(annotationsLib.getCanonicalPath()); - findbugsProject.addAuxClasspathEntry(jsr305Lib.getCanonicalPath()); - } findbugsProject.setCurrentWorkingDirectory(fileSystem.workDir()); } @@ -370,72 +362,6 @@ public boolean isAllowUncompiledCode() { return config.getBoolean(FindbugsConstants.ALLOW_UNCOMPILED_CODE).orElse(FindbugsConstants.ALLOW_UNCOMPILED_CODE_VALUE); } - private File jsr305Lib; - private File annotationsLib; - private File fbContrib; - private File findSecBugs; - - public void copyLibs() { - if (jsr305Lib == null) { - jsr305Lib = copyLib("/jsr305.jar"); - } - if (annotationsLib == null) { - annotationsLib = copyLib("/annotations.jar"); - } - if (fbContrib == null) { - fbContrib = copyLib("/sb-contrib.jar"); - } - if (findSecBugs == null) { - findSecBugs = copyLib("/findsecbugs-plugin.jar"); - } - } - - @Override - public void start() { - // do nothing - } - - /** - * Invoked by PicoContainer to remove temporary files. - */ - @SuppressWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE") - @Override - public void stop() { - if (jsr305Lib != null) { - jsr305Lib.delete(); - } - if (annotationsLib != null) { - annotationsLib.delete(); - } - if (fbContrib != null) { - fbContrib.delete(); - } - - if (findSecBugs != null) { - findSecBugs.delete(); - } - } - - private File copyLib(String name) { - try (InputStream input = getClass().getResourceAsStream(name)) { - File dir = new File(fileSystem.workDir(), "findbugs"); - FileUtils.forceMkdir(dir); - File target = new File(dir, name); - FileUtils.copyInputStreamToFile(input, target); - return target; - } catch (IOException e) { - throw new IllegalStateException("Fail to extract Findbugs dependency", e); - } - } - - public File getFbContribJar() { - return fbContrib; - } - - public File getFindSecBugsJar() { - return findSecBugs; - } - public static List getPropertyDefinitions() { String subCategory = "FindBugs"; return ImmutableList.of( diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java index ea47b505..e1ffc44f 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java @@ -19,11 +19,11 @@ */ package org.sonar.plugins.findbugs; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Throwables; -import com.google.common.collect.Lists; import edu.umd.cs.findbugs.BugCollection; import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugPattern; +import edu.umd.cs.findbugs.DetectorFactory; import edu.umd.cs.findbugs.DetectorFactoryCollection; import edu.umd.cs.findbugs.FindBugs; import edu.umd.cs.findbugs.FindBugs2; @@ -41,13 +41,17 @@ import org.slf4j.LoggerFactory; import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.fs.FileSystem; +import org.sonar.api.batch.rule.ActiveRules; import org.sonar.api.config.Configuration; +import org.sonar.api.rule.RuleKey; +import org.sonar.plugins.findbugs.rules.FindbugsRules; import java.io.File; import java.io.FileReader; import java.io.IOException; import java.io.OutputStream; import java.io.PrintStream; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.util.*; @@ -89,17 +93,8 @@ public FindbugsExecutor(FindbugsConfiguration configuration, FileSystem fs, Conf this.fs = fs; this.config = config; } - - @VisibleForTesting - Collection execute() { - return execute(true); - } - - public Collection execute(boolean useAllPlugin) { - return execute(useAllPlugin,useAllPlugin); - } - - public Collection execute(boolean useFbContrib, boolean useFindSecBugs) { + + public Collection execute(ActiveRules activeRules) { // We keep a handle on the current security manager because FB plays with it and we need to restore it before shutting down the executor // service SecurityManager currentSecurityManager = System.getSecurityManager(); @@ -111,7 +106,6 @@ public Collection execute(boolean useFbContrib, boolean useFindSecB Locale.setDefault(Locale.ENGLISH); OutputStream xmlOutput = null; - Collection customPlugins = null; ExecutorService executorService = Executors.newSingleThreadExecutor(); try (FindBugs2 engine = new FindBugs2(); Project project = new Project()) { configuration.initializeFindbugsProject(project); @@ -121,8 +115,8 @@ public Collection execute(boolean useFbContrib, boolean useFindSecB return new ArrayList<>(); } - customPlugins = loadFindbugsPlugins(useFbContrib,useFindSecBugs); - + loadFindbugsPlugins(); + disableUnnecessaryDetectors(project.getConfiguration(), activeRules); disableUpdateChecksOnEveryPlugin(); engine.setProject(project); @@ -189,7 +183,6 @@ public Collection execute(boolean useFbContrib, boolean useFindSecB } finally { // we set back the original security manager BEFORE shutting down the executor service, otherwise there's a problem with Java 5 System.setSecurityManager(currentSecurityManager); - resetCustomPluginList(customPlugins); executorService.shutdown(); IOUtils.closeQuietly(xmlOutput); Thread.currentThread().setContextClassLoader(initialClassLoader); @@ -242,39 +235,32 @@ public Object call() { } } - private Collection loadFindbugsPlugins(boolean useFbContrib,boolean useFindSecBugs) { - ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + public static Map loadFindbugsPlugins() { + ClassLoader contextClassLoader = FindbugsExecutor.class.getClassLoader(); - List pluginJarPathList = Lists.newArrayList(); + List pluginJarPathList = new ArrayList<>(); try { Enumeration urls = contextClassLoader.getResources("findbugs.xml"); while (urls.hasMoreElements()) { URL url = urls.nextElement(); pluginJarPathList.add(normalizeUrl(url)); } - //Add fb-contrib plugin. - if (useFbContrib && configuration.getFbContribJar() != null) { - // fb-contrib plugin is packaged by Maven. It is not available during execution of unit tests. - pluginJarPathList.add(configuration.getFbContribJar().getAbsolutePath()); - } - //Add find-sec-bugs plugin. (same as fb-contrib) - if (useFindSecBugs && configuration.getFindSecBugsJar() != null) { - pluginJarPathList.add(configuration.getFindSecBugsJar().getAbsolutePath()); - } } catch (IOException e) { throw new IllegalStateException(e); } catch (URISyntaxException e) { throw new IllegalStateException(e); } - List customPluginList = Lists.newArrayList(); + Map plugins = new HashMap<>(); for (String path : pluginJarPathList) { try { - Plugin plugin = Plugin.addCustomPlugin(new File(path).toURI(), contextClassLoader); - if (plugin != null) { - customPluginList.add(plugin); - LOG.info("Loading findbugs plugin: " + path); - } + URI uri = new File(path).toURI(); + Plugin plugin = Plugin.getAllPluginsMap().get(uri); + if (plugin == null) { + LOG.info("Loading findbugs plugin: " + path); + plugin = Plugin.addCustomPlugin(uri, contextClassLoader); + } + plugins.put(plugin.getPluginId(), plugin); } catch (PluginException e) { LOG.warn("Failed to load plugin for custom detector: " + path); LOG.debug("Cause of failure", e); @@ -287,7 +273,7 @@ private Collection loadFindbugsPlugins(boolean useFbContrib,boolean useF } } - return customPluginList; + return plugins; } private static String normalizeUrl(URL url) throws URISyntaxException { @@ -303,12 +289,34 @@ private static void disableUpdateChecksOnEveryPlugin() { } } - private static void resetCustomPluginList(Collection customPlugins) { - if (customPlugins != null) { - for (Plugin plugin : customPlugins) { - Plugin.removeCustomPlugin(plugin); + public static void disableUnnecessaryDetectors(UserPreferences userPreferences, ActiveRules activeRules) { + for (DetectorFactory detectorFactory : DetectorFactoryCollection.instance().getFactories()) { + boolean enabled = !detectorFactory.isReportingDetector() || detectorFactoryHasActiveRules(detectorFactory, activeRules); + + userPreferences.enableDetector(detectorFactory, enabled); + } + } + + private static boolean detectorFactoryHasActiveRules(DetectorFactory detectorFactory, ActiveRules activeRules) { + Collection repositories = FindbugsRules.repositoriesForPlugin(detectorFactory.getPlugin()); + + if (repositories.isEmpty()) { + LOG.warn("Detector {} is activated because it is not from a built-in plugin, cannot check if there are some active rules", detectorFactory); + return true; + } + + for (BugPattern bugPattern : detectorFactory.getReportedBugPatterns()) { + String bugPatternType = bugPattern.getType(); + + for (String repository : repositories) { + RuleKey ruleKey = RuleKey.of(repository, bugPatternType); + + if (activeRules.find(ruleKey) != null) { + return true; + } } } + + return false; } - } diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java index e919291c..cdbc4b43 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java @@ -21,23 +21,15 @@ import java.util.Arrays; import java.util.stream.Collectors; + import org.sonar.api.Plugin; import org.sonar.api.batch.fs.FilePredicate; import org.sonar.api.batch.fs.FilePredicates; import org.sonar.plugins.findbugs.language.Jsp; import org.sonar.plugins.findbugs.language.scala.Scala; -import org.sonar.plugins.findbugs.profiles.FindbugsContribProfile; import org.sonar.plugins.findbugs.profiles.FindbugsProfile; -import org.sonar.plugins.findbugs.profiles.FindbugsSecurityAuditProfile; -import org.sonar.plugins.findbugs.profiles.FindbugsSecurityJspProfile; -import org.sonar.plugins.findbugs.profiles.FindbugsSecurityMinimalProfile; -import org.sonar.plugins.findbugs.profiles.FindbugsSecurityScalaProfile; import org.sonar.plugins.findbugs.resource.ByteCodeResourceLocator; -import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition; -import org.sonar.plugins.findbugs.rules.FindSecurityBugsJspRulesDefinition; -import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition; -import org.sonar.plugins.findbugs.rules.FindSecurityBugsScalaRulesDefinition; -import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindbugsRulesPluginsDefinition; import org.sonar.plugins.java.Java; public class FindbugsPlugin implements Plugin { @@ -63,22 +55,12 @@ public void define(Context context) { context.addExtensions(Arrays.asList( FindbugsSensor.class, FindbugsProfileExporter.class, - FindbugsProfileImporter.class, FindbugsConfiguration.class, FindbugsExecutor.class, FindbugsProfile.class, - FindbugsContribProfile.class, - FindbugsSecurityAuditProfile.class, - FindbugsSecurityMinimalProfile.class, - FindbugsSecurityJspProfile.class, - FindbugsSecurityScalaProfile.class, - FindbugsRulesDefinition.class, - FbContribRulesDefinition.class, - FindSecurityBugsRulesDefinition.class, - FindSecurityBugsJspRulesDefinition.class, - FindSecurityBugsScalaRulesDefinition.class, + FindbugsRulesPluginsDefinition.class, ByteCodeResourceLocator.class)); } } diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java index d252eeac..6c5e5786 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java @@ -123,7 +123,7 @@ public void execute(SensorContext context) { return; } - Collection collection = executor.execute(hasActiveFbContribRules(), hasActiveFindSecBugsRules()); + Collection collection = executor.execute(context.activeRules()); try { diff --git a/src/main/java/org/sonar/plugins/findbugs/profiles/FindbugsProfile.java b/src/main/java/org/sonar/plugins/findbugs/profiles/FindbugsProfile.java index c3d83572..4b22a96e 100644 --- a/src/main/java/org/sonar/plugins/findbugs/profiles/FindbugsProfile.java +++ b/src/main/java/org/sonar/plugins/findbugs/profiles/FindbugsProfile.java @@ -19,30 +19,156 @@ */ package org.sonar.plugins.findbugs.profiles; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.EXCLUDED_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.FINDBUGS_JSP_RULES; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.FINDBUGS_PATTERNS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.INFORMATIONAL_PATTERNS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.SECURITY_JSP_RULES; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.SECURITY_SCALA_RULES; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.union; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; + +import org.sonar.api.rules.Rule; +import org.sonar.api.rules.RuleFinder; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; +import org.sonar.plugins.findbugs.FindbugsExecutor; +import org.sonar.plugins.findbugs.language.Jsp; +import org.sonar.plugins.findbugs.language.scala.Scala; +import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindSecurityBugsJspRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindSecurityBugsScalaRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindbugsRules; +import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; import org.sonar.plugins.java.Java; -import java.io.InputStreamReader; -import java.io.Reader; +import edu.umd.cs.findbugs.BugPattern; +import edu.umd.cs.findbugs.Plugin; +import edu.umd.cs.findbugs.PluginLoader; public class FindbugsProfile implements BuiltInQualityProfilesDefinition { public static final String FINDBUGS_PROFILE_NAME = "FindBugs"; - private final FindbugsProfileImporter importer; + public static final String FB_CONTRIB_PROFILE_NAME = "FindBugs + FB-Contrib"; + + /** + * Security rules including informational rules. This profile is intend for in + * depth security code review. + */ + public static final String FINDBUGS_SECURITY_AUDIT_PROFILE_NAME = "FindBugs Security Audit"; + + /** + * Security rules with only the issue that require immediate analysis. It is + * intend for periodic scan that will trigger a moderate number of false + * positive. + */ + public static final String FINDBUGS_SECURITY_MINIMAL_PROFILE_NAME = "FindBugs Security Minimal"; + public static final String FINDBUGS_SECURITY_JSP_PROFILE_NAME = "FindBugs Security JSP"; + public static final String FINDBUGS_SECURITY_SCALA_PROFILE_NAME = "FindBugs Security Scala"; + + private RuleFinder ruleFinder; - public FindbugsProfile(FindbugsProfileImporter importer) { - this.importer = importer; + public FindbugsProfile(RuleFinder ruleFinder) { + this.ruleFinder = ruleFinder; } @Override public void define(Context context) { - Reader findbugsProfile = new InputStreamReader(this.getClass().getResourceAsStream( - "/org/sonar/plugins/findbugs/profile-findbugs-only.xml")); - NewBuiltInQualityProfile profile = context.createBuiltInQualityProfile(FINDBUGS_PROFILE_NAME, Java.KEY); - importer.importProfile(findbugsProfile, profile); + try { + NewBuiltInQualityProfile findbugsProfile = context.createBuiltInQualityProfile(FINDBUGS_PROFILE_NAME, Java.KEY); + NewBuiltInQualityProfile findbugsContribProfile = context.createBuiltInQualityProfile(FB_CONTRIB_PROFILE_NAME, + Java.KEY); + NewBuiltInQualityProfile findbugsSecurityAuditProfile = context + .createBuiltInQualityProfile(FINDBUGS_SECURITY_AUDIT_PROFILE_NAME, Java.KEY); + NewBuiltInQualityProfile findbugsSecurityMinimalProfile = context + .createBuiltInQualityProfile(FINDBUGS_SECURITY_MINIMAL_PROFILE_NAME, Java.KEY); + NewBuiltInQualityProfile findbugsSecurityJspProfile = context + .createBuiltInQualityProfile(FINDBUGS_SECURITY_JSP_PROFILE_NAME, Jsp.KEY); + NewBuiltInQualityProfile findbugsSecurityScalaProfile = context + .createBuiltInQualityProfile(FINDBUGS_SECURITY_SCALA_PROFILE_NAME, Scala.KEY); + + Map plugins = FindbugsExecutor.loadFindbugsPlugins(); + Plugin corePlugin = PluginLoader.getCorePluginLoader().getPlugin(); + Plugin fbContribPlugin = plugins.get(FindbugsRules.PLUGIN_ID_FINDBUGS_CONTRIB); + Plugin findsecbugsPlugin = plugins.get(FindbugsRules.PLUGIN_ID_FINDSECBUGS); + + // FindBugs profile + activateRules(findbugsProfile, FindbugsRulesDefinition.REPOSITORY_KEY, getAllPatternsFromPlugin(corePlugin), + Collections.emptyList(), union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES)); + + // FindBugs + FB Contrib profile + activateRules(findbugsContribProfile, FindbugsRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(corePlugin), Collections.emptyList(), + union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES)); + activateRules(findbugsContribProfile, FbContribRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(fbContribPlugin), Collections.emptyList(), + union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES)); + + // FindSecBugs Audit profile + activateRules(findbugsSecurityAuditProfile, FindSecurityBugsRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(findsecbugsPlugin), Collections.emptyList(), + union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES, EXCLUDED_BUGS)); + activateRules(findbugsSecurityAuditProfile, FindbugsRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(corePlugin), FINDBUGS_PATTERNS, Collections.emptyList()); + + // FindSecBugs Minimal profile + activateRules(findbugsSecurityMinimalProfile, FindSecurityBugsRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(findsecbugsPlugin), Collections.emptyList(), + union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES, EXCLUDED_BUGS, INFORMATIONAL_PATTERNS)); + activateRules(findbugsSecurityMinimalProfile, FindbugsRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(corePlugin), FINDBUGS_PATTERNS, Collections.emptyList()); - profile.done(); + // FindSecBugs JPS profile + activateRules(findbugsSecurityJspProfile, FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(findsecbugsPlugin), SECURITY_JSP_RULES, Collections.emptyList()); + activateRules(findbugsSecurityJspProfile, FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(corePlugin), FINDBUGS_JSP_RULES, Collections.emptyList()); + + // FindSecBugs Scala profile + activateRules(findbugsSecurityScalaProfile, FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY, + getAllPatternsFromPlugin(findsecbugsPlugin), SECURITY_SCALA_RULES, Collections.emptyList()); + + findbugsProfile.done(); + findbugsContribProfile.done(); + findbugsSecurityAuditProfile.done(); + findbugsSecurityMinimalProfile.done(); + findbugsSecurityJspProfile.done(); + findbugsSecurityScalaProfile.done(); + } catch (Exception e) { + throw new RuntimeException("Error defining quality profiles", e); + } + } + + public void activateRules(NewBuiltInQualityProfile profile, String repoKey, Collection bugPatterns, + Collection includedBugs, Collection excludedBugs) { + for (BugPattern bugPattern : bugPatterns) { + String type = bugPattern.getType(); + + if ((includedBugs.isEmpty() || includedBugs.contains(type)) && !excludedBugs.contains(type)) { + Rule rule = ruleFinder.findByKey(repoKey, type); + + if (rule != null && !Rule.STATUS_REMOVED.equals(rule.getStatus())) { + profile.activateRule(repoKey, type); + } + } + } } + private Collection getAllPatternsFromPlugin(Plugin plugin) { + Collection bugPatterns = new HashSet<>(); + + for (BugPattern bugPattern : plugin.getBugPatterns()) { + String category = bugPattern.getCategory(); + + if (!"EXPERIMENTAL".equals(category) && !"NOISE".equals(category)) { + bugPatterns.add(bugPattern); + } + } + + return bugPatterns; + } } diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java index 1bab518b..9c475b8d 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java @@ -19,26 +19,13 @@ */ package org.sonar.plugins.findbugs.rules; -import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionXmlLoader; -import org.sonar.plugins.java.Java; - -public class FbContribRulesDefinition implements RulesDefinition { +public class FbContribRulesDefinition { public static final String REPOSITORY_KEY = "fb-contrib"; public static final String REPOSITORY_NAME = "FindBugs Contrib"; public static final int RULE_COUNT = 307; public static final int DEACTIVED_RULE_COUNT = 0; - - @Override - public void define(Context context) { - NewRepository repository = context - .createRepository(REPOSITORY_KEY, Java.KEY) - .setName(REPOSITORY_NAME); - - RulesDefinitionXmlLoader ruleLoader = new RulesDefinitionXmlLoader(); - ruleLoader.load(repository, FbContribRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-fbcontrib.xml"), "UTF-8"); - repository.done(); + + private FbContribRulesDefinition() { } - } diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsJspRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsJspRulesDefinition.java index a87b08b1..e0566cf3 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsJspRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsJspRulesDefinition.java @@ -19,35 +19,23 @@ */ package org.sonar.plugins.findbugs.rules; -import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionXmlLoader; -import org.sonar.plugins.findbugs.language.Jsp; +import org.sonar.api.server.rule.RulesDefinition.NewRepository; +import org.sonar.api.server.rule.RulesDefinition.NewRule; /** * This RulesDefinition build a separate repository from the FindSecurityBugsRulesDefinition to allow a separate ruleset * for JSP language. * @see FindSecurityBugsRulesDefinition */ -public class FindSecurityBugsJspRulesDefinition implements RulesDefinition { +public class FindSecurityBugsJspRulesDefinition { public static final String REPOSITORY_KEY = "findsecbugs-jsp"; public static final String REPOSITORY_JSP_NAME = "Find Security Bugs (JSP)"; - - @Override - public void define(Context context) { - NewRepository repositoryJsp = context - .createRepository(REPOSITORY_KEY, Jsp.KEY) - .setName(REPOSITORY_JSP_NAME); - - RulesDefinitionXmlLoader ruleLoaderJsp = new RulesDefinitionXmlLoader(); - ruleLoaderJsp.load(repositoryJsp, FindSecurityBugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-jsp.xml"), "UTF-8"); - - addDeprecatedRuleKeys(repositoryJsp); - - repositoryJsp.done(); + + private FindSecurityBugsJspRulesDefinition() { } - - public void addDeprecatedRuleKeys(NewRepository repositoryJsp) { + + public static void addDeprecatedRuleKeys(NewRepository repositoryJsp) { String[] rulesMovedFromFindSecurityBugs = new String[] { "XSS_JSP_PRINT", "XSS_REQUEST_PARAMETER_TO_JSP_WRITER" diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsRulesDefinition.java index 4fc33d0d..09565b81 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsRulesDefinition.java @@ -19,25 +19,12 @@ */ package org.sonar.plugins.findbugs.rules; -import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionXmlLoader; -import org.sonar.plugins.java.Java; - -public final class FindSecurityBugsRulesDefinition implements RulesDefinition { +public final class FindSecurityBugsRulesDefinition { public static final String REPOSITORY_KEY = "findsecbugs"; public static final String REPOSITORY_NAME = "Find Security Bugs"; public static final int RULE_COUNT = 126; - @Override - public void define(Context context) { - NewRepository repository = context - .createRepository(REPOSITORY_KEY, Java.KEY) - .setName(REPOSITORY_NAME); - - RulesDefinitionXmlLoader ruleLoader = new RulesDefinitionXmlLoader(); - ruleLoader.load(repository, FindSecurityBugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-findsecbugs.xml"), "UTF-8"); - repository.done(); - + private FindSecurityBugsRulesDefinition() { } } diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsScalaRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsScalaRulesDefinition.java index bec561a0..e445013c 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsScalaRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindSecurityBugsScalaRulesDefinition.java @@ -1,22 +1,10 @@ package org.sonar.plugins.findbugs.rules; -import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionXmlLoader; -import org.sonar.plugins.findbugs.language.scala.Scala; - -public class FindSecurityBugsScalaRulesDefinition implements RulesDefinition { +public class FindSecurityBugsScalaRulesDefinition { public static final String REPOSITORY_KEY = "findsecbugs-scala"; public static final String REPOSITORY_SCALA_NAME = "Find Security Bugs (Scala)"; public static final int RULE_COUNT = 9; - @Override - public void define(Context context) { - NewRepository repositoryJsp = context - .createRepository(REPOSITORY_KEY, Scala.KEY) - .setName(REPOSITORY_SCALA_NAME); - - RulesDefinitionXmlLoader ruleLoaderJsp = new RulesDefinitionXmlLoader(); - ruleLoaderJsp.load(repositoryJsp, FindSecurityBugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-scala.xml"), "UTF-8"); - repositoryJsp.done(); + private FindSecurityBugsScalaRulesDefinition() { } } diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRules.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRules.java new file mode 100644 index 00000000..e2fd6535 --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRules.java @@ -0,0 +1,264 @@ +/* + * SonarQube SpotBugs Plugin + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.findbugs.rules; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import edu.umd.cs.findbugs.Plugin; + +/** + * @author gtoison + * + */ +public class FindbugsRules { + private static final Logger LOGGER = LoggerFactory.getLogger(FindbugsRules.class); + + public static final String PLUGIN_ID_FINDBUGS_CORE = "edu.umd.cs.findbugs.plugins.core"; + public static final String PLUGIN_ID_FINDSECBUGS = "com.h3xstream.findsecbugs"; + public static final String PLUGIN_ID_FINDBUGS_CONTRIB = "com.mebigfatguy.fbcontrib"; + + //Includes all the bugs that are bundle with FindBugs by default + public static final List FINDBUGS_PATTERNS = Arrays.asList( + "XSS_REQUEST_PARAMETER_TO_SEND_ERROR", + "XSS_REQUEST_PARAMETER_TO_SERVLET_WRITER", + "HRS_REQUEST_PARAMETER_TO_HTTP_HEADER", + "HRS_REQUEST_PARAMETER_TO_COOKIE", + "DMI_CONSTANT_DB_PASSWORD", + "DMI_EMPTY_DB_PASSWORD", + "SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE", + "SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING" + ); + + public static final List CRYPTO_BUGS = Arrays.asList( + "WEAK_TRUST_MANAGER", + "WEAK_HOSTNAME_VERIFIER", + //"WEAK_MESSAGE_DIGEST", //Deprecated + "WEAK_MESSAGE_DIGEST_MD5", + "WEAK_MESSAGE_DIGEST_SHA1", + "CUSTOM_MESSAGE_DIGEST", + "HAZELCAST_SYMMETRIC_ENCRYPTION", + "NULL_CIPHER", + "UNENCRYPTED_SOCKET", + "DES_USAGE", + "RSA_NO_PADDING", + "RSA_KEY_SIZE", + "BLOWFISH_KEY_SIZE", + "STATIC_IV", + "ECB_MODE", + "PADDING_ORACLE", + "CIPHER_INTEGRITY", + "SSL_CONTEXT", + "UNENCRYPTED_SERVER_SOCKET", + "DEFAULT_HTTP_CLIENT", //TLS 1.2 vs SSL + "INSECURE_SMTP_SSL", + "UNSAFE_HASH_EQUALS", + "TDES_USAGE" + ); + + public static final List CRITICAL_BUGS = Arrays.asList( //RCE or powerful function + "COMMAND_INJECTION", + "XXE_SAXPARSER", + "XXE_XMLREADER", + "XXE_DOCUMENT", + "XXE_XMLSTREAMREADER", + "XXE_XPATH", + "XXE_XSLT_TRANSFORM_FACTORY", + "XXE_DTD_TRANSFORM_FACTORY", + "SQL_INJECTION_HIBERNATE", + "SQL_INJECTION_JDO", + "SQL_INJECTION_JPA", + "SQL_INJECTION", + "SQL_INJECTION_TURBINE", + "SQL_INJECTION_ANDROID", + "OGNL_INJECTION", + "LDAP_INJECTION", + "XPATH_INJECTION", + "XML_DECODER", + "SCALA_SENSITIVE_DATA_EXPOSURE", + "SCALA_PLAY_SSRF", + "SCALA_XSS_TWIRL", + "SCALA_XSS_MVC_API", + "SCALA_PATH_TRAVERSAL_IN", + "SCALA_COMMAND_INJECTION", + "SCALA_SQL_INJECTION_SLICK", + "SCALA_SQL_INJECTION_ANORM", + "PREDICTABLE_RANDOM_SCALA", + "SCRIPT_ENGINE_INJECTION", + "SPEL_INJECTION", + "SQL_INJECTION_SPRING_JDBC", + "SQL_INJECTION_JDBC", + "EL_INJECTION", + "SEAM_LOG_INJECTION", + "OBJECT_DESERIALIZATION", + "MALICIOUS_XSLT", + "JACKSON_UNSAFE_DESERIALIZATION", + "TEMPLATE_INJECTION_VELOCITY", + "TEMPLATE_INJECTION_FREEMARKER", + "AWS_QUERY_INJECTION", + "LDAP_ENTRY_POISONING", + "BEAN_PROPERTY_INJECTION", + "RPC_ENABLED_EXTENSIONS", + "GROOVY_SHELL", + "TEMPLATE_INJECTION_PEBBLE", + "SQL_INJECTION_VERTX", + "IMPROPER_UNICODE", + "SAML_IGNORE_COMMENTS", + "DANGEROUS_PERMISSION_COMBINATION"); + + public static final List MAJOR_BUGS = Arrays.asList( + "PREDICTABLE_RANDOM", + "PATH_TRAVERSAL_IN", + "PATH_TRAVERSAL_OUT", + "REDOS", + "BAD_HEXA_CONVERSION", + "HARD_CODE_PASSWORD", + "HARD_CODE_KEY", + "XSS_REQUEST_WRAPPER", + "UNVALIDATED_REDIRECT", + "ANDROID_EXTERNAL_FILE_ACCESS", + "ANDROID_WORLD_WRITABLE", + "INSECURE_COOKIE", + "HTTPONLY_COOKIE", + "TRUST_BOUNDARY_VIOLATION", + "XSS_SERVLET", + "SPRING_CSRF_PROTECTION_DISABLED", + "SPRING_CSRF_UNRESTRICTED_REQUEST_MAPPING", + "COOKIE_PERSISTENT", + "PLAY_UNVALIDATED_REDIRECT", + "SPRING_UNVALIDATED_REDIRECT", + "PERMISSIVE_CORS", + "LDAP_ANONYMOUS", + "URL_REWRITING", + "STRUTS_FILE_DISCLOSURE", + "SPRING_FILE_DISCLOSURE", + "HTTP_PARAMETER_POLLUTION", + "SMTP_HEADER_INJECTION", + "REQUESTDISPATCHER_FILE_DISCLOSURE", + "URLCONNECTION_SSRF_FD", + "SPRING_ENTITY_LEAK", + "WICKET_XSS1", + "ENTITY_LEAK", + "ENTITY_MASS_ASSIGNMENT", + "OVERLY_PERMISSIVE_FILE_PERMISSION", + "MODIFICATION_AFTER_VALIDATION", + "NORMALIZATION_AFTER_VALIDATION"); + + public static final List INFORMATIONAL_PATTERNS = Arrays.asList( + "SERVLET_PARAMETER", + "SERVLET_CONTENT_TYPE", + "SERVLET_SERVER_NAME", + "SERVLET_SESSION_ID", + "SERVLET_QUERY_STRING", + "SERVLET_HEADER", + "SERVLET_HEADER_REFERER", + "SERVLET_HEADER_USER_AGENT", + "COOKIE_USAGE", + "WEAK_FILENAMEUTILS", + "JAXWS_ENDPOINT", + "JAXRS_ENDPOINT", + "TAPESTRY_ENDPOINT", + "WICKET_ENDPOINT", + "FILE_UPLOAD_FILENAME", + "STRUTS1_ENDPOINT", + "STRUTS2_ENDPOINT", + "SPRING_ENDPOINT", + "HTTP_RESPONSE_SPLITTING", + "CRLF_INJECTION_LOGS", + "EXTERNAL_CONFIG_CONTROL", + "STRUTS_FORM_VALIDATION", + "ESAPI_ENCRYPTOR", + "ANDROID_BROADCAST", + "ANDROID_GEOLOCATION", + "ANDROID_WEB_VIEW_JAVASCRIPT", + "ANDROID_WEB_VIEW_JAVASCRIPT_INTERFACE", + "FORMAT_STRING_MANIPULATION", + "DESERIALIZATION_GADGET", //Prone to false positive.. therefore only in audit profile + "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE"); + + + + public static final List MAJOR_JSP_BUGS = Arrays.asList("XSS_REQUEST_PARAMETER_TO_JSP_WRITER", "XSS_JSP_PRINT", "JSP_JSTL_OUT"); + + // RCE from JSP specific functions (taglibs) + public static final List CRITICAL_JSP_BUGS = Arrays.asList("JSP_INCLUDE","JSP_SPRING_EVAL","JSP_XSLT"); + + public static final List CRITICAL_SCALA_BUGS = Arrays.asList( + "SCALA_SENSITIVE_DATA_EXPOSURE", + "SCALA_PLAY_SSRF", + "SCALA_XSS_TWIRL", + "SCALA_XSS_MVC_API", + "SCALA_PATH_TRAVERSAL_IN", + "SCALA_COMMAND_INJECTION", + "SCALA_SQL_INJECTION_SLICK", + "SCALA_SQL_INJECTION_ANORM", + "PREDICTABLE_RANDOM_SCALA"); + + /** + * Bug patterns for JSP code from the SpotBugs core plugin (not from FindSecBugs) + */ + public static final Set FINDBUGS_JSP_RULES = Collections.singleton("XSS_REQUEST_PARAMETER_TO_JSP_WRITER"); + + public static final Set SECURITY_JSP_RULES = union(MAJOR_JSP_BUGS, CRITICAL_JSP_BUGS); + public static final Set SECURITY_SCALA_RULES = union(CRITICAL_SCALA_BUGS); + public static final Set EXCLUDED_BUGS = Collections.singleton("CUSTOM_INJECTION"); + + private FindbugsRules() { + } + + /** + * @param plugin + * @return a collection of repository keys or an empty collection in case the plugin is unknown + */ + public static Collection repositoriesForPlugin(Plugin plugin) { + switch (plugin.getPluginId()) { + case PLUGIN_ID_FINDBUGS_CORE: + return Arrays.asList( + FindbugsRulesDefinition.REPOSITORY_KEY, + FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY); + case PLUGIN_ID_FINDSECBUGS: + return Arrays.asList( + FindSecurityBugsRulesDefinition.REPOSITORY_KEY, + FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, + FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY); + case PLUGIN_ID_FINDBUGS_CONTRIB: + return Collections.singletonList(FbContribRulesDefinition.REPOSITORY_KEY); + default: + LOGGER.warn("Unknown plugin: {}", plugin.getPluginId()); + return Collections.emptyList(); + } + } + + @SafeVarargs + public static Set union(Collection ... collections) { + Set union = new HashSet<>(); + + for (Collection c : collections) { + union.addAll(c); + } + + return union; + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesDefinition.java index 0c239ac8..1f63792b 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesDefinition.java @@ -19,35 +19,13 @@ */ package org.sonar.plugins.findbugs.rules; -import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionXmlLoader; -import org.sonar.plugins.java.Java; -import org.sonar.squidbridge.rules.SqaleXmlLoader; - -public final class FindbugsRulesDefinition implements RulesDefinition { +public final class FindbugsRulesDefinition { public static final String REPOSITORY_KEY = "findbugs"; public static final String REPOSITORY_NAME = "FindBugs"; public static final int RULE_COUNT = 461; - public static final int DEACTIVED_RULE_COUNT = 6; - - @Override - public void define(Context context) { - NewRepository repository = context - .createRepository(REPOSITORY_KEY, Java.KEY) - .setName(REPOSITORY_NAME); - - - RulesDefinitionXmlLoader ruleLoaderJsp = new RulesDefinitionXmlLoader(); - ruleLoaderJsp.load(repository, FindSecurityBugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-findbugs.xml"), "UTF-8"); - SqaleXmlLoader.load(repository, "/com/sonar/sqale/findbugs-model.xml"); - repository.done(); - -// RulesDefinitionXmlLoader ruleLoader = new RulesDefinitionXmlLoader(); -// ruleLoader.load(repository, FindbugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules.xml"), "UTF-8"); -// ExternalDescriptionLoader.loadHtmlDescriptions(repository, "/org/sonar/l10n/findbugs/rules/findbugs"); -// SqaleXmlLoader.load(repository, "/com/sonar/sqale/findbugs-model.xml"); -// repository.done(); + public static final int DEACTIVED_RULE_COUNT = 4; + + private FindbugsRulesDefinition() { } - } diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesPluginsDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesPluginsDefinition.java new file mode 100644 index 00000000..46c63364 --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesPluginsDefinition.java @@ -0,0 +1,267 @@ +/* + * SonarQube SpotBugs Plugin + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.findbugs.rules; + +import static org.sonar.plugins.findbugs.rules.FindbugsRules.CRITICAL_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.CRITICAL_JSP_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.CRITICAL_SCALA_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.CRYPTO_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.EXCLUDED_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.INFORMATIONAL_PATTERNS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.MAJOR_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.MAJOR_JSP_BUGS; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.SECURITY_JSP_RULES; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.SECURITY_SCALA_RULES; +import static org.sonar.plugins.findbugs.rules.FindbugsRules.union; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.sonar.api.rule.RuleStatus; +import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.plugins.findbugs.FindbugsExecutor; +import org.sonar.plugins.findbugs.language.Jsp; +import org.sonar.plugins.findbugs.language.scala.Scala; +import org.sonar.plugins.java.Java; + +import edu.umd.cs.findbugs.BugPattern; +import edu.umd.cs.findbugs.Plugin; +import edu.umd.cs.findbugs.PluginLoader; + +/** + * Builds the rules definitions based on the plugins bug patterns + * + * @author gtoison + */ +public final class FindbugsRulesPluginsDefinition implements RulesDefinition { + private static final Logger LOG = LoggerFactory.getLogger(FindbugsRulesPluginsDefinition.class); + + @Override + public void define(Context context) { + try { + NewRepository repository = context + .createRepository(FindbugsRulesDefinition.REPOSITORY_KEY, Java.KEY) + .setName(FindbugsRulesDefinition.REPOSITORY_NAME); + NewRepository fbContribRepository = context + .createRepository(FbContribRulesDefinition.REPOSITORY_KEY, Java.KEY) + .setName(FbContribRulesDefinition.REPOSITORY_NAME); + NewRepository findsecbugsRepository = context + .createRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY, Java.KEY) + .setName(FindSecurityBugsRulesDefinition.REPOSITORY_NAME); + NewRepository findsecbugsJspRepository = context + .createRepository(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, Jsp.KEY) + .setName(FindSecurityBugsJspRulesDefinition.REPOSITORY_JSP_NAME); + NewRepository findsecbugsScalaRepository = context + .createRepository(FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY, Scala.KEY) + .setName(FindSecurityBugsScalaRulesDefinition.REPOSITORY_SCALA_NAME); + + // Rules marked as deprecated (and their equivalent) because there are redundant with native SonarQube rules + Map deprecatedFbContribRules = new HashMap<>(); + deprecatedFbContribRules.put("AFBR_ABNORMAL_FINALLY_BLOCK_RETURN", "java:S1143"); + deprecatedFbContribRules.put("AIOB_ARRAY_STORE_TO_NULL_REFERENCE", "java:S2259"); + deprecatedFbContribRules.put("FII_USE_METHOD_REFERENCE", "java:S1612"); + deprecatedFbContribRules.put("SPP_TOSTRING_ON_STRING", "java:S1858"); + deprecatedFbContribRules.put("SPP_USE_BIGDECIMAL_STRING_CTOR", "java:S2111"); + deprecatedFbContribRules.put("SPP_USELESS_TERNARY", "java:S1125"); + deprecatedFbContribRules.put("USBR_UNNECESSARY_STORE_BEFORE_RETURN", "java:S1488"); + + Map plugins = FindbugsExecutor.loadFindbugsPlugins(); + Plugin corePlugin = PluginLoader.getCorePluginLoader().getPlugin(); + Plugin fbContribPlugin = plugins.get(FindbugsRules.PLUGIN_ID_FINDBUGS_CONTRIB); + Plugin findsecbugsPlugin = plugins.get(FindbugsRules.PLUGIN_ID_FINDSECBUGS); + + initializeRulesRepository(repository, corePlugin, Collections.emptyList(), union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES), Collections.emptyMap()); + initializeRulesRepository(fbContribRepository, fbContribPlugin, Collections.emptyList(), Collections.emptyList(), deprecatedFbContribRules); + initializeRulesRepository(findsecbugsRepository, findsecbugsPlugin, Collections.emptyList(), union(SECURITY_JSP_RULES, SECURITY_SCALA_RULES, EXCLUDED_BUGS), Collections.emptyMap()); + initializeRulesRepository(findsecbugsJspRepository, corePlugin, SECURITY_JSP_RULES, Collections.emptyList(), Collections.emptyMap()); + initializeRulesRepository(findsecbugsJspRepository, findsecbugsPlugin, SECURITY_JSP_RULES, Collections.emptyList(), Collections.emptyMap()); + initializeRulesRepository(findsecbugsScalaRepository, findsecbugsPlugin, SECURITY_SCALA_RULES, Collections.emptyList(), Collections.emptyMap()); + + FindSecurityBugsJspRulesDefinition.addDeprecatedRuleKeys(findsecbugsJspRepository); + + repository.done(); + fbContribRepository.done(); + findsecbugsRepository.done(); + findsecbugsJspRepository.done(); + findsecbugsScalaRepository.done(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + /** + * @param repository The {@link NewRepository} where we will create the rules + * @param plugin The {@link Plugin} we want to load the {@link BugPattern} from + * @param includedBugs The ids of the {@link BugPattern} we want to include or an empty collection to include all of them + * @param excludedBugs The ids of the excluded {@link BugPattern} + * @param deprecatedRules The ids of the deprecated {@link BugPattern} and their replacement + */ + public void initializeRulesRepository(NewRepository repository, Plugin plugin, Collection includedBugs, + Collection excludedBugs, Map deprecatedRules) { + for (BugPattern bugPattern : plugin.getBugPatterns()) { + String type = bugPattern.getType(); + + String category = bugPattern.getCategory(); + + if(category.equals("NOISE") || Arrays.asList("TESTING", "TESTING1", "TESTING2", "TESTING3", "UNKNOWN").contains(type)) { + continue; + } + + if(category.equals("MT_CORRECTNESS")) { + category = "MULTI-THREADING"; + } + + String htmlDescription = bugPattern.getDetailText(); + String severity = getSonarPriority(type, category, htmlDescription); + String name = capitalize(category.toLowerCase()).replace("_"," ") + " - " + bugPattern.getShortDescription(); + boolean deprecated = bugPattern.isDeprecated(); + String deprecationReplacement = deprecatedRules.get(type); + + RuleStatus ruleStatus = RuleStatus.READY; + + if (deprecationReplacement != null) { + htmlDescription = htmlDescription.trim() + "\n

Deprecated

\n

This rule is deprecated; use {rule:" + deprecationReplacement + "} instead.

"; + ruleStatus = RuleStatus.DEPRECATED; + } else if (deprecated) { + htmlDescription = htmlDescription.trim() + "\n

Deprecated

\n

This rule is deprecated

"; + ruleStatus = RuleStatus.DEPRECATED; + } + + List tags = new ArrayList<>(); + + //OWASP TOP 10 2013 + if (htmlDescription.toLowerCase().contains("injection") || htmlDescription.contains("A1-Injection")) { + tags.add("owasp-a1"); + tags.add("injection"); + } + if (htmlDescription.contains("A2-Broken_Authentication_and_Session_Management")) { + tags.add("owasp-a2"); + } + if (type.contains("XSS") || htmlDescription.contains("A3-Cross-Site_Scripting")) { + tags.add("owasp-a3"); + } + if (htmlDescription.contains("A4-Insecure_Direct_Object_References") || htmlDescription.contains("Path_Traversal")) { + tags.add("owasp-a4"); + } + if (htmlDescription.contains("A5-Security_Misconfiguration")) { + tags.add("owasp-a5"); + } + if (type.equals("HARD_CODE_PASSWORD") || + CRYPTO_BUGS.contains(type) || + htmlDescription.contains("A6-Sensitive_Data_Exposure")) { + tags.add("owasp-a6"); + tags.add("cryptography"); + } + if (htmlDescription.contains("A7-Missing_Function_Level_Access_Control")) { + tags.add("owasp-a7"); + } + if (htmlDescription.toLowerCase().contains("A8-Cross-Site_Request_Forgery")) { + tags.add("owasp-a8"); + } + if (htmlDescription.toLowerCase().contains("A9-Using_Components_with_Known_Vulnerabilities")) { + tags.add("owasp-a9"); + } + if (htmlDescription.toLowerCase().contains("A10-Unvalidated_Redirects_and_Forwards")) { + tags.add("owasp-a10"); + } + + //Misc tags + + if (htmlDescription.toLowerCase().contains("wasc")) { + tags.add("wasc"); + } + if (htmlDescription.toLowerCase().contains("cwe")) { + tags.add("cwe"); + } + if (bugPattern.getShortDescription().toLowerCase().contains("android")) { + tags.add("android"); + } + if (type.contains("JSP")) { + tags.add("jsp"); + } + + //Category related + tags.add(category.toLowerCase().replace("_","-")); + + if(Arrays.asList("PERFORMANCE","CORRECTNESS","MULTI-THREADING").contains(category)) { + tags.add("bug"); + } + + if((includedBugs.isEmpty() || includedBugs.contains(type)) && !excludedBugs.contains(type)) { + repository + .createRule(type) + .setInternalKey(type) + .setSeverity(severity) + .setName(name) + .setHtmlDescription(htmlDescription) + .setStatus(ruleStatus) + .setTags(tags.toArray(String[]::new)); + } + } + } + + public static String capitalize(String s) { + return s.substring(0, 1).toUpperCase() + s.substring(1); + } + + public String getSonarPriority(String type, String category, String description) { + String priority = getFsbPriorityFromType(type,category); + if(priority != null) { + return priority; + } + + //Findbugs critical base on the type or message + if(type.contains("IMPOSSIBLE")) { + return "CRITICAL"; + } + + Pattern willResultInExceptionAtRuntimePattern = Pattern.compile("[\\S\\s]*will result in [\\w]+Exception at runtime[\\S\\s]*"); + Pattern willAlwaysThrowExceptionPattern = Pattern.compile("[\\S\\s]*will always throw a [\\w]+Exception[\\S\\s]*"); + + if(willResultInExceptionAtRuntimePattern.matcher(description).matches() || willAlwaysThrowExceptionPattern.matcher(description).matches()) { + return "CRITICAL"; + } + + //Findbugs general + if(Arrays.asList("CORRECTNESS", "PERFORMANCE", "SECURITY","MULTI-THREADING","BAD_PRACTICE").contains(category)) return "MAJOR"; + if(Arrays.asList("STYLE", "MALICIOUS_CODE", "I18N","EXPERIMENTAL").contains(category)) return "INFO"; + + LOG.warn("Unknown priority for "+type+" ("+category+")"); + return "INFO"; + } + + private String getFsbPriorityFromType(String type, String category) { + if (CRITICAL_BUGS.contains(type) || CRITICAL_JSP_BUGS.contains(type) || CRITICAL_SCALA_BUGS.contains(type)) return "CRITICAL"; + if (MAJOR_BUGS.contains(type) || CRYPTO_BUGS.contains(type) || MAJOR_JSP_BUGS.contains(type)) return "MAJOR"; + if (INFORMATIONAL_PATTERNS.contains(type)) return "INFO"; + + if(category.equals("SECURITY")) { + return "MAJOR"; + } + + return null; + } +} diff --git a/src/test/java/org/sonar/plugins/findbugs/FbContribRulesDefinitionTest.java b/src/test/java/org/sonar/plugins/findbugs/FbContribRulesDefinitionTest.java index 17feeab7..ad7e26e7 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FbContribRulesDefinitionTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FbContribRulesDefinitionTest.java @@ -23,6 +23,7 @@ import org.sonar.api.server.rule.RulesDefinition; import org.sonar.api.server.rule.RulesDefinition.Rule; import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindbugsRulesPluginsDefinition; import org.sonar.plugins.java.Java; import java.util.List; @@ -33,7 +34,7 @@ class FbContribRulesDefinitionTest { @Test void test() { - FbContribRulesDefinition definition = new FbContribRulesDefinition(); + FindbugsRulesPluginsDefinition definition = new FindbugsRulesPluginsDefinition(); RulesDefinition.Context context = new RulesDefinition.Context(); definition.define(context); RulesDefinition.Repository repository = context.repository(FbContribRulesDefinition.REPOSITORY_KEY); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java b/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java index 53ff20f1..f17fbaf9 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java @@ -23,6 +23,7 @@ import org.sonar.api.server.rule.RulesDefinition; import org.sonar.api.server.rule.RulesDefinition.Rule; import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindbugsRulesPluginsDefinition; import org.sonar.plugins.java.Java; import java.util.List; @@ -33,7 +34,7 @@ class FindSecurityBugsRulesDefinitionTest { @Test void testLoadRepositoryFromXml() { - FindSecurityBugsRulesDefinition definition = new FindSecurityBugsRulesDefinition(); + FindbugsRulesPluginsDefinition definition = new FindbugsRulesPluginsDefinition(); RulesDefinition.Context context = new RulesDefinition.Context(); definition.define(context); RulesDefinition.Repository repository = context.repository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java index a1457c38..9192e8ba 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java @@ -135,7 +135,6 @@ void should_set_class_files() throws IOException { conf.initializeFindbugsProject(findbugsProject); assertThat(findbugsProject.getFileList()).containsOnly(file.getCanonicalPath()); - conf.stop(); } } @@ -147,46 +146,8 @@ void should_set_class_path() throws IOException { conf.initializeFindbugsProject(findbugsProject); assertThat(findbugsProject.getAuxClasspathEntryList()).contains(classpath.getCanonicalPath()); - conf.stop(); } } - - @Test - void should_copy_lib_in_working_dir() throws IOException { - String jsr305 = "findbugs/jsr305.jar"; - String annotations = "findbugs/annotations.jar"; - - // stop at start - conf.stop(); - assertThat(new File(fs.workDir(), jsr305)).doesNotExist(); - assertThat(new File(fs.workDir(), annotations)).doesNotExist(); - - conf.copyLibs(); - assertThat(new File(fs.workDir(), jsr305)).isFile(); - assertThat(new File(fs.workDir(), annotations)).isFile(); - - // copy again - conf.copyLibs(); - assertThat(new File(fs.workDir(), jsr305)).isFile(); - assertThat(new File(fs.workDir(), annotations)).isFile(); - - conf.stop(); - assertThat(new File(fs.workDir(), jsr305)).doesNotExist(); - assertThat(new File(fs.workDir(), annotations)).doesNotExist(); - - } - - @Test - void should_get_fbcontrib() throws IOException { - conf.copyLibs(); - assertThat(conf.getFbContribJar()).isFile(); - } - - @Test - void should_get_findSecBugs() throws IOException { - conf.copyLibs(); - assertThat(conf.getFindSecBugsJar()).isFile(); - } @Test public void should_get_only_analyze_filter() { diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java index 07694b9b..a78ae6d4 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java @@ -21,7 +21,14 @@ import com.google.common.collect.Lists; +import edu.umd.cs.findbugs.DetectorFactory; +import edu.umd.cs.findbugs.DetectorFactoryCollection; +import edu.umd.cs.findbugs.Plugin; import edu.umd.cs.findbugs.Project; +import edu.umd.cs.findbugs.config.UserPreferences; +import edu.umd.cs.findbugs.detect.DumbMethods; +import edu.umd.cs.findbugs.detect.FindFinalizeInvocations; +import edu.umd.cs.findbugs.detect.TrainFieldStoreTypes; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeEach; @@ -31,13 +38,18 @@ import org.sonar.api.batch.fs.FilePredicates; import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.rule.ActiveRule; +import org.sonar.api.batch.rule.ActiveRules; import org.sonar.api.config.Configuration; +import org.sonar.api.rule.RuleKey; import org.sonar.plugins.findbugs.configuration.SimpleConfiguration; +import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; import java.io.File; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; +import java.util.Map; import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; @@ -56,6 +68,8 @@ class FindbugsExecutorTest { FilePredicates predicatesEmpty; Configuration configEmpty; + + ActiveRules activeRules; @BeforeEach public void setUp() { @@ -68,6 +82,8 @@ public void setUp() { configEmpty = mock(Configuration.class); when(configEmpty.getStringArray(any())).thenReturn(new String[0]); when(configEmpty.get(any())).thenReturn(Optional.of("")); + + activeRules = mock(ActiveRules.class); } @Test @@ -77,7 +93,7 @@ void canGenerateXMLReport() throws Exception { File reportFile = new File(temporaryFolder, "findbugs-report.xml"); when(conf.getTargetXMLReport()).thenReturn(reportFile); - new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); + new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(activeRules); assertThat(reportFile).exists(); String report = FileUtils.readFileToString(reportFile, StandardCharsets.UTF_8); @@ -96,7 +112,7 @@ void canGenerateXMLReportWithCustomConfidence() throws Exception { when(conf.getTargetXMLReport()).thenReturn(reportFile); when(conf.getConfidenceLevel()).thenReturn("low"); - new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); + new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(activeRules); assertThat(reportFile).exists(); String report = FileUtils.readFileToString(reportFile, StandardCharsets.UTF_8); @@ -114,7 +130,7 @@ public void shouldTerminateAfterTimeout() throws Exception { FindbugsExecutor executor = new FindbugsExecutor(conf, fsEmpty, configEmpty); assertThrows(IllegalStateException.class, () -> { - executor.execute(); + executor.execute(activeRules); }); } @@ -127,7 +143,7 @@ public void shoulFailIfNoCompiledClasses() throws Exception { FindbugsExecutor executor = new FindbugsExecutor(conf, fsEmpty, configEmpty); assertThrows(IllegalStateException.class, () -> { - executor.execute(); + executor.execute(activeRules); }); } @@ -148,4 +164,24 @@ private FindbugsConfiguration mockConf() throws Exception { return conf; } + @Test + void disableUnnecessaryDetectors() { + Map plugins = FindbugsExecutor.loadFindbugsPlugins(); + + when(activeRules.find(RuleKey.of(FindbugsRulesDefinition.REPOSITORY_KEY, "DM_INVALID_MIN_MAX"))).thenReturn(mock(ActiveRule.class)); + + UserPreferences userPreferences = UserPreferences.createDefaultUserPreferences(); + FindbugsExecutor.disableUnnecessaryDetectors(userPreferences, activeRules); + + DetectorFactory dumbMethods = DetectorFactoryCollection.instance().getFactoryByClassName(DumbMethods.class.getName()); + DetectorFactory findFinalize = DetectorFactoryCollection.instance().getFactoryByClassName(FindFinalizeInvocations.class.getName()); + DetectorFactory trainFieldStoreTypes = DetectorFactoryCollection.instance().getFactoryByClassName(TrainFieldStoreTypes.class.getName()); + + // DM_INVALID_MIN_MAX is reported by DumbMethods so the detector should be enabled + assertThat(userPreferences.isDetectorEnabled(dumbMethods)).withFailMessage("DumbMethods should be enabled").isTrue(); + // No active rule reported by FindFinalizeInvocations so it should be disabled + assertThat(userPreferences.isDetectorEnabled(findFinalize)).withFailMessage("FindFinalizeInvocations should be enabled").isFalse(); + // TrainFieldStoreTypes is not a reporting detector so it should always be enabled + assertThat(userPreferences.isDetectorEnabled(trainFieldStoreTypes)).withFailMessage("TrainFieldStoreTypes should be enabled").isTrue(); + } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java index b56ea42c..d8fc9b65 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java @@ -38,6 +38,6 @@ void testGetExtensions() { FindbugsPlugin plugin = new FindbugsPlugin(); plugin.define(ctx); - assertEquals(24, ctx.getExtensions().size(), "extension count"); + assertEquals(14, ctx.getExtensions().size(), "extension count"); } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java index 3bb104a8..ec038181 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java @@ -19,28 +19,31 @@ */ package org.sonar.plugins.findbugs; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.sonar.api.rule.RuleStatus; import org.sonar.api.server.rule.RulesDefinition; import org.sonar.api.server.rule.RulesDefinition.Rule; import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; +import org.sonar.plugins.findbugs.rules.FindbugsRulesPluginsDefinition; import org.sonar.plugins.java.Java; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - class FindbugsRulesDefinitionTest { /** * The SpotBugs rules repository */ private RulesDefinition.Repository repository; + private RulesDefinition.Context context; @BeforeEach public void setupRepository() { - FindbugsRulesDefinition definition = new FindbugsRulesDefinition(); - RulesDefinition.Context context = new RulesDefinition.Context(); + FindbugsRulesPluginsDefinition definition = new FindbugsRulesPluginsDefinition(); + + context = new RulesDefinition.Context(); definition.define(context); repository = context.repository(FindbugsRulesDefinition.REPOSITORY_KEY); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java index 658ff0ed..d5967942 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java @@ -43,6 +43,7 @@ import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.TextRange; +import org.sonar.api.batch.rule.ActiveRules; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; import org.sonar.api.batch.sensor.issue.NewIssue; @@ -69,12 +70,14 @@ class FindbugsSensorTest extends FindbugsTests { private ByteCodeResourceLocator byteCodeResourceLocator; private MutablePicoContainer pico; private SensorContext sensorContext; + private ActiveRules activeRules; private FindbugsExecutor executor; private JavaResourceLocator javaResourceLocator; @BeforeEach public void setUp() throws IOException { sensorContext = mock(SensorContext.class); + activeRules = mock(ActiveRules.class); byteCodeResourceLocator = mock(ByteCodeResourceLocator.class); executor = mock(FindbugsExecutor.class); javaResourceLocator = mockJavaResourceLocator(); @@ -117,6 +120,7 @@ public void setUp() throws IOException { //-- when(sensorContext.newIssue()).thenReturn(newIssue); + when(sensorContext.activeRules()).thenReturn(activeRules); pico.addComponent(executor); pico.addComponent(javaResourceLocator); @@ -134,7 +138,7 @@ void should_execute_findbugs() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 6, true); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -142,7 +146,7 @@ void should_execute_findbugs() throws Exception { FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); sensor.execute(sensorContext); - verify(executor).execute(false, false); + verify(executor).execute(activeRules); verify(sensorContext, times(1)).newIssue(); } @@ -151,7 +155,7 @@ void should_not_add_issue_if_resource_not_found() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 13, false); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); when(javaResourceLocator.findResourceByClassName(anyString())).thenReturn(null); when(fs.inputFiles(any(FilePredicate.class))).thenReturn(new ArrayList()); @@ -161,7 +165,7 @@ void should_not_add_issue_if_resource_not_found() throws Exception { FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor).execute(false, false); + verify(executor).execute(activeRules); verify(sensorContext, never()).newIssue(); } @@ -171,7 +175,7 @@ void should_execute_findbugs_even_if_only_fbcontrib() throws Exception { BugInstance bugInstance = getBugInstance("ISB_INEFFICIENT_STRING_BUFFERING", 49, true); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(true, false)).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -180,7 +184,7 @@ void should_execute_findbugs_even_if_only_fbcontrib() throws Exception { FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor).execute(true, false); + verify(executor).execute(activeRules); verify(sensorContext, times(1)).newIssue(); } @@ -189,7 +193,7 @@ void should_execute_findbugs_even_if_only_findsecbug() throws Exception { BugInstance bugInstance = getBugInstance("PREDICTABLE_RANDOM", 0, true); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, true)).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -198,7 +202,7 @@ void should_execute_findbugs_even_if_only_findsecbug() throws Exception { FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor).execute(false, true); + verify(executor).execute(activeRules); verify(sensorContext, times(1)).newIssue(); } @@ -207,7 +211,7 @@ void should_execute_findbugs_but_not_find_violation() throws Exception { BugInstance bugInstance = getBugInstance("THIS_RULE_DOES_NOT_EXIST", 107, true); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -216,7 +220,7 @@ void should_execute_findbugs_but_not_find_violation() throws Exception { FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor).execute(false, false); + verify(executor).execute(activeRules); verify(sensorContext, never()).newIssue(); } @@ -230,7 +234,7 @@ void should_not_execute_findbugs_if_no_active() throws Exception { FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor, never()).execute(false, false); + verify(executor, never()).execute(activeRules); verify(sensorContext, never()).newIssue(); } @@ -251,7 +255,7 @@ void should_not_execute_findbugs_if_only_jsp_rules_and_no_jsp_file() throws Exce FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor, never()).execute(false, false); + verify(executor, never()).execute(activeRules); verify(sensorContext, never()).newIssue(); } @@ -270,7 +274,7 @@ void should_execute_findbugs_if_only_jsp_rules_and_some_jsp_files() throws Excep FindbugsSensor analyser = pico.getComponent(FindbugsSensor.class); analyser.execute(sensorContext); - verify(executor).execute(false, true); + verify(executor).execute(activeRules); verify(sensorContext, never()).newIssue(); } @@ -296,16 +300,6 @@ private BugInstance getBugInstance(String name, int line, boolean mockFindSource return bugInstance; } - @Test - void should_not_execute_if_no_compiled_class_available() throws Exception { - when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Collections.emptyList()); - pico.addComponent(FakeActiveRules.createWithOnlyFindbugsRules()); - FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); - sensor.execute(sensorContext); - - verify(executor, never()).execute(); - } - @Test void shouldIgnoreNotActiveViolations() throws Exception { BugInstance bugInstance = new BugInstance("UNKNOWN", 2); @@ -314,7 +308,7 @@ void shouldIgnoreNotActiveViolations() throws Exception { ClassAnnotation classAnnotation = new ClassAnnotation(className, sourceFile); bugInstance.add(classAnnotation); Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute()).thenReturn(collection); + when(executor.execute(activeRules)).thenReturn(collection); pico.addComponent(FakeActiveRules.createWithOnlyFindbugsRules()); FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); diff --git a/src/test/java/org/sonar/plugins/findbugs/it/ScalaIT.java b/src/test/java/org/sonar/plugins/findbugs/it/ScalaIT.java index 65b74e85..4ef3deaf 100644 --- a/src/test/java/org/sonar/plugins/findbugs/it/ScalaIT.java +++ b/src/test/java/org/sonar/plugins/findbugs/it/ScalaIT.java @@ -19,15 +19,15 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.List; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.sonar.plugins.findbugs.profiles.FindbugsSecurityScalaProfile; +import org.sonar.plugins.findbugs.profiles.FindbugsProfile; import org.sonarqube.ws.Issues.Issue; import org.sonarqube.ws.client.issues.IssuesService; -import java.util.List; - import com.sonar.orchestrator.Orchestrator; import com.sonar.orchestrator.build.MavenBuild; @@ -39,7 +39,7 @@ class ScalaIT { @BeforeEach public void setupProfile() { FindbugsTestSuite.setupProjectAndProfile(PROJECT_KEY, "Scala Integration Tests", "IT", "java"); - FindbugsTestSuite.setupProfile(PROJECT_KEY, FindbugsSecurityScalaProfile.FINDBUGS_SECURITY_SCALA_PROFILE_NAME, "scala"); + FindbugsTestSuite.setupProfile(PROJECT_KEY, FindbugsProfile.FINDBUGS_SECURITY_SCALA_PROFILE_NAME, "scala"); } @AfterEach diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsContribProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsContribProfileTest.java index 5c9877ba..38efd28c 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsContribProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsContribProfileTest.java @@ -2,11 +2,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.api.rules.RuleFinder; +import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInActiveRule; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInQualityProfile; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition; import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; @@ -15,6 +16,9 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.List; +import java.util.stream.Collectors; + class FindbugsContribProfileTest { @RegisterExtension @@ -22,16 +26,46 @@ class FindbugsContribProfileTest { @Test void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsContribProfile findbugsProfile = new FindbugsContribProfile(importer); + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); - BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsContribProfile.FB_CONTRIB_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsProfile.FB_CONTRIB_PROFILE_NAME); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FindbugsRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(FindbugsRulesDefinition.RULE_COUNT); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FbContribRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(FbContribRulesDefinition.RULE_COUNT); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); FindbugsProfileTest.assertHasOnlyRulesForLanguage(profile.rules(), Java.KEY); } + + @Test + void coreRulesAreFindBugsProfile() { + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); + Context context = new Context(); + findbugsProfile.define(context); + + BuiltInQualityProfile fbContribQualityProfile = context.profile(Java.KEY, FindbugsProfile.FB_CONTRIB_PROFILE_NAME); + BuiltInQualityProfile findbugsQualityProfile = context.profile(Java.KEY, FindbugsProfile.FINDBUGS_PROFILE_NAME); + + List findbugsRulesInFbContribProfile = fbContribQualityProfile + .rules() + .stream() + .filter(r -> r.repoKey().equals(FindbugsRulesDefinition.REPOSITORY_KEY)) + .map(BuiltInActiveRule::ruleKey) + .sorted() + .collect(Collectors.toList()); + + List findbugsRules = findbugsQualityProfile + .rules() + .stream() + .filter(r -> r.repoKey().equals(FindbugsRulesDefinition.REPOSITORY_KEY)) + .map(BuiltInActiveRule::ruleKey) + .sorted() + .collect(Collectors.toList()); + + assertThat(findbugsRulesInFbContribProfile).containsExactlyElementsOf(findbugsRules); + assertThat(findbugsRulesInFbContribProfile).hasSize(FindbugsRulesDefinition.RULE_COUNT); + } } diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsProfileTest.java index 613cd305..3e1b9b8b 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsProfileTest.java @@ -21,12 +21,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.api.rules.RuleFinder; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInActiveRule; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInQualityProfile; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.language.Jsp; import org.sonar.plugins.findbugs.language.scala.Scala; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; @@ -48,15 +48,16 @@ class FindbugsProfileTest { public LogTester logTester = new JupiterLogTester(); @Test - void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsProfile findbugsProfile = new FindbugsProfile(importer); + void findbugsProfile() { + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsProfile.FINDBUGS_PROFILE_NAME); assertThat(profile.rules()).hasSize(FindbugsRulesDefinition.RULE_COUNT); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FindbugsRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(FindbugsRulesDefinition.RULE_COUNT); + assertThat(profile.rules().stream().filter(r -> !r.repoKey().equals(FindbugsRulesDefinition.REPOSITORY_KEY)).count()).isZero(); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); FindbugsProfileTest.assertHasOnlyRulesForLanguage(profile.rules(), Java.KEY); diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsScalaProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsScalaProfileTest.java index c8d5ba8d..73b057f2 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsScalaProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsScalaProfileTest.java @@ -21,11 +21,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.api.rules.RuleFinder; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInQualityProfile; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.language.scala.Scala; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; import org.sonar.plugins.findbugs.rules.FindSecurityBugsScalaRulesDefinition; @@ -38,12 +38,12 @@ class FindbugsScalaProfileTest { @Test void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsSecurityScalaProfile findbugsProfile = new FindbugsSecurityScalaProfile(importer); + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); - BuiltInQualityProfile profile = context.profile(Scala.KEY, FindbugsSecurityScalaProfile.FINDBUGS_SECURITY_SCALA_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Scala.KEY, FindbugsProfile.FINDBUGS_SECURITY_SCALA_PROFILE_NAME); assertThat(profile.rules()).hasSize(FindSecurityBugsScalaRulesDefinition.RULE_COUNT); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(FindSecurityBugsScalaRulesDefinition.RULE_COUNT); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityAuditProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityAuditProfileTest.java index a2c4decb..2a7aae44 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityAuditProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityAuditProfileTest.java @@ -21,11 +21,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.api.rules.RuleFinder; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInQualityProfile; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition; import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; @@ -41,13 +41,13 @@ class FindbugsSecurityAuditProfileTest { @Test void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsSecurityAuditProfile findbugsProfile = new FindbugsSecurityAuditProfile(importer); + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); // The standard FindBugs include only 9. Fb-Contrib and FindSecurityBugs include other rules - BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsSecurityAuditProfile.FINDBUGS_SECURITY_AUDIT_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsProfile.FINDBUGS_SECURITY_AUDIT_PROFILE_NAME); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); // FSB rules must be added to FsbClassifier.groovy otherwise new rules metadata are not added in rules-findsecbugs.xml assertThat(logTester.getLogs(LoggerLevel.WARN)).isNull(); diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityJspProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityJspProfileTest.java index 562e50ba..0c2886e3 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityJspProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityJspProfileTest.java @@ -26,7 +26,6 @@ import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.language.Jsp; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; import org.sonar.plugins.findbugs.rules.FindSecurityBugsJspRulesDefinition; @@ -42,13 +41,13 @@ class FindbugsSecurityJspProfileTest { @Test void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsSecurityJspProfile findbugsProfile = new FindbugsSecurityJspProfile(importer); + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); //There are 6 rules that are JSP specific (the other findbugs rules can also be found in JSP files) - BuiltInQualityProfile profile = context.profile(Jsp.KEY, FindbugsSecurityJspProfile.FINDBUGS_SECURITY_JSP_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Jsp.KEY, FindbugsProfile.FINDBUGS_SECURITY_JSP_PROFILE_NAME); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); assertThat(logTester.getLogs(LoggerLevel.WARN)).isNull(); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(6); @@ -64,14 +63,13 @@ void disabledRuleMustNotBeActivated() { // Mark a rule as removed org.sonar.api.rules.Rule rule = ruleFinder.findByKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, "XSS_JSP_PRINT"); rule.setStatus(org.sonar.api.rules.Rule.STATUS_REMOVED); - - FindbugsProfileImporter importer = new FindbugsProfileImporter(ruleFinder); - FindbugsSecurityJspProfile findbugsProfile = new FindbugsSecurityJspProfile(importer); + + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); //There should be 5 rules left since we removed one - BuiltInQualityProfile profile = context.profile(Jsp.KEY, FindbugsSecurityJspProfile.FINDBUGS_SECURITY_JSP_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Jsp.KEY, FindbugsProfile.FINDBUGS_SECURITY_JSP_PROFILE_NAME); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); assertThat(logTester.getLogs(LoggerLevel.WARN)).isNull(); assertThat(profile.rules().stream().filter(r -> r.repoKey().equals(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY)).count()).isEqualTo(5); diff --git a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityMinimalProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityMinimalProfileTest.java index 69c3afd2..da08debe 100644 --- a/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityMinimalProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/profiles/FindbugsSecurityMinimalProfileTest.java @@ -21,11 +21,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.api.rules.RuleFinder; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInQualityProfile; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.Context; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.plugins.findbugs.FindbugsProfileImporter; import org.sonar.plugins.findbugs.rule.FakeRuleFinder; import org.sonar.plugins.findbugs.rules.FindSecurityBugsRulesDefinition; import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; @@ -41,12 +41,12 @@ class FindbugsSecurityMinimalProfileTest { @Test void shouldCreateProfile() { - FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); - FindbugsSecurityMinimalProfile findbugsProfile = new FindbugsSecurityMinimalProfile(importer); + RuleFinder ruleFinder = FakeRuleFinder.createWithAllRules(); + FindbugsProfile findbugsProfile = new FindbugsProfile(ruleFinder); Context context = new Context(); findbugsProfile.define(context); - BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsSecurityMinimalProfile.FINDBUGS_SECURITY_AUDIT_PROFILE_NAME); + BuiltInQualityProfile profile = context.profile(Java.KEY, FindbugsProfile.FINDBUGS_SECURITY_MINIMAL_PROFILE_NAME); assertThat(logTester.getLogs(LoggerLevel.ERROR)).isNull(); // FSB rules must be added to FsbClassifier.groovy otherwise new rules metadata are not added in rules-findsecbugs.xml assertThat(logTester.getLogs(LoggerLevel.WARN)).isNull(); diff --git a/src/test/java/org/sonar/plugins/findbugs/rule/FakeRuleFinder.java b/src/test/java/org/sonar/plugins/findbugs/rule/FakeRuleFinder.java index ca8aa28c..309ce291 100644 --- a/src/test/java/org/sonar/plugins/findbugs/rule/FakeRuleFinder.java +++ b/src/test/java/org/sonar/plugins/findbugs/rule/FakeRuleFinder.java @@ -51,39 +51,33 @@ private static RuleFinder create(boolean findbugs, boolean fbContrib, boolean fi RuleFinder ruleFinder = mock(RuleFinder.class); RulesDefinition.Context context = new RulesDefinition.Context(); List allRules = new ArrayList<>(); + + RulesDefinition rulesDefinition = new FindbugsRulesPluginsDefinition(); + rulesDefinition.define(context); + if (findbugs) { - RulesDefinition rulesDefinition = new FindbugsRulesDefinition(); - rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FindbugsRulesDefinition.REPOSITORY_KEY); allRules.addAll(convert(context.repository(FindbugsRulesDefinition.REPOSITORY_KEY).rules())); } if (fbContrib) { - RulesDefinition rulesDefinition = new FbContribRulesDefinition(); - rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FbContribRulesDefinition.REPOSITORY_KEY); allRules.addAll(convert(context.repository(FbContribRulesDefinition.REPOSITORY_KEY).rules())); } if (findSecBug) { - RulesDefinition rulesDefinition = new FindSecurityBugsRulesDefinition(); - rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FindSecurityBugsRulesDefinition.REPOSITORY_KEY); allRules.addAll(convert(context.repository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY).rules())); } if (findSecBugJsp) { - RulesDefinition rulesDefinition = new FindSecurityBugsJspRulesDefinition(); - rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY); allRules.addAll(convert(context.repository(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY).rules())); } if (findSecBugScala) { - RulesDefinition rulesDefinition = new FindSecurityBugsScalaRulesDefinition(); - rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY); allRules.addAll(convert(context.repository(FindSecurityBugsScalaRulesDefinition.REPOSITORY_KEY).rules())); }