Skip to content

Commit

Permalink
fix: ArrayIndexOutOfBoundsException in string concatenation visitor (#…
Browse files Browse the repository at this point in the history
…427)

* fix: ArrayIndexOutOfBoundsException in string concatenation visitor
* fix: typo in comment
* fix: StringBuilder chain processing created wrong code
* test: simple JUnit test cases added for testing StringBuilder chain processing (chains that can be and that can't be simplified)
  • Loading branch information
jpstotz authored and skylot committed Jan 12, 2019
1 parent 727285e commit 72b2663
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package jadx.core.dex.instructions;

import jadx.core.dex.info.MethodInfo;

public interface CallMthInterface {

public MethodInfo getCallMth();
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import jadx.core.utils.InsnUtils;
import jadx.core.utils.Utils;

public class InvokeNode extends InsnNode {
public class InvokeNode extends InsnNode implements CallMthInterface {

private final InvokeType type;
private final MethodInfo mth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

import jadx.core.dex.info.ClassInfo;
import jadx.core.dex.info.MethodInfo;
import jadx.core.dex.instructions.CallMthInterface;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.InvokeNode;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;

public class ConstructorInsn extends InsnNode {
public class ConstructorInsn extends InsnNode implements CallMthInterface {

private final MethodInfo callMth;
private final CallType callType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@
import java.util.Collections;
import java.util.List;

import jadx.core.dex.instructions.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import jadx.core.Consts;
import jadx.core.dex.info.FieldInfo;
import jadx.core.dex.info.MethodInfo;
import jadx.core.dex.instructions.ArithNode;
import jadx.core.dex.instructions.ArithOp;
import jadx.core.dex.instructions.ConstStringNode;
import jadx.core.dex.instructions.IfNode;
import jadx.core.dex.instructions.IndexInsnNode;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.InvokeNode;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.FieldArg;
import jadx.core.dex.instructions.args.InsnArg;
Expand Down Expand Up @@ -148,6 +142,15 @@ private static void simplifyTernary(TernaryInsn insn) {
}
}

/**
* Simplify chains of calls to StringBuilder#append() plus constructor of StringBuilder.
* Those chains are usually automatically generated by the Java compiler when you create String
* concatenations like <code>"text " + 1 + " text"</code>.
*
* @param mth
* @param insn
* @return
*/
private static InsnNode convertInvoke(MethodNode mth, InsnNode insn) {
MethodInfo callMth = ((InvokeNode) insn).getCallMth();

Expand Down Expand Up @@ -201,7 +204,15 @@ private static InsnNode convertInvoke(MethodNode mth, InsnNode insn) {
}

for (; argInd < len; argInd++) { // Add the .append(xxx) arg string to concat
concatInsn.addArg(chain.get(argInd).getArg(1));
InsnNode node = chain.get(argInd);
MethodInfo method = ((CallMthInterface) node).getCallMth();
if (!(node.getArgsCount() < 2 && method.isConstructor() || method.getName().equals("append"))) {
// The chain contains other calls to StringBuilder methods than the constructor or append.
// We can't simplify such chains, therefore we leave them as they are.
return null;
}
// process only constructor and append() calls
concatInsn.addArg(node.getArg(1));
}
concatInsn.setResult(insn.getResult());
return concatInsn;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package jadx.tests.integration;

import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.visitors.SimplifyVisitor;
import jadx.core.utils.exceptions.JadxException;
import jadx.tests.api.IntegrationTest;
import org.junit.Test;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;

/**
* Test the StringBuilder simplification part of {@link SimplifyVisitor}
*
* @author Jan Peter Stotz
*/
public class SimplifyVisitorStringBuilderTest extends IntegrationTest {

public static class TestCls1 {
public String test() {
return new StringBuilder("[init]").append("a1").append('c').append(2).append(0l).append(1.0f).
append(2.0d).append(true).toString();
}
}

@Test
public void test1() throws JadxException {
ClassNode cls = getClassNode(SimplifyVisitorStringBuilderTest.TestCls1.class);
SimplifyVisitor visitor = new SimplifyVisitor();
visitor.visit(cls);
String code = cls.getCode().toString();
assertThat(code, containsString("return \"[init]\" + \"a1\" + 'c' + 2 + 0 + 1.0f + 2.0d + true;"));
}

public static class TestCls2 {
public String test() {
// A chain with non-final variables
String sInit = "[init]";
String s = "a1";
char c = 'c';
int i = 1;
long l = 2;
float f = 1.0f;
double d = 2.0d;
boolean b = true;
return new StringBuilder(sInit).append(s).append(c).append(i).append(l).append(f).
append(d).append(b).toString();
}
}

@Test
public void test2() throws JadxException {
ClassNode cls = getClassNode(SimplifyVisitorStringBuilderTest.TestCls2.class);
SimplifyVisitor visitor = new SimplifyVisitor();
visitor.visit(cls);
String code = cls.getCode().toString();
assertThat(code, containsString("return \"[init]\" + \"a1\" + 'c' + 1 + 2 + 1.0f + 2.0d + true;"));
}

public static class TestClsStringUtilsReverse {

/**
* Simplified version of org.apache.commons.lang3.StringUtils.reverse()
*/
public static String reverse(final String str) {
return new StringBuilder(str).reverse().toString();
}
}

@Test
public void test3() throws JadxException {
ClassNode cls = getClassNode(SimplifyVisitorStringBuilderTest.TestClsStringUtilsReverse.class);
SimplifyVisitor visitor = new SimplifyVisitor();
visitor.visit(cls);
String code = cls.getCode().toString();
assertThat(code, containsString("return new StringBuilder(str).reverse().toString();"));
}

public static class TestClsChainWithDelete {
public String test() {
// a chain we can't simplify
return new StringBuilder("[init]").append("a1").delete(1, 2).toString();
}
}

@Test
public void testChainWithDelete() throws JadxException {
ClassNode cls = getClassNode(TestClsChainWithDelete.class);
SimplifyVisitor visitor = new SimplifyVisitor();
visitor.visit(cls);
String code = cls.getCode().toString();
assertThat(code, containsString("return new StringBuilder(\"[init]\").append(\"a1\").delete(1, 2).toString();"));
}
}

0 comments on commit 72b2663

Please sign in to comment.