Skip to content

Commit

Permalink
Merge pull request #780 from mjdetullio/fix/issue-766
Browse files Browse the repository at this point in the history 
Fix #766: Handle relative report paths with backticks outside basedir.
  • Loading branch information
@guwirth
guwirth committed Feb 20, 2016
2 parents 7cf1808 + 1962987 commit 7395724
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 44 deletions.
37 changes: 20 additions & 17 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@
import java.util.List;
import java.util.ArrayList;
import java.util.Set;
import javax.annotation.Nullable;
import com.google.common.base.Function;
import com.google.common.collect.Lists;
import org.apache.commons.io.FilenameUtils;
import org.apache.tools.ant.DirectoryScanner;
import org.sonar.api.batch.Sensor;
Expand Down Expand Up @@ -152,24 +149,30 @@ public static List<File> getReports(Settings settings, final File moduleBaseDir,

List<File> reports = new ArrayList<>();

List<String> includes = Arrays.asList(settings.getStringArray(reportPathPropertyKey));
if (!includes.isEmpty()) {
includes = Lists.transform(includes, new Function<String, String>() {
@Override
public String apply(@Nullable String path) {
if (path == null){
return null;
}
path = FilenameUtils.normalize(path);
if (new File(path).isAbsolute()) {
return path;
}
return FilenameUtils.normalize(moduleBaseDir.getAbsolutePath() + File.separator + path);
List<String> reportPaths = Arrays.asList(settings.getStringArray(reportPathPropertyKey));
if (!reportPaths.isEmpty()) {
List<String> includes = new ArrayList<>();
for (String reportPath : reportPaths) {
// Normalization can return null if path is null, is invalid, or is a path with back-ticks outside known directory structure
String normalizedPath = FilenameUtils.normalize(reportPath);
if (normalizedPath != null && new File(normalizedPath).isAbsolute()) {
includes.add(normalizedPath);
continue;
}
});

// Prefix with absolute module base dir, attempt normalization again -- can still get null here
normalizedPath = FilenameUtils.normalize(moduleBaseDir.getAbsolutePath() + File.separator + reportPath);
if (normalizedPath != null) {
includes.add(normalizedPath);
continue;
}

CxxUtils.LOG.debug("Not a valid report path '{}'", reportPath);
}

CxxUtils.LOG.debug("Normalized report includes to '{}'", includes);

// Includes array cannot contain null elements
DirectoryScanner directoryScanner = new DirectoryScanner();
directoryScanner.setIncludes(includes.toArray(new String[includes.size()]));
directoryScanner.scan();
Expand Down
69 changes: 42 additions & 27 deletions ...cxx-plugin/src/test/java/org/sonar/plugins/cxx/utils/CxxReportSensor_getReports_Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,19 @@

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.config.Settings;
import org.sonar.plugins.cxx.TestUtils;

public class CxxReportSensor_getReports_Test {

private static final String REPORT_PATH_KEY = "sonar.cxx.cppcheck.reportPath";

private class CxxReportSensorImpl extends CxxReportSensor {

public CxxReportSensorImpl(Settings settings, FileSystem fs) {
super(settings, fs);
}
};

@Rule
public TemporaryFolder base = new TemporaryFolder();

private CxxReportSensor sensor;
private Settings settings;
private FileSystem fs;

@Before
public void init() {
settings = new Settings();
fs = TestUtils.mockFileSystem();
sensor = new CxxReportSensorImpl(settings, fs);
}

@Test
public void getReports_patternMatching() throws java.io.IOException, java.lang.InterruptedException {
Settings settings = new Settings();
Expand All @@ -75,6 +55,7 @@ public void getReports_patternMatching() throws java.io.IOException, java.lang.I
examples.add(new String[]{"dir/A.ext", "dir/A.ext", "A.ext,dir/B.ext"});
examples.add(new String[]{"dir/../A.ext", "A.ext", "B.ext,dir/A.ext"});
examples.add(new String[]{"./A.ext", "A.ext", "B.ext"});
examples.add(new String[]{"./A.ext", "A.ext", "B.ext"});
// empty
examples.add(new String[]{"", "", ""});
// question mark glob
Expand All @@ -94,7 +75,7 @@ public void getReports_patternMatching() throws java.io.IOException, java.lang.I
setupExample(allpaths);
settings.setProperty(REPORT_PATH_KEY, pattern);

reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);

assertMatch(reports, match, example[0]);
deleteExample(base.getRoot());
Expand Down Expand Up @@ -135,7 +116,7 @@ public void testAbsoluteInsideBasedir() throws IOException {
Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString());

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(1, reports.size());
}

Expand All @@ -147,7 +128,7 @@ public void testAbsoluteOutsideBasedir() {
Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString());

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(1, reports.size());
}

Expand All @@ -159,7 +140,7 @@ public void testAbsoluteOutsideBasedirWithGlobbing() {
Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString());

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(2, reports.size());
}

Expand All @@ -174,7 +155,7 @@ public void testAbsoluteOutsideBasedirAndRelative() throws IOException {
Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + "," + relativeReport);

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(2, reports.size());
}

Expand All @@ -188,11 +169,14 @@ public void testAbsoluteOutsideBasedirWithGlobbingAndRelativeWithGlobbing() thro
FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml"));
FileUtils.touch(new File(base.getRoot(), "some/reports/a"));
FileUtils.touch(new File(base.getRoot(), "some/reports/b"));


Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + ",**/*.xml");

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(7, reports.size());
}

Expand All @@ -205,11 +189,42 @@ public void testAbsoluteOutsideBasedirWithGlobbingAndNestedRelativeWithGlobbing(
FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml"));
FileUtils.touch(new File(base.getRoot(), "some/reports/a.xml"));
FileUtils.touch(new File(base.getRoot(), "some/reports/b.xml"));

Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + ",path/**/*.xml");

List<File> reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(6, reports.size());
}

@Test
public void testRelativeBackticksOutsideBasedirThenBackInside() throws IOException {
FileUtils.touch(new File(base.getRoot(), "path/to/supercoolreport.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml"));
FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml"));

Settings settings = new Settings();
settings.setProperty(REPORT_PATH_KEY, "../" + base.getRoot().getName() + "/path/**/*.xml");

List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(4, reports.size());
}

@Test
public void testRelativeExcessiveBackticks() throws IOException {
FileUtils.touch(new File(base.getRoot(), "path/to/supercoolreport.xml"));

Settings settings = new Settings();
// Might be valid if java.io.tmpdir is nested excessively deep -- not likely
settings.setProperty(REPORT_PATH_KEY, "../../../../../../../../../../../../../../../../../../../../../../../../" +
"../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../" +
"../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../" +
"../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../*.xml");

List<File> reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY);
assertEquals(0, reports.size());
}
}

0 comments on commit 7395724

Please sign in to comment.