Skip to content

Commit

Permalink
Improve code quality
Browse files Browse the repository at this point in the history
  • Loading branch information
SachinAkash01 committed Feb 27, 2025
1 parent e781e48 commit c79470a
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class Constants {
private Constants() {}

public static final String SCANNER_CONTEXT = "ScannerContext";
public static final String IO = "io";

public static final List<String> IO_FUNCTIONS = List.of(

Check warning on line 29 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/Constants.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/Constants.java#L29

Added line #L29 was not covered by tests
"fileReadBytes",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode;
import io.ballerina.compiler.syntax.tree.Node;
import io.ballerina.compiler.syntax.tree.NodeList;
import io.ballerina.compiler.syntax.tree.ParameterNode;
import io.ballerina.compiler.syntax.tree.PositionalArgumentNode;
import io.ballerina.compiler.syntax.tree.QualifiedNameReferenceNode;
import io.ballerina.compiler.syntax.tree.RequiredParameterNode;
Expand All @@ -43,6 +42,7 @@

import java.util.Optional;

import static io.ballerina.stdlib.io.compiler.Constants.IO;
import static io.ballerina.stdlib.io.compiler.Constants.IO_FUNCTIONS;
import static io.ballerina.stdlib.io.compiler.staticcodeanalyzer.IORule.AVOID_PATH_TRAVERSAL;

Expand Down Expand Up @@ -73,11 +73,9 @@ public void perform(SyntaxNodeAnalysisContext context) {
String moduleName = qualifiedName.modulePrefix().text();
String functionNameStr = qualifiedName.identifier().text();

Check warning on line 74 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L73-L74

Added lines #L73 - L74 were not covered by tests

if ("io".equals(moduleName) && IO_FUNCTIONS.contains(functionNameStr)) {
if (!isSafePath(functionCall)) {
Location location = functionCall.location();
this.reporter.reportIssue(getDocument(context), location, AVOID_PATH_TRAVERSAL.getId());
}
if (IO.equals(moduleName) && IO_FUNCTIONS.contains(functionNameStr) && !isSafePath(functionCall)) {
Location location = functionCall.location();
this.reporter.reportIssue(getDocument(context), location, AVOID_PATH_TRAVERSAL.getId());

Check warning on line 78 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L77-L78

Added lines #L77 - L78 were not covered by tests
}
}
}

Check warning on line 81 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L81

Added line #L81 was not covered by tests
Expand Down Expand Up @@ -112,57 +110,68 @@ private boolean isSafePath(FunctionCallExpressionNode functionCall) {
private boolean isVariableSafe(SimpleNameReferenceNode variableRef) {
String variableName = variableRef.name().text();
Node currentNode = variableRef.parent();

Check warning on line 112 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L111-L112

Added lines #L111 - L112 were not covered by tests

while (currentNode != null) {
if (currentNode instanceof FunctionBodyBlockNode functionBody) {
for (StatementNode statement : functionBody.statements()) {
if (statement instanceof VariableDeclarationNode varDecl) {
if (varDecl.typedBindingPattern().bindingPattern() instanceof
CaptureBindingPatternNode bindingPattern) {
if (bindingPattern.variableName().text().equals(variableName)) {
// Check if assigned using concatenation
if (varDecl.initializer().orElse(null) instanceof BinaryExpressionNode) {
BinaryExpressionNode binaryExpr = (BinaryExpressionNode)
varDecl.initializer().get();
if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) {
return isFunctionParameter(variableRef);
}
}
// Check if assigned directly from a function parameter
if (varDecl.initializer().orElse(null) instanceof SimpleNameReferenceNode) {
SimpleNameReferenceNode initializer = (SimpleNameReferenceNode)
varDecl.initializer().get();
return isFunctionParameter(initializer);
}
return true;
}
}
}
}
return isVariableDeclaredSafely(functionBody, variableName, variableRef);

Check warning on line 116 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L116

Added line #L116 was not covered by tests
}
currentNode = currentNode.parent();

Check warning on line 118 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L118

Added line #L118 was not covered by tests
}
return true;

Check warning on line 120 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L120

Added line #L120 was not covered by tests
}

private boolean isVariableDeclaredSafely(FunctionBodyBlockNode functionBody, String variableName,
SimpleNameReferenceNode variableRef) {
for (StatementNode statement : functionBody.statements()) {
if (!(statement instanceof VariableDeclarationNode varDecl)) {
continue;
}

if (!isMatchingVariable(varDecl, variableName)) {
continue;

Check warning on line 131 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L131

Added line #L131 was not covered by tests
}

ExpressionNode initializer = varDecl.initializer().orElse(null);

Check warning on line 134 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L134

Added line #L134 was not covered by tests
if (initializer == null) {
return true;

Check warning on line 136 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L136

Added line #L136 was not covered by tests
}

if (isConcatenationAssignment(initializer)) {
return isFunctionParameter(variableRef);

Check warning on line 140 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L140

Added line #L140 was not covered by tests
}

if (initializer instanceof SimpleNameReferenceNode refNode) {
return isFunctionParameter(refNode);

Check warning on line 144 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L144

Added line #L144 was not covered by tests
}

return true;

Check warning on line 147 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L147

Added line #L147 was not covered by tests
}
return true;

Check warning on line 149 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L149

Added line #L149 was not covered by tests
}

private boolean isMatchingVariable(VariableDeclarationNode varDecl, String variableName) {
return varDecl.typedBindingPattern().bindingPattern() instanceof CaptureBindingPatternNode bindingPattern
&& bindingPattern.variableName().text().equals(variableName);
}

private boolean isConcatenationAssignment(ExpressionNode initializer) {
return initializer instanceof BinaryExpressionNode binaryExpr

Check warning on line 158 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L158

Added line #L158 was not covered by tests
&& binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN;
}

private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) {
String paramName = variableRef.name().text();
Node currentNode = variableRef.parent();

Check warning on line 164 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L163-L164

Added lines #L163 - L164 were not covered by tests

while (currentNode != null) {
if (currentNode instanceof FunctionDefinitionNode functionDef) {
// Iterate over function parameters to check direct reference
for (ParameterNode param : functionDef.functionSignature().parameters()) {
if (param instanceof RequiredParameterNode reqParam) {
// Check direct parameter reference
if (reqParam.paramName().isPresent() &&
reqParam.paramName().get().toString().equals(paramName)) {
return false;
}
// Check indirect reference chain (assignments)
if (isIndirectFunctionParameter(variableRef, reqParam)) {
return false;
}
}
}
return functionDef.functionSignature().parameters().stream()
.filter(RequiredParameterNode.class::isInstance)
.map(RequiredParameterNode.class::cast)
.noneMatch(reqParam -> reqParam.paramName()

Check warning on line 171 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L168-L171

Added lines #L168 - L171 were not covered by tests
.map(name -> name.toString().equals(paramName) ||
isIndirectFunctionParameter(variableRef, reqParam))
.orElse(false));

Check warning on line 174 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L174

Added line #L174 was not covered by tests
}
currentNode = currentNode.parent();

Check warning on line 176 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L176

Added line #L176 was not covered by tests
}
Expand All @@ -171,45 +180,45 @@ private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) {

private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, RequiredParameterNode reqParam) {
Node currentNode = variableRef.parent();

Check warning on line 182 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L182

Added line #L182 was not covered by tests

while (currentNode != null) {
if (currentNode instanceof FunctionBodyBlockNode functionBody) {
for (StatementNode statement : functionBody.statements()) {
if (statement instanceof VariableDeclarationNode varDecl) {
if (varDecl.typedBindingPattern().bindingPattern() instanceof
CaptureBindingPatternNode bindingPattern) {
if (bindingPattern.variableName().text().equals(variableRef.name().text())) {
// Check if the variable is assigned to another variable
if (varDecl.initializer().isPresent()) {
ExpressionNode initializer = varDecl.initializer().get();
// If it's a reference to the function parameter, return true
if (initializer instanceof SimpleNameReferenceNode initializerRef) {
if (initializerRef.name().text().equals(reqParam.paramName().get().text())) {
return true;
}
}
// If it's a binary expression, recurse
if (initializer instanceof BinaryExpressionNode binaryExpr) {
if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) {
// Recursively check both sides of the binary expression
if (isIndirectFunctionParameterFromBinary(binaryExpr, reqParam)) {
return true;
}
}
}
}
}
}
}
}
return functionBody.statements().stream()
.filter(VariableDeclarationNode.class::isInstance)
.map(VariableDeclarationNode.class::cast)
.anyMatch(varDecl ->
isAssignedToFunctionParameter(varDecl, variableRef, reqParam));

Check warning on line 190 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L186-L190

Added lines #L186 - L190 were not covered by tests
}
currentNode = currentNode.parent();

Check warning on line 192 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L192

Added line #L192 was not covered by tests
}
return false;

Check warning on line 194 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L194

Added line #L194 was not covered by tests
}

