Skip to content

fix security issue and do refactoring #1

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix security issue and do refactoring #1

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
17 changes: 12 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

<groupId>com.github.unchai</groupId>
<artifactId>checkstyle-github-maven-plugin</artifactId>
<version>0.0.1</version>
<version>0.0.2-SNAPSHOT</version>
<packaging>maven-plugin</packaging>

<name>Checkstyle github pull request maven plugin</name>
Expand Down Expand Up @@ -69,12 +69,12 @@
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-plugin-api</artifactId>
<version>3.5.4</version>
<version>3.6.1</version>
</dependency>
<dependency>
<groupId>org.apache.maven.plugin-tools</groupId>
<artifactId>maven-plugin-annotations</artifactId>
<version>3.5.2</version>
<version>3.6.0</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
Expand All @@ -85,19 +85,26 @@
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>github-api</artifactId>
<version>1.93</version>
<version>1.95</version>
</dependency>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>8.2</version>
<version>[8.29,)</version>
<scope>provided</scope>
<exclusions>
<exclusion>
<groupId>com.sun</groupId>
<artifactId>tools</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.18.12</version>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>junit</groupId>
Expand Down
24 changes: 2 additions & 22 deletions src/main/java/com/github/unchai/maven/checkstyle/ChangedFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,10 @@

import java.util.Map;

import org.apache.commons.lang3.builder.ToStringBuilder;
import lombok.Data;

