Skip to content

S5445, S5332, S4507, S5443: use new API for symbol fully qualified name #358

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

Merged
pynicolas merged 2 commits into from
Sep 24, 2019
Merged

S5445, S5332, S4507, S5443: use new API for symbol fully qualified name #358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebbed99
S5445, S5332, S4507, S5443: use new API for symbol fully qualified name
andreaguarino Sep 24, 2019
5306c94
increase code coverage
andreaguarino Sep 24, 2019
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
8 changes: 4 additions & 4 deletions python-checks/src/main/java/org/sonar/python/checks/TempFileCreationCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import org.sonar.python.PythonSubscriptionCheck;
import org.sonar.python.api.tree.PyCallExpressionTree;
import org.sonar.python.api.tree.Tree;
import org.sonar.python.semantic.Symbol;
import org.sonar.python.semantic.TreeSymbol;

@Rule(key = "S5445")
public class TempFileCreationCheck extends PythonSubscriptionCheck {
Expand All @@ -38,15 +38,15 @@ public class TempFileCreationCheck extends PythonSubscriptionCheck {
public void initialize(Context context) {
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
PyCallExpressionTree callExpr = (PyCallExpressionTree) ctx.syntaxNode();
Symbol symbol = ctx.symbolTable().getSymbol(callExpr);
TreeSymbol symbol = callExpr.calleeSymbol();
isInsecureTempFile(symbol).ifPresent(s -> ctx.addIssue(callExpr, String.format("'%s' is insecure. Use 'tempfile.TemporaryFile' instead", s)));
});
}

private static Optional<String> isInsecureTempFile(@Nullable Symbol symbol) {
private static Optional<String> isInsecureTempFile(@Nullable TreeSymbol symbol) {
if (symbol == null) {
return Optional.empty();
}
return SUSPICIOUS_CALLS.stream().filter(symbol.qualifiedName()::equals).findFirst();
return SUSPICIOUS_CALLS.stream().filter(call -> call.equals(symbol.fullyQualifiedName())).findFirst();
}
}
8 changes: 4 additions & 4 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/ClearTextProtocolsCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.sonar.python.api.tree.PyCallExpressionTree;
import org.sonar.python.api.tree.PyStringElementTree;
import org.sonar.python.api.tree.Tree;
import org.sonar.python.semantic.Symbol;
import org.sonar.python.semantic.TreeSymbol;

@Rule(key = "S5332")
public class ClearTextProtocolsCheck extends PythonSubscriptionCheck {
Expand All @@ -59,7 +59,7 @@ public void initialize(Context context) {
.ifPresent(protocol -> ctx.addIssue(node, message(protocol)));
});
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
Symbol symbol = ctx.symbolTable().getSymbol((PyCallExpressionTree) ctx.syntaxNode());
TreeSymbol symbol = ((PyCallExpressionTree) ctx.syntaxNode()).calleeSymbol();
isUnsafeLib(symbol).ifPresent(protocol -> ctx.addIssue(ctx.syntaxNode(), message(protocol)));
});
}
Expand Down Expand Up @@ -89,9 +89,9 @@ private static Optional<String> unsafeProtocol(String literalValue) {
return Optional.empty();
}

