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.

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

Fix loose forbidden file #339

Merged
merged 2 commits into from Dec 18, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -96,13 +96,22 @@ public void setPattern(String pattern) {
*/
public boolean isInteresting(List<String> files) {
for (String file : files) {
if (compareType.matches(pattern, file)) {
if (isInteresting(file)) {
return true;
}
}
return false;
}

/**
* Tells if the given file is matched by this rule.
* @param file the file in the patch set.
* @return true if the files match.
*/
public boolean isInteresting(String file) {
return compareType.matches(pattern, file);
}

/**
* The Descriptor for the FilePath.
*/
@@ -34,6 +34,7 @@

import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;

import org.kohsuke.stapler.DataBoundConstructor;
@@ -216,32 +217,29 @@ public boolean isInteresting(String project, String branch, String topic, List<S
if (compareType.matches(pattern, project)) {
for (Branch b : branches) {
boolean foundInterestingForbidden = false;
boolean foundInterestingTopicOrFile = false;
if (b.isInteresting(branch)) {
if (forbiddenFilePaths != null) {
for (FilePath ffp : forbiddenFilePaths) {
if (ffp.isInteresting(files)) {
foundInterestingForbidden = true;
break;
Iterator<String> i = files.iterator();
while (i.hasNext()) {
String file = i.next();
for (FilePath ffp : forbiddenFilePaths) {
if (ffp.isInteresting(file)) {
if (!disableStrictForbiddenFileVerification) {
return false;
} else {
foundInterestingForbidden = true;
i.remove();

This comment has been minimized.

Copy link
@Jimilian

Jimilian Jan 15, 2018

Contributor

Potentially this line creates a bug. Because https://github.com/sonyxperiadev/gerrit-events/blob/7f3547c6d55946e25e99a847b5160d69e59994ba/src/main/java/com/sonymobile/tools/gerrit/gerritevents/dto/events/ChangeBasedEvent.java caches the list of files and checks it against null. So, one trigger modifies list of files visible for another trigger.

break;
}
}
}
}
}
if (isInterestingTopic(topic) && isInterestingFile(files)) {
foundInterestingTopicOrFile = true;
}
if (disableStrictForbiddenFileVerification) {
// Here we want to be able to trigger a build if the event contains
// wanted topics or file paths even though there may be a forbidden file
return foundInterestingTopicOrFile;
} else {
if (foundInterestingForbidden) {
// we have a forbidden file and a wanted file path.
return false;
} else if (foundInterestingTopicOrFile) {
// we DO not have a forbidden file and but we have a wanted file path.
return true;
}
if (foundInterestingForbidden && files.isEmpty()) {
// All changed files are forbidden, so this is not interesting
return false;
}
return isInterestingTopic(topic) && isInterestingFile(files);
}
}
}
@@ -70,73 +70,56 @@ public static Collection getParameters() {
List<InterestingScenarioWithFiles[]> parameters = new LinkedList<InterestingScenarioWithFiles[]>();

List<Branch> branches = new LinkedList<Branch>();
Branch branch = new Branch(CompareType.PLAIN, "master");
branches.add(branch);
branches.add(new Branch(CompareType.PLAIN, "master"));
List<Topic> topics = new LinkedList<Topic>();
List<FilePath> filePaths = new LinkedList<FilePath>();
FilePath filePath = new FilePath(CompareType.PLAIN, "test.txt");
filePaths.add(filePath);
List<FilePath> forbiddenFilePaths = null;
filePaths.add(new FilePath(CompareType.PLAIN, "test.txt"));
GerritProject config = new GerritProject(
CompareType.PLAIN, "project", branches, topics, filePaths, forbiddenFilePaths, false);
CompareType.PLAIN, "project", branches, topics, filePaths, null, false);
List<String> files = new LinkedList<String>();
files.add("test.txt");
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "project", "master", null, files, true), });

branches = new LinkedList<Branch>();
branch = new Branch(CompareType.REG_EXP, "feature/.*master");
branches.add(branch);
branches.add(new Branch(CompareType.REG_EXP, "feature/.*master"));
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.REG_EXP, "tests/.*");
filePaths.add(filePath);
filePaths.add(new FilePath(CompareType.REG_EXP, "tests/.*"));
files = new LinkedList<String>();
files.add("tests/test.txt");
forbiddenFilePaths = null;
config = new GerritProject(CompareType.REG_EXP, "project.*5", branches, topics, filePaths, forbiddenFilePaths,
false);
config = new GerritProject(CompareType.REG_EXP, "project.*5", branches, topics, filePaths, null, false);
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "projectNumber5", "feature/mymaster", null, files, true), });

branches = new LinkedList<Branch>();
branch = new Branch(CompareType.ANT, "**/master");
branches.add(branch);
branches.add(new Branch(CompareType.ANT, "**/master"));
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.ANT, "**/*test*");
filePaths.add(filePath);
forbiddenFilePaths = null;
filePaths.add(new FilePath(CompareType.ANT, "**/*test*"));
config = new GerritProject(
CompareType.ANT, "vendor/**/project", branches, topics, filePaths, forbiddenFilePaths, false);
CompareType.ANT, "vendor/**/project", branches, topics, filePaths, null, false);
files = new LinkedList<String>();
files.add("resources/test.xml");
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "vendor/semc/master/project", "origin/master", null, files, true), });

branches = new LinkedList<Branch>();
branch = new Branch(CompareType.REG_EXP, "feature/.*master");
branches.add(branch);
branches.add(new Branch(CompareType.REG_EXP, "feature/.*master"));
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.REG_EXP, "tests/.*");
filePaths.add(filePath);
filePaths.add(new FilePath(CompareType.REG_EXP, "tests/.*"));
files = new LinkedList<String>();
files.add("notintests/test.txt");
forbiddenFilePaths = null;
config = new GerritProject(CompareType.REG_EXP, "project.*5", branches, topics, filePaths, forbiddenFilePaths,
false);
config = new GerritProject(CompareType.REG_EXP, "project.*5", branches, topics, filePaths, null, false);
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "projectNumber5", "feature/mymaster", null, files, false), });

