Skip to content

Commit

Permalink
Allow for multiple search markers per element (#71)
Browse files Browse the repository at this point in the history 
* Revert "Revert "adding test which have been fixed""

This reverts commit d3f0d79.

* Add language hints

* Add Nullable annotation to SecurityUtils.Result.java

* Do not mark source or sink as flow participants

Now that we allow for multiple search results to stack: openrewrite/rewrite@3c314df

---------

Co-authored-by: lkerford <[email protected]>
  • Loading branch information
@timtebeek @lkerford
timtebeek and lkerford authored Nov 11, 2024
1 parent d3f0d79 commit 6268a76
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/main/java/org/openrewrite/analysis/controlflow/ControlFlowVisualization.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
@Value
@EqualsAndHashCode(callSuper = false)
public class ControlFlowVisualization extends Recipe {

@Option(displayName = "Include Dotfile",
description = "Also output with a Dotfile which can be then later visualized by Graphviz.")
boolean includeDotfile;
Expand Down
10 changes: 8 additions & 2 deletions src/main/java/org/openrewrite/analysis/dataflow/global/RenderGlobalFlowPaths.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ public class RenderGlobalFlowPaths<P> extends JavaIsoVisitor<P> {
@Override
public Expression visitExpression(Expression expression, P p) {
Expression e = super.visitExpression(expression, p);
boolean marked = false;
if (acc.isSource(getCursor())) {
e = SearchResult.mergingFound(e, "source");
marked = true;
}
if (acc.isSink(getCursor())) {
e = SearchResult.mergingFound(e, "sink");
marked = true;
}
if (expression != e) {
if (marked) {
return e;
}
if (acc.isFlowParticipant(getCursor())) {
Expand All @@ -46,13 +49,16 @@ public Expression visitExpression(Expression expression, P p) {
@Override
public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, P p) {
J.VariableDeclarations.NamedVariable v = super.visitVariable(variable, p);
boolean marked = false;
if (acc.isSource(getCursor())) {
v = SearchResult.mergingFound(v, "source");
marked = true;
}
if (acc.isSink(getCursor())) {
v = SearchResult.mergingFound(v, "sink");
marked = true;
}
if (variable != v) {
if (marked) {
return v;
}
if (acc.isFlowParticipant(getCursor())) {
Expand Down
15 changes: 8 additions & 7 deletions src/test/java/org/openrewrite/analysis/controlflow/ControlFlowTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public void defaults(RecipeSpec spec) {
@Test
void displayControlFlowGraphForSingleBasicBlock() {
rewriteRun(
//language=java
java(
"""
abstract class Test {
Expand Down Expand Up @@ -1665,10 +1666,10 @@ void test() /*~~(BB: 22 CN: 12 EX: 1 | 1L)~~>*/{
/*~~(3L)~~>*/if ((/*~~(2C)~~>*/potato)) /*~~(4L)~~>*/{
// ...
}
/*~~(5L)~~>*/if (/*~~(3C)~~>*/potato && /*~~(6L)~~>*/turnip) /*~~(7L)~~>*/{
/*~~(5L)~~>*/if (/*~~(3C)~~>*/potato && /*~~(6L)~~>*//*~~(4C)~~>*/turnip) /*~~(7L)~~>*/{
// ...
}
/*~~(8L)~~>*/if (/*~~(5C)~~>*/potato && /*~~(9L)~~>*/turnip || /*~~(10L)~~>*/squash) /*~~(11L)~~>*/{
/*~~(8L)~~>*/if (/*~~(5C)~~>*/potato && /*~~(9L)~~>*//*~~(6C)~~>*/turnip || /*~~(10L)~~>*//*~~(7C)~~>*/squash) /*~~(11L)~~>*/{
// ...
}
int a = /*~~(12L)~~>*/1, b = 2;
Expand Down Expand Up @@ -1925,7 +1926,7 @@ abstract class Test {
abstract boolean otherCondition();
void test() /*~~(BB: 3 CN: 2 EX: 1 | 1L)~~>*/{
while (/*~~(1C)~~>*/condition() && /*~~(2L | 2C)~~>*/otherCondition())/*~~(3L)~~>*/;
while (/*~~(1C)~~>*/condition() && /*~~(2L | 2C)~~>*//*~~(2C)~~>*/otherCondition())/*~~(3L)~~>*/;
}
}
"""
Expand Down Expand Up @@ -1956,7 +1957,7 @@ abstract class Test {
abstract boolean thirdCondition();
void test() /*~~(BB: 4 CN: 3 EX: 1 | 1L)~~>*/{
while (/*~~(1C)~~>*/condition() && /*~~(2L | 2C)~~>*/otherCondition() && /*~~(3L | 3C)~~>*/thirdCondition())/*~~(4L)~~>*/;
while (/*~~(1C)~~>*/condition() && /*~~(2L | 2C)~~>*//*~~(2C)~~>*/otherCondition() && /*~~(3L | 3C)~~>*//*~~(3C)~~>*/thirdCondition())/*~~(4L)~~>*/;
}
}
"""
Expand Down Expand Up @@ -2439,7 +2440,7 @@ void test(boolean condition) {
"""
abstract class Test {
abstract String[] array();
void test(boolean condition) /*~~(BB: 5 CN: 2 EX: 1 | 1L)~~>*/{
for (String s : /*~~(1C)~~>*/condition ? /*~~(2L)~~>*/array() : new /*~~(3L)~~>*/String[] { "Hello!" }) /*~~(4L)~~>*/{
System.out.println(s);
Expand Down Expand Up @@ -2698,7 +2699,7 @@ void switchCaseNoDefault() {
"""
class Test {
enum E { A, B, C }
void test(E x) {
switch (x) {
case A:
Expand All @@ -2718,7 +2719,7 @@ void test(E x) {
"""
class Test {
enum E { A, B, C }
void test(E x) /*~~(BB: 7 CN: 3 EX: 1 | 1L)~~>*/{
switch (x) {
/*~~(1C)~~>*/case A:
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/openrewrite/analysis/controlflow/GuardTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ void test() {
@Test
void identifiesGuardsForControlParenthesesWithMissingTypeInformation() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
spec -> spec.typeValidationOptions(TypeValidation.none()),
java(
"""
class Test {
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/org/openrewrite/analysis/dataflow/FindLocalTaintFlowTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public boolean isSanitizer(DataFlowNode node) {
@Test
void taintTrackingThroughStringManipulations() {
rewriteRun(
//language=java
java(
"""
class Test {
Expand Down Expand Up @@ -736,7 +737,7 @@ void taintTrackingThroughMethodCallArgument() {
java(
"""
import java.util.Objects;
class Test {
String source() { return null; }
void test() {
Expand All @@ -747,7 +748,7 @@ void test() {
""",
"""
import java.util.Objects;
class Test {
String source() { return null; }
void test() {
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/org/openrewrite/analysis/search/FindFlowBetweenMethodsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void taintFlowBetweenSubjectOnly() {
"Taint"
)
),
//language=java
java(
"""
import java.util.LinkedList;
Expand Down Expand Up @@ -78,6 +79,7 @@ void taintFlowBetweenArgumentsOnly() {
"Taint"
)
),
//language=java
java(
"""
import java.util.LinkedList;
Expand Down Expand Up @@ -118,6 +120,7 @@ void taintFlowThroughMultipleSubjectsIntegerSourceAndSinkMethodsSpecified() {
"Taint"
)
),
//language=java
java(
"""
import java.util.LinkedList;
Expand Down Expand Up @@ -160,6 +163,7 @@ void noTaintFlowThroughArguments() {
"Taint"
)
),
//language=java
java(
"""
import java.util.LinkedList;
Expand All @@ -186,6 +190,7 @@ void taintFlowBetweenArgumentsAndSubject() {
"Taint"
)
),
//language=java
java(
"""
import java.util.LinkedList;
Expand Down Expand Up @@ -227,6 +232,7 @@ void dataFlowBetweenFiles() {
"Value"
)
),
//language=java
java(
"""
class Test {
Expand All @@ -246,6 +252,7 @@ void test() {
}
"""
),
//language=java
java(
"""
class Provider {
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/org/openrewrite/analysis/search/FindMethodsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class FindMethodsTest implements RewriteTest {
void findConstructors() {
rewriteRun(
spec -> spec.recipe(new FindMethods("A <constructor>(String)", false, null)),
//language=java
java(
"""
class Test {
Expand All @@ -41,6 +42,7 @@ class Test {
}
"""
),
//language=java
java(
"""
class A {
Expand Down
4 changes: 3 additions & 1 deletion src/test/resources/find-local-taint-flow-tests/SecurityUtils.Result.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
*/
package com.amazonaws.serverless.proxy.internal;

import org.jspecify.annotations.Nullable;

import java.util.Locale;

public class SecurityUtils {
Expand All @@ -25,7 +27,7 @@ private static StringBuffer source() {
* @param s The string to be cleaned
* @return The escaped string
*/
public static String encode(String s) {
public static @Nullable String encode(String s) {
if (s == null) {
return null;
}
Expand Down

0 comments on commit 6268a76

Please sign in to comment.