private boolean isAssignedToFunctionParameter(VariableDeclarationNode varDecl, SimpleNameReferenceNode variableRef,
RequiredParameterNode reqParam) {
if (varDecl.typedBindingPattern().bindingPattern() instanceof CaptureBindingPatternNode bindingPattern &&
bindingPattern.variableName().text().equals(variableRef.name().text())) {

return varDecl.initializer().map(initializer ->
isInitializerAssignedToFunctionParameter(initializer, reqParam))
.orElse(false);

Check warning on line 204 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L202-L204

Added lines #L202 - L204 were not covered by tests
}
return false;

Check warning on line 206 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L206

Added line #L206 was not covered by tests
}

private boolean isInitializerAssignedToFunctionParameter(ExpressionNode initializer,
RequiredParameterNode reqParam) {
if (initializer instanceof SimpleNameReferenceNode initializerRef) {
return initializerRef.name().text().equals(reqParam.paramName().get().text());

Check warning on line 212 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L212

Added line #L212 was not covered by tests
} else if (initializer instanceof BinaryExpressionNode binaryExpr &&
binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) {
return isIndirectFunctionParameterFromBinary(binaryExpr, reqParam);

Check warning on line 215 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L215

Added line #L215 was not covered by tests
}
return false;

Check warning on line 217 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L217

Added line #L217 was not covered by tests
}

private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr,
RequiredParameterNode reqParam) {
// Check both the left and right sides of the binary expression
if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef) {
if (leftRef.name().text().equals(reqParam.paramName().get().text())) {
return true;

Check warning on line 224 in compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java

View check run for this annotation

Codecov / codecov/patch

compiler-plugin/src/main/java/io/ballerina/stdlib/io/compiler/staticcodeanalyzer/IOPathInjectionAnalyzer.java#L224

Added line #L224 was not covered by tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
* {@code RuleFactory} contains the logic to create a {@link Rule}.
*/
public class RuleFactory {

private RuleFactory() {}

public static Rule createRule(int id, String description, RuleKind kind) {
return new RuleImpl(id, description, kind);
}
Expand Down

0 comments on commit c79470a

Please sign in to comment.