//Testing with Forbidden File Paths now
branches = new LinkedList<Branch>();
branch = new Branch(CompareType.PLAIN, "master");
branches.add(branch);
branches.add(new Branch(CompareType.PLAIN, "master"));
topics = new LinkedList<Topic>();
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.PLAIN, "test.txt");
filePaths.add(filePath);
forbiddenFilePaths = new LinkedList<FilePath>();
FilePath forbiddenFilePath = new FilePath(CompareType.PLAIN, "test2.txt");
forbiddenFilePaths.add(forbiddenFilePath);
filePaths.add(new FilePath(CompareType.PLAIN, "test.txt"));
List<FilePath> forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePaths.add(new FilePath(CompareType.PLAIN, "test2.txt"));
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, filePaths, forbiddenFilePaths, false);
files = new LinkedList<String>();
files.add("test.txt");
@@ -147,15 +130,12 @@ public static Collection getParameters() {
//Testing with Forbidden File Paths now BUT disableStrictForbiddenFileVerification
// is true
branches = new LinkedList<Branch>();
branch = new Branch(CompareType.PLAIN, "master");
branches.add(branch);
branches.add(new Branch(CompareType.PLAIN, "master"));
topics = new LinkedList<Topic>();
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.PLAIN, "test.txt");
filePaths.add(filePath);
filePaths.add(new FilePath(CompareType.PLAIN, "test.txt"));
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePath = new FilePath(CompareType.PLAIN, "test2.txt");
forbiddenFilePaths.add(forbiddenFilePath);
forbiddenFilePaths.add(new FilePath(CompareType.PLAIN, "test2.txt"));
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, filePaths, forbiddenFilePaths, true);
files = new LinkedList<String>();
files.add("test.txt");
@@ -165,29 +145,50 @@ public static Collection getParameters() {

//Testing with Forbidden File Paths now BUT no filepaths defined
branches = new LinkedList<Branch>();
branch = new Branch(CompareType.PLAIN, "master");
branches.add(branch);
branches.add(new Branch(CompareType.PLAIN, "master"));
topics = new LinkedList<Topic>();
filePaths = null;
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePath = new FilePath(CompareType.PLAIN, "test2.txt");
forbiddenFilePaths.add(forbiddenFilePath);
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, filePaths, forbiddenFilePaths, false);
forbiddenFilePaths.add(new FilePath(CompareType.PLAIN, "test2.txt"));
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, null, forbiddenFilePaths, false);
files = new LinkedList<String>();
files.add("test.txt");
files.add("test2.txt");
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "project", "master", null, files, false), });

//Testing with Forbidden File Paths now BUT no filepaths defined AND
//disableStrictForbiddenFileVerification is true, with both forbidden & allowed files
branches = new LinkedList<Branch>();
branches.add(new Branch(CompareType.PLAIN, "master"));
topics = new LinkedList<Topic>();
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePaths.add(new FilePath(CompareType.PLAIN, "test2.txt"));
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, null, forbiddenFilePaths, true);
files = new LinkedList<String>();
files.add("test.txt");
files.add("test2.txt");
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "project", "master", null, files, true), });

//Testing with Forbidden File Paths BUT no filepaths defined AND
//disableStrictForbiddenFileVerification is true, with only forbidden files
branches = new LinkedList<Branch>();
branches.add(new Branch(CompareType.PLAIN, "master"));
topics = new LinkedList<Topic>();
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePaths.add(new FilePath(CompareType.PLAIN, "test2.txt"));
config = new GerritProject(CompareType.PLAIN, "project", branches, topics, null, forbiddenFilePaths, true);
files = new LinkedList<String>();
files.add("test2.txt");
parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles(
config, "project", "master", null, files, false), });

branches = new LinkedList<Branch>();
branch = new Branch(CompareType.REG_EXP, "feature/.*master");
branches.add(branch);
branches.add(new Branch(CompareType.REG_EXP, "feature/.*master"));
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.REG_EXP, "tests/.*");
filePaths.add(filePath);
filePaths.add(new FilePath(CompareType.REG_EXP, "tests/.*"));
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePath = new FilePath(CompareType.REG_EXP, "tests/.*2.*");
forbiddenFilePaths.add(forbiddenFilePath);
forbiddenFilePaths.add(new FilePath(CompareType.REG_EXP, "tests/.*2.*"));
files = new LinkedList<String>();
files.add("tests/test.txt");
files.add("tests/test2.txt");
@@ -197,14 +198,11 @@ public static Collection getParameters() {
config, "projectNumber5", "feature/mymaster", null, files, false), });

branches = new LinkedList<Branch>();
branch = new Branch(CompareType.ANT, "**/master");
branches.add(branch);
branches.add(new Branch(CompareType.ANT, "**/master"));
filePaths = new LinkedList<FilePath>();
filePath = new FilePath(CompareType.ANT, "**/*test*");
filePaths.add(filePath);
filePaths.add(new FilePath(CompareType.ANT, "**/*test*"));
forbiddenFilePaths = new LinkedList<FilePath>();
forbiddenFilePath = new FilePath(CompareType.ANT, "**/*skip*");
forbiddenFilePaths.add(forbiddenFilePath);
forbiddenFilePaths.add(new FilePath(CompareType.ANT, "**/*skip*"));
config = new GerritProject(
CompareType.ANT, "vendor/**/project", branches, topics, filePaths, forbiddenFilePaths, false);
files = new LinkedList<String>();
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.