private static Optional<String> isUnsafeLib(@Nullable Symbol symbol) {
private static Optional<String> isUnsafeLib(@Nullable TreeSymbol symbol) {
if (symbol != null) {
String qualifiedName = symbol.qualifiedName();
String qualifiedName = symbol.fullyQualifiedName();
if ("telnetlib.Telnet".equals(qualifiedName)) {
return Optional.of("telnet");
}
Expand Down
14 changes: 7 additions & 7 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/DebugModeCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import javax.annotation.CheckForNull;
import org.sonar.check.Rule;
import org.sonar.python.PythonSubscriptionCheck;
import org.sonar.python.SubscriptionContext;
import org.sonar.python.api.tree.PyArgumentTree;
import org.sonar.python.api.tree.PyAssignmentStatementTree;
import org.sonar.python.api.tree.PyCallExpressionTree;
Expand All @@ -34,7 +34,7 @@
import org.sonar.python.api.tree.PyNameTree;
import org.sonar.python.api.tree.PyQualifiedExpressionTree;
import org.sonar.python.api.tree.Tree.Kind;
import org.sonar.python.semantic.Symbol;
import org.sonar.python.semantic.TreeSymbol;

@Rule(key = DebugModeCheck.CHECK_KEY)
public class DebugModeCheck extends PythonSubscriptionCheck {
Expand All @@ -50,8 +50,7 @@ public void initialize(Context context) {
if (!(callExpression.callee() instanceof PyQualifiedExpressionTree)) {
return;
}
PyQualifiedExpressionTree callee = (PyQualifiedExpressionTree) callExpression.callee();
if (getQualifiedName(callee.qualifier(), ctx).equals("django.conf.settings") && callee.name().name().equals("configure") && !arguments.isEmpty()) {
if ("django.conf.settings.configure".equals(getQualifiedName(callExpression)) && !arguments.isEmpty()) {
arguments.stream().filter(DebugModeCheck::isDebugArgument).forEach(arg -> ctx.addIssue(arg, MESSAGE));
}
});
Expand Down Expand Up @@ -86,8 +85,9 @@ private static boolean isDebugArgument(PyArgumentTree argument) {
return false;
}

private static String getQualifiedName(PyExpressionTree node, SubscriptionContext ctx) {
Symbol symbol = ctx.symbolTable().getSymbol(node);
return symbol != null ? symbol.qualifiedName() : "";
@CheckForNull
private static String getQualifiedName(PyCallExpressionTree callExpression) {
TreeSymbol symbol = callExpression.calleeSymbol();
return symbol != null ? symbol.fullyQualifiedName() : "";
}
}
41 changes: 13 additions & 28 deletions ...ecks/src/main/java/org/sonar/python/checks/hotspots/PubliclyWritableDirectoriesCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,15 @@
import java.util.regex.Pattern;
import org.sonar.check.Rule;
import org.sonar.python.PythonSubscriptionCheck;
import org.sonar.python.api.tree.HasSymbol;
import org.sonar.python.api.tree.PyArgumentTree;
import org.sonar.python.api.tree.PyCallExpressionTree;
import org.sonar.python.api.tree.PyExpressionTree;
import org.sonar.python.api.tree.PyNameTree;
import org.sonar.python.api.tree.PyQualifiedExpressionTree;
import org.sonar.python.api.tree.PyStringElementTree;
import org.sonar.python.api.tree.PyStringLiteralTree;
import org.sonar.python.api.tree.PySubscriptionExpressionTree;
import org.sonar.python.api.tree.Tree.Kind;
import org.sonar.python.semantic.Symbol;
import org.sonar.python.semantic.SymbolTable;
import org.sonar.python.semantic.TreeSymbol;

@Rule(key = "S5443")
public class PubliclyWritableDirectoriesCheck extends PythonSubscriptionCheck {
Expand All @@ -62,7 +60,7 @@ public void initialize(Context context) {
context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, ctx -> {
PyCallExpressionTree tree = (PyCallExpressionTree) ctx.syntaxNode();
List<PyArgumentTree> arguments = tree.arguments();
if (isOsEnvironGetter(tree.callee(), ctx.symbolTable()) &&
if (isOsEnvironGetter(tree) &&
arguments.stream().map(PyArgumentTree::expression)
.anyMatch(PubliclyWritableDirectoriesCheck::isNonCompliantOsEnvironArgument)) {
ctx.addIssue(tree, MESSAGE);
Expand All @@ -71,7 +69,7 @@ public void initialize(Context context) {

context.registerSyntaxNodeConsumer(Kind.SUBSCRIPTION, ctx -> {
PySubscriptionExpressionTree tree = (PySubscriptionExpressionTree) ctx.syntaxNode();
if (isOsEnvironQualifiedExpression(tree.object(), ctx.symbolTable()) && tree.subscripts().expressions().stream()
if (isOsEnvironQualifiedExpression(tree.object()) && tree.subscripts().expressions().stream()
.anyMatch(PubliclyWritableDirectoriesCheck::isNonCompliantOsEnvironArgument)) {
ctx.addIssue(tree, MESSAGE);
}
Expand All @@ -88,30 +86,17 @@ private static boolean isNonCompliantOsEnvironArgument(PyExpressionTree expressi
((PyStringLiteralTree) expression).stringElements().stream().map(s -> s.trimmedQuotesValue().toLowerCase(Locale.ENGLISH)).anyMatch(NONCOMPLIANT_ENVIRON_VARIABLES::contains);
}

// Could be either `os.environ.get` or `environ.get`
private static boolean isOsEnvironGetter(PyExpressionTree expression, SymbolTable symbolTable) {
if (!expression.is(Kind.QUALIFIED_EXPR)) {
return false;
}
PyQualifiedExpressionTree qualifiedExpression = (PyQualifiedExpressionTree) expression;
if (qualifiedExpression.name().name().equals("get")) {
return isOsEnvironQualifiedExpression(qualifiedExpression.qualifier(), symbolTable);
}
return false;
private static boolean isOsEnvironGetter(PyCallExpressionTree callExpressionTree) {
TreeSymbol symbol = callExpressionTree.calleeSymbol();
return symbol != null && "os.environ.get".equals(symbol.fullyQualifiedName());
}

// Could be either `os.environ` or `from os import environ; ... ; environ`
private static boolean isOsEnvironQualifiedExpression(PyExpressionTree expression, SymbolTable symbolTable) {
Symbol symbol = symbolTable.getSymbol(expression);
if (symbol != null) {
return symbol.qualifiedName().equals("os.environ");
}
if (!expression.is(Kind.QUALIFIED_EXPR)) {
return false;
}
PyQualifiedExpressionTree qualifiedExpression = (PyQualifiedExpressionTree) expression;
if (qualifiedExpression.qualifier().is(Kind.NAME)) {
return ((PyNameTree) qualifiedExpression.qualifier()).name().equals("os") && qualifiedExpression.name().name().equals("environ");
private static boolean isOsEnvironQualifiedExpression(PyExpressionTree expression) {
if (expression instanceof HasSymbol) {
TreeSymbol symbol = ((HasSymbol) expression).symbol();
if (symbol != null) {
return "os.environ".equals(symbol.fullyQualifiedName());
}
}
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions python-checks/src/test/resources/checks/hotspots/debugMode/debugModeActivated.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
mysettings.configure(DEBUG=True) # OK
settings.otherFn(DEBUG=True) # OK
settings.configure()

configure(DEBUG=True) # OK
configure() # OK
foo.configure(DEBUG=True) # OK

def custom_config(config):
settings.configure(default_settings=config, DEBUG=True) # Noncompliant
Expand Down
1 change: 1 addition & 0 deletions python-checks/src/test/resources/checks/hotspots/publiclyWritableDirectories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def environ_variables():
tmp_dir = os.environ['OTHER'] # OK
tmp_dir = os.other['TMPDIR'] # OK
tmp_dir = other['TMPDIR'] # OK
tmp_dir = foo()['TMPDIR'] # OK
tmp_dir = os.foo.environ['TMPDIR'] # OK
tmp_dir = environ['TMPDIR'] # Noncompliant
tmp_dir = environ.get('TMPDIR') # Noncompliant
Expand Down