Skip to content

Commit

Permalink
cached location lookup data
Browse files Browse the repository at this point in the history
  • Loading branch information
nahsra committed Nov 17, 2024
1 parent d764dec commit 26b851c
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,15 @@ public Integer call() throws IOException {
log.debug("excluding paths: {}", pathExcludes);

// get all files that match
log.debug("Listing source directories");
List<SourceDirectory> sourceDirectories =
sourceDirectoryLister.listJavaSourceDirectories(List.of(projectDirectory));

log.debug("Listing files");
List<Path> filePaths = fileFinder.findFiles(projectPath, includesExcludes);

log.debug("Creating codemod regulator");

// get codemod includes/excludes
final CodemodRegulator regulator;
if (codemodIncludes != null && codemodExcludes != null) {
Expand All @@ -386,10 +391,13 @@ public Integer call() throws IOException {
}

// create the loader
log.debug("Loading input files");
CodeDirectory codeDirectory = new DefaultCodeDirectory(projectPath);
List<Path> sarifFiles = convertToPaths(sarifs);
List<Path> sonarIssuesJsonFiles = convertToPaths(sonarIssuesJsonFilePaths);
List<Path> sonarHotspotJsonFiles = convertToPaths(sonarHotspotsJsonFilePaths);

log.debug("Parsing SARIFs");
Map<String, List<RuleSarif>> pathSarifMap =
SarifParser.create().parseIntoMap(sarifFiles, codeDirectory);
List<ParameterArgument> codemodParameters =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.google.inject.AbstractModule;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** A module that only configures if it is responsible for a codemod that's being loaded. */
public abstract class CodemodCheckingAbstractModule extends AbstractModule {
Expand All @@ -19,10 +21,14 @@ protected CodemodCheckingAbstractModule(final List<Class<? extends CodeChanger>>
@Override
protected final void configure() {
if (shouldActivate) {
log.info("Configuring module {}", this.getClass().getSimpleName());
doConfigure();
log.info("Done configuration {}", this.getClass().getSimpleName());
}
}

/** Do the configuration that you would normally do in the configure method. */
protected abstract void doConfigure();

private static final Logger log = LoggerFactory.getLogger(CodemodCheckingAbstractModule.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import java.util.regex.Pattern;
import javax.inject.Inject;
import org.jetbrains.annotations.VisibleForTesting;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** This type is responsible for loading codemods and the surrounding subsystem. */
public final class CodemodLoader {
Expand All @@ -33,6 +35,8 @@ public CodemodLoader(
final Path defectDojoFindingsJsonFile,
final Path contrastVulnerabilitiesXmlFilePath) {

log.info("Loading providers");

// get all the providers ready for dependency injection & codemod instantiation
final List<CodemodProvider> providers =
ServiceLoader.load(CodemodProvider.class).stream()
Expand Down Expand Up @@ -106,6 +110,7 @@ public CodemodLoader(
wantsSarif.stream()
.flatMap(toolName -> ruleSarifByTool.getOrDefault(toolName, List.of()).stream())
.toList();
log.info("Loading modules from provider: {}", provider.getClass().getSimpleName());
final Set<AbstractModule> modules =
provider.getModules(
repositoryDir,
Expand All @@ -125,7 +130,9 @@ public CodemodLoader(
final List<CodemodIdPair> codemods = new ArrayList<>();

// validate and instantiate the codemods
log.info("Instantiating codemods");
final Injector injector = Guice.createInjector(allModules);
log.info("Codemods instantiated");
final Set<String> codemodIds = new HashSet<>();
for (final Class<? extends CodeChanger> type : orderedCodemodTypes) {
final Codemod codemodAnnotation = type.getAnnotation(Codemod.class);
Expand Down Expand Up @@ -164,6 +171,8 @@ static boolean isValidCodemodId(final String codemodId) {
return codemodIdPattern.matcher(codemodId).matches();
}

private static final Logger log = LoggerFactory.getLogger(CodemodLoader.class);

private static final Pattern codemodIdPattern =
Pattern.compile("^([A-Za-z0-9]+):(([A-Za-z0-9]+)/)+([A-Za-z0-9\\-\\.]+)$");
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.contrastsecurity.sarif.SarifSchema210;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.*;
Expand All @@ -17,10 +18,14 @@ final class DefaultSarifParser implements SarifParser {

private Optional<SarifSchema210> readSarifFile(final Path sarifFile) {
try {
return Optional.of(
new ObjectMapper().readValue(Files.newInputStream(sarifFile), SarifSchema210.class));
log.debug("Reading input file: {}", sarifFile);
InputStream stream = Files.newInputStream(sarifFile);
log.debug("Parsing to SARIF input files");
SarifSchema210 sarif = new ObjectMapper().readValue(stream, SarifSchema210.class);
log.debug("Parsed SARIF input files");
return Optional.of(sarif);
} catch (final IOException e) {
LOG.error("Problem deserializing SARIF file: {}", sarifFile, e);
log.error("Problem deserializing SARIF file: {}", sarifFile, e);
return Optional.empty();
}
}
Expand All @@ -33,14 +38,15 @@ private Optional<Map.Entry<String, RuleSarif>> tryToBuild(
final CodeDirectory codeDirectory,
final List<RuleSarifFactory> factories) {
for (final var factory : factories) {
log.debug("Building SARIF: {}", factory.getClass().getSimpleName());
final var maybeRuleSarif =
factory.build(toolName, rule.ruleId, rule.messageText, sarif, codeDirectory);
if (maybeRuleSarif.isPresent()) {
return Optional.of(Map.entry(toolName, maybeRuleSarif.get()));
}
}

LOG.info("Found SARIF from unsupported tool: {}", toolName);
log.info("Found SARIF from unsupported tool: {}", toolName);
return Optional.empty();
}

Expand All @@ -60,7 +66,7 @@ private RuleDescriptor extractRuleId(final Result result, final Run run) {
if (maybeRule.isPresent()) {
return maybeRule.get();
} else {
LOG.info("Could not find rule id for result.");
log.info("Could not find rule id for result.");
return null;
}
}
Expand All @@ -72,10 +78,12 @@ private Stream<Map.Entry<String, RuleSarif>> fromSarif(
final Run run, final SarifSchema210 sarif, final CodeDirectory codeDirectory) {
// driver name
final var toolName = run.getTool().getDriver().getName();
log.debug("Loading SARIF rule factories");
final List<RuleSarifFactory> factories =
ServiceLoader.load(RuleSarifFactory.class).stream()
.map(ServiceLoader.Provider::get)
.toList();
log.debug("Done loading SARIF rule factories");
final var runResults = run.getResults();
final var allResults =
runResults != null
Expand Down Expand Up @@ -105,16 +113,18 @@ public Map<String, List<RuleSarif>> parseIntoMap(
.flatMap(
sarif -> sarif.getRuns().stream().flatMap(run -> fromSarif(run, sarif, codeDirectory)))
.forEach(
p ->
map.merge(
p.getKey(),
new ArrayList<>(Collections.singletonList(p.getValue())),
(l1, l2) -> {
l1.add(l2.get(0));
return l1;
}));
p -> {
log.debug("Merging SARIF results");
map.merge(
p.getKey(),
new ArrayList<>(Collections.singletonList(p.getValue())),
(l1, l2) -> {
l1.add(l2.get(0));
return l1;
});
});
return map;
}

private static final Logger LOG = LoggerFactory.getLogger(DefaultSarifParser.class);
private static final Logger log = LoggerFactory.getLogger(DefaultSarifParser.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ public ExecutionStatus configure(final LoggerContext lc) {
ca.setContext(lc);
ca.setName(APPENDER_NAME);
PatternLayoutEncoder patternLayoutEncoder = new PatternLayoutEncoder();
patternLayoutEncoder.setPattern("%m%n");
// add the timestamp
// patternLayoutEncoder.setPattern("%m%n");
patternLayoutEncoder.setPattern("%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n");
patternLayoutEncoder.setContext(lc);
patternLayoutEncoder.setCharset(StandardCharsets.UTF_8);
patternLayoutEncoder.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ signing {
val signingPassword: String? by project
useInMemoryPgpKeys(signingKey, signingPassword)
}
// sign(extensions.getByType<PublishingExtension>().publications.getByName(publicationName))
sign(extensions.getByType<PublishingExtension>().publications.getByName(publicationName))
}

publishing {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,32 @@
package io.codemodder.providers.sarif.appscan;

import com.contrastsecurity.sarif.*;
import io.codemodder.CodeDirectory;
import io.codemodder.RuleSarif;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.util.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** A {@link RuleSarif} for AppScan results. */
final class AppScanRuleSarif implements RuleSarif {

private final SarifSchema210 sarif;
private final String messageText;
private final Map<Path, List<Result>> resultsCache;
private final List<String> locations;

/** A map of a AppScan SARIF "location" URIs mapped to their respective file paths. */
private final Map<Path, Set<Integer>> artifactLocationIndices;
private final AppScanSarifLocationData sarifLocationData;

/**
* Creates an {@link AppScanRuleSarif} that has already done the work of mapping AppScan SARIF
* locations, which are strange combinations of class name and file path, into predictable paths.
*/
AppScanRuleSarif(
final String messageText, final SarifSchema210 sarif, final CodeDirectory codeDirectory) {
final String messageText,
final SarifSchema210 sarif,
final AppScanSarifLocationData sarifLocationData) {
this.sarif = Objects.requireNonNull(sarif);
this.messageText = Objects.requireNonNull(messageText);
this.resultsCache = new HashMap<>();
this.locations =
sarif.getRuns().get(0).getArtifacts().stream()
.map(Artifact::getLocation)
.map(ArtifactLocation::getUri)
.map(u -> u.substring(8)) // trim the file:/// of all results
.toList();
Map<Path, Set<Integer>> artifactLocationIndicesMap = new HashMap<>();

for (int i = 0; i < locations.size(); i++) {
final Integer index = i;
String path = locations.get(i);
path = path.replace('\\', '/');
// we have a real but partial path, now we have to find it in the repository
Optional<Path> existingRealPath;
try {
existingRealPath = codeDirectory.findFilesWithTrailingPath(path);
} catch (IOException e) {
throw new UncheckedIOException(e);
}

// add to the map if we found a matching file
existingRealPath.ifPresent(
p -> artifactLocationIndicesMap.computeIfAbsent(p, k -> new HashSet<>()).add(index));
}
this.artifactLocationIndices = Map.copyOf(artifactLocationIndicesMap);
this.sarifLocationData = Objects.requireNonNull(sarifLocationData);
}

@Override
Expand All @@ -71,6 +45,9 @@ public List<Region> getRegionsFromResultsByRule(final Path path) {
*/
@Override
public List<Result> getResultsByLocationPath(final Path path) {

Map<Path, Set<Integer>> artifactLocationIndices =
sarifLocationData.getArtifactLocationIndices();
return resultsCache.computeIfAbsent(
path,
p ->
Expand Down Expand Up @@ -117,4 +94,6 @@ public String getRule() {
}

static final String toolName = "HCL AppScan Static Analyzer";

private static final Logger log = LoggerFactory.getLogger(AppScanRuleSarif.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@
import io.codemodder.CodeDirectory;
import io.codemodder.RuleSarif;
import io.codemodder.RuleSarifFactory;
import java.util.Optional;
import java.util.*;

/** A factory for building {@link AppScanRuleSarif}s. */
public final class AppScanRuleSarifFactory implements RuleSarifFactory {

/** A map of a AppScan SARIF "location" URIs mapped to their respective file paths. */
private final Map<SarifSchema210, AppScanSarifLocationData> sarifLocationDataCache;

public AppScanRuleSarifFactory() {
this.sarifLocationDataCache = new HashMap<>();
}

@Override
public Optional<RuleSarif> build(
final String toolName,
Expand All @@ -17,7 +24,12 @@ public Optional<RuleSarif> build(
final SarifSchema210 sarif,
final CodeDirectory codeDirectory) {
if (AppScanRuleSarif.toolName.equals(toolName)) {
return Optional.of(new AppScanRuleSarif(messageText, sarif, codeDirectory));
AppScanSarifLocationData sarifLocationData = sarifLocationDataCache.get(sarif);
if (sarifLocationData == null) {
sarifLocationData = new AppScanSarifLocationData(sarif, codeDirectory);
sarifLocationDataCache.put(sarif, sarifLocationData);
}
return Optional.of(new AppScanRuleSarif(messageText, sarif, sarifLocationData));
}
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package io.codemodder.providers.sarif.appscan;

import com.contrastsecurity.sarif.Artifact;
import com.contrastsecurity.sarif.ArtifactLocation;
import com.contrastsecurity.sarif.SarifSchema210;
import io.codemodder.CodeDirectory;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.util.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Holds the locations of the artifacts in the SARIF file. */
final class AppScanSarifLocationData {

private final Map<Path, Set<Integer>> artifactLocationIndices;

AppScanSarifLocationData(final SarifSchema210 sarif, final CodeDirectory codeDirectory) {
log.debug("Cleaning locations");
List<String> locations =
sarif.getRuns().get(0).getArtifacts().stream()
.map(Artifact::getLocation)
.map(ArtifactLocation::getUri)
.map(u -> u.substring(8)) // trim the file:/// of all results
.toList();

Map<Path, Set<Integer>> artifactLocationIndicesMap = new HashMap<>();

log.info("Calculating locations in project dir");
for (int i = 0; i < locations.size(); i++) {
final Integer index = i;
String path = locations.get(i);
path = path.replace('\\', '/');
// we have a real but partial path, now we have to find it in the repository
Optional<Path> existingRealPath;
try {
existingRealPath = codeDirectory.findFilesWithTrailingPath(path);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
// add to the map if we found a matching file
existingRealPath.ifPresent(
p -> artifactLocationIndicesMap.computeIfAbsent(p, k -> new HashSet<>()).add(index));
}
log.info("Done calculating locations");
this.artifactLocationIndices = Map.copyOf(artifactLocationIndicesMap);
}

Map<Path, Set<Integer>> getArtifactLocationIndices() {
return artifactLocationIndices;
}

private static final Logger log = LoggerFactory.getLogger(AppScanSarifLocationData.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
final class AppScanModuleTest {

@Codemod(
id = "appscan-test:java/finds-stuff",
id = "appscan:java/finds-stuff",
importance = Importance.LOW,
reviewGuidance = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW)
static class AppScanSarifTestCodemod implements CodeChanger {
Expand Down

0 comments on commit 26b851c

Please sign in to comment.