@Data
public class ChangedFile {
private String path;
private Map<Integer, Integer> linePositionMap;

public String getPath() {
return path;
}

public void setPath(String path) {
this.path = path;
}

public Map<Integer, Integer> getLinePositionMap() {
return linePositionMap;
}

public void setLinePositionMap(Map<Integer, Integer> linePositionMap) {
this.linePositionMap = linePositionMap;
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}
}
2 changes: 1 addition & 1 deletion src/main/java/com/github/unchai/maven/checkstyle/CheckstyleAuditListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void fileFinished(AuditEvent event) {
public void addError(AuditEvent event) {
final CheckstyleError error = new CheckstyleError();
error.setSeverityLevel(event.getSeverityLevel());
error.setFilename(event.getFileName());
error.setPath(event.getFileName());
error.setLine(event.getLine());
error.setMessage(event.getMessage());

Expand Down
43 changes: 3 additions & 40 deletions src/main/java/com/github/unchai/maven/checkstyle/CheckstyleError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,13 @@
*/
package com.github.unchai.maven.checkstyle;

import org.apache.commons.lang3.builder.ToStringBuilder;

import com.puppycrawl.tools.checkstyle.api.SeverityLevel;
import lombok.Data;

@Data
public class CheckstyleError {
private SeverityLevel severityLevel;
private String filename;
private String path;
private int line;
private String message;

public SeverityLevel getSeverityLevel() {
return severityLevel;
}

public void setSeverityLevel(SeverityLevel severityLevel) {
this.severityLevel = severityLevel;
}

public String getFilename() {
return filename;
}

public void setFilename(String filename) {
this.filename = filename;
}

public int getLine() {
return line;
}

public void setLine(int line) {
this.line = line;
}

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}
}
24 changes: 17 additions & 7 deletions src/main/java/com/github/unchai/maven/checkstyle/CheckstyleExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.File;
import java.util.List;
import java.util.stream.Collectors;

import org.apache.maven.plugin.MojoExecutionException;

Expand All @@ -29,16 +30,17 @@
import com.puppycrawl.tools.checkstyle.api.Configuration;

public class CheckstyleExecutor {
private Checker checker;
private final Checker checker = new Checker();
private final String configLocation;

public CheckstyleExecutor() {
this.checker = new Checker();
public CheckstyleExecutor(String configLocation) {
this.configLocation = configLocation;
}

public List<CheckstyleError> execute(String config, List<File> files) throws MojoExecutionException {
public List<CheckstyleError> execute(File baseDir, List<String> files) throws MojoExecutionException {
try {
final Configuration configuration = ConfigurationLoader.loadConfiguration(
config,
configLocation,
new PropertiesExpander(System.getProperties())
);

Expand All @@ -47,11 +49,19 @@ public List<CheckstyleError> execute(String config, List<File> files) throws Moj
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(configuration);
checker.addListener(listener);
checker.process(files);
checker.process(files.stream().map(file -> new File(baseDir, file)).collect(Collectors.toList()));

return listener.getErrors();
return listener.getErrors()
.stream()
.map(error -> stripBaseDir(baseDir.getPath(), error))
.collect(Collectors.toList());
} catch (Exception e) {
throw new MojoExecutionException(e.getMessage(), e);
}
}

private CheckstyleError stripBaseDir(String baseDir, CheckstyleError checkstyleError) {
checkstyleError.setPath(checkstyleError.getPath().replace(baseDir, "").substring(1));
return checkstyleError;
}
}
32 changes: 2 additions & 30 deletions src/main/java/com/github/unchai/maven/checkstyle/Comment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,11 @@

import java.util.List;

import org.apache.commons.lang3.builder.ToStringBuilder;
import lombok.Data;

@Data
public class Comment {
private String path;
private int position;
private List<CheckstyleError> checkstyleErrors;

public String getPath() {
return path;
}

public void setPath(String path) {
this.path = path;
}

public int getPosition() {
return position;
}

public void setPosition(int position) {
this.position = position;
}

public List<CheckstyleError> getCheckstyleErrors() {
return checkstyleErrors;
}

public void setCheckstyleErrors(List<CheckstyleError> checkstyleErrors) {
this.checkstyleErrors = checkstyleErrors;
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}
}
22 changes: 5 additions & 17 deletions src/main/java/com/github/unchai/maven/checkstyle/GithubHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@
import org.kohsuke.github.GHPullRequestReviewComment;
import org.kohsuke.github.GHRepository;
import org.kohsuke.github.GitHub;
import org.kohsuke.github.GitHubBuilder;

class GithubHelper {
private static final String CONTEXT = "coding-convention/checkstyle";
private static final String PREFIX = "#### :rotating_light: checkstyle defects";

private GHRepository repo;
private GHPullRequest pr;
private String username;

GithubHelper() {
GithubHelper(GitHub github, String repository, int pullRequest) throws IOException {
this.username = github.getMyself().getLogin();
this.repo = github.getRepository(repository);
this.pr = this.repo.getPullRequest(pullRequest);
}

Map<Integer, Integer> parsePatch(String patch) {
Expand All @@ -69,21 +72,6 @@ Map<Integer, Integer> parsePatch(String patch) {
return map;
}

void connect(String endpoint, String token, String repository, int pullRequest) throws MojoExecutionException {
try {
final GitHub github = new GitHubBuilder()
.withEndpoint(endpoint)
.withOAuthToken(token)
.build();

this.username = github.getMyself().getLogin();
this.repo = github.getRepository(repository);
this.pr = this.repo.getPullRequest(pullRequest);
} catch (IOException e) {
throw new MojoExecutionException(e.getMessage(), e);
}
}

List<ChangedFile> listChangedFile() {
final List<ChangedFile> linePositionMap = new ArrayList<>();

Expand Down
48 changes: 21 additions & 27 deletions src/main/java/com/github/unchai/maven/checkstyle/MainMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package com.github.unchai.maven.checkstyle;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.EnumMap;
Expand All @@ -30,11 +31,11 @@

import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.kohsuke.github.GHCommitState;
import org.kohsuke.github.GitHub;
import org.kohsuke.github.GitHubBuilder;

import com.puppycrawl.tools.checkstyle.api.SeverityLevel;

Expand All @@ -58,32 +59,31 @@ public class MainMojo extends AbstractMojo {
@Parameter(property = "checkstyle-github.configLocation", required = true)
String configLocation;

@Component(role = GithubHelper.class)
GithubHelper githubHelper;

@Component(role = CheckstyleExecutor.class)
CheckstyleExecutor checkstyleExecutor;

@Override
public void execute() throws MojoExecutionException, MojoFailureException {
githubHelper.connect(ghEndpoint, ghToken, ghRepository, ghPullRequest);
public void execute() throws MojoExecutionException {
final GithubHelper githubHelper;

try {
final GitHub github = new GitHubBuilder().withEndpoint(ghEndpoint).withOAuthToken(ghToken).build();
githubHelper = new GithubHelper(github, ghRepository, ghPullRequest);
} catch (IOException e) {
throw new MojoExecutionException("Cannot connect github.");
}

githubHelper.changeStatus(GHCommitState.PENDING, null);

final CheckstyleExecutor checkstyleExecutor = new CheckstyleExecutor(configLocation);

final List<ChangedFile> changedFiles = githubHelper.listChangedFile();

final Map<String, ChangedFile> changedFileMap =
changedFiles
.stream()
.filter(changedFile -> changedFile.getPath().endsWith(".java"))
.collect(Collectors.toMap(ChangedFile::getPath, Function.identity()));

final List<File> files =
changedFiles
.stream()
.map(changedFile -> new File(projectBasedir, changedFile.getPath()))
.filter(file -> file.getName().endsWith(".java"))
.collect(Collectors.toList());

final List<CheckstyleError> checkstyleErrors =
checkstyleExecutor.execute(configLocation, files)
checkstyleExecutor.execute(projectBasedir, changedFiles.stream().map(ChangedFile::getPath).collect(Collectors.toList()))
.stream()
.filter(checkstyleError -> contains(checkstyleError, changedFileMap))
.collect(Collectors.toList());
Expand Down Expand Up @@ -114,14 +114,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
}

private boolean contains(CheckstyleError checkstyleError, Map<String, ChangedFile> changedFileMap) {
final String path = stripBasedir(checkstyleError.getFilename());

return changedFileMap.containsKey(path)
&& changedFileMap.get(path).getLinePositionMap().containsKey(checkstyleError.getLine());
}

private String stripBasedir(String filepath) {
return filepath.replace(projectBasedir.getPath(), "").substring(1);
return changedFileMap.containsKey(checkstyleError.getPath())
&& changedFileMap.get(checkstyleError.getPath()).getLinePositionMap().containsKey(checkstyleError.getLine());
}

private Map<SeverityLevel, Integer> buildSeverityLevelCountMap(List<CheckstyleError> errors) {
Expand All @@ -145,7 +139,7 @@ private Collection<Comment> buildComments(
final Map<String, Comment> commentMap = new HashMap<>();

for (CheckstyleError error : errors) {
final String path = stripBasedir(error.getFilename());
final String path = error.getPath();
final String key = path + "|" + error.getLine();

if (commentMap.containsKey(key)) {
Expand Down
Loading