Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #766: Handle relative report paths with backticks outside basedir. #780

Merged
merged 1 commit into from
Feb 20, 2016
Merged

Fix #766: Handle relative report paths with backticks outside basedir. #780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java
View file
Open in desktop
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjdetullio I'm not a fan of continue in loops. Think if / else if / else would be the same ....

}
});

// 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
Open in desktop
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());
}
}