From 4816a0f3405992627cab6b87b3fab371f3987b52 Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 21:36:05 +0200 Subject: [PATCH 1/8] Fix a regression of PR#171 - Correct usage of Type.getType(...) --- .../java/org/apache/bcel/generic/LDC.java | 2 +- .../java/org/apache/bcel/generic/Type.java | 17 ++++++ .../org/apache/bcel/generic/TypeTestCase.java | 58 ++++++++++++++++++- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/bcel/generic/LDC.java b/src/main/java/org/apache/bcel/generic/LDC.java index 85f1c7c219..e0cb408160 100644 --- a/src/main/java/org/apache/bcel/generic/LDC.java +++ b/src/main/java/org/apache/bcel/generic/LDC.java @@ -108,7 +108,7 @@ public Object getValue(final ConstantPoolGen cpg) { case org.apache.bcel.Const.CONSTANT_Class: final int nameIndex = ((org.apache.bcel.classfile.ConstantClass) c).getNameIndex(); c = cpg.getConstantPool().getConstant(nameIndex); - return Type.getType(((org.apache.bcel.classfile.ConstantUtf8) c).getBytes()); + return Type.getType(Type.internalTypeNametoSignature(((org.apache.bcel.classfile.ConstantUtf8) c).getBytes())); default: // Never reached throw new IllegalArgumentException("Unknown or invalid constant type at " + super.getIndex()); } diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index e8e4cc3103..293be54b6d 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -25,6 +25,7 @@ import org.apache.bcel.classfile.ClassFormatException; import org.apache.bcel.classfile.InvalidMethodSignatureException; import org.apache.bcel.classfile.Utility; +import org.apache.commons.lang3.Validate; /** * Abstract super class for all possible java types, namely basic types such as int, object types like String and array @@ -365,6 +366,22 @@ public int hashCode() { return type ^ signature.hashCode(); } + static String internalTypeNametoSignature(final String internalTypeName) { + Validate.notEmpty(internalTypeName, "Cannot call org.apache.bcel.generic.Type.internalTypeNametoSignature(String) on empty internalTypeName"); + switch(internalTypeName.charAt(0)) { + case '[': + return internalTypeName; + case 'L': + case 'T': + if (internalTypeName.charAt(internalTypeName.length() - 1) == ';') { + return internalTypeName; + } + // fall through + default: + return 'L' + internalTypeName + ';'; + } + } + /** * boolean, short and char variable are considered as int in the stack or local variable area. Returns {@link Type#INT} * for {@link Type#BOOLEAN}, {@link Type#SHORT} or {@link Type#CHAR}, otherwise returns the given type. diff --git a/src/test/java/org/apache/bcel/generic/TypeTestCase.java b/src/test/java/org/apache/bcel/generic/TypeTestCase.java index a2632151de..718d93322c 100644 --- a/src/test/java/org/apache/bcel/generic/TypeTestCase.java +++ b/src/test/java/org/apache/bcel/generic/TypeTestCase.java @@ -16,9 +16,16 @@ */ package org.apache.bcel.generic; -import static org.junit.jupiter.api.Assertions.assertEquals; - +import org.apache.bcel.Repository; +import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.JavaClass; +import org.apache.bcel.classfile.Method; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; public class TypeTestCase { @Test @@ -32,4 +39,51 @@ public void testBCEL243() { assertEquals(expectedValue, actualValue, "Type.getType"); } + @ParameterizedTest + @ValueSource(strings = { + // @formatter:off + "java/io/Externalizable", + "java/io/ObjectOutputStream", + "java/io/Serializable", + "java/lang/Cloneable", + "java/lang/RuntimeException", + "java/lang/String", + "java/lang/System", + "java/lang/Throwable", + "java/net/URI", + "java/sql/Statement", + "java/util/ArrayList", + "java/util/Calendar", + "java/util/EnumMap", + "java/util/HashSet", + "java/util/Iterator", + "java/util/LinkedList", + "java/util/List", + "java/util/Map", + "java/util/concurrent/ConcurrentMap", + "java/util/concurrent/ExecutorService", + "org/apache/bcel/classfile/JavaClass", + "org/apache/bcel/classfile/Method", + "org/apache/bcel/classfile/Synthetic", + "org/apache/bcel/generic/ConstantPoolGen", + "org/apache/bcel/generic/MethodGen"}) + // @formatter:on + public void testLDC(final String className) throws Exception { + final JavaClass jc = Repository.lookupClass(className); + final ConstantPoolGen cpg = new ConstantPoolGen(jc.getConstantPool()); + for (final Method method : jc.getMethods()) { + final Code code = method.getCode(); + if (code != null) { + final InstructionList instructionList = new InstructionList(code.getCode()); + for (final InstructionHandle instructionHandle : instructionList) { + instructionHandle.accept(new EmptyVisitor() { + @Override + public void visitLDC(LDC obj) { + assertNotNull(obj.getValue(cpg)); + } + }); + } + } + } + } } From c25de87746c6679b04ca25f3520b4d59362de97b Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 21:42:26 +0200 Subject: [PATCH 2/8] Remove fall through --- src/main/java/org/apache/bcel/generic/Type.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index 293be54b6d..a158674afc 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -376,7 +376,7 @@ static String internalTypeNametoSignature(final String internalTypeName) { if (internalTypeName.charAt(internalTypeName.length() - 1) == ';') { return internalTypeName; } - // fall through + return 'L' + internalTypeName + ';'; default: return 'L' + internalTypeName + ';'; } From 8fd3efddac088859c778f7bc7ddd26d1118b42af Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 22:19:51 +0200 Subject: [PATCH 3/8] added tests - do not throw --- src/main/java/org/apache/bcel/generic/Type.java | 8 +++++--- .../java/org/apache/bcel/generic/TypeTestCase.java | 12 ++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index a158674afc..00e2a1aca9 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -25,7 +25,7 @@ import org.apache.bcel.classfile.ClassFormatException; import org.apache.bcel.classfile.InvalidMethodSignatureException; import org.apache.bcel.classfile.Utility; -import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.StringUtils; /** * Abstract super class for all possible java types, namely basic types such as int, object types like String and array @@ -181,7 +181,7 @@ public static String getSignature(final java.lang.reflect.Method meth) { public static Type getType(final Class cls) { Objects.requireNonNull(cls, "cls"); /* - * That's an amzingly easy case, because getName() returns the signature. That's what we would have liked anyway. + * That's an amazingly easy case, because getName() returns the signature. That's what we would have liked anyway. */ if (cls.isArray()) { return getType(cls.getName()); @@ -367,7 +367,9 @@ public int hashCode() { } static String internalTypeNametoSignature(final String internalTypeName) { - Validate.notEmpty(internalTypeName, "Cannot call org.apache.bcel.generic.Type.internalTypeNametoSignature(String) on empty internalTypeName"); + if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, "B", "C", "D", "F", "I", "J", "S", "Z")) { + return internalTypeName; + } switch(internalTypeName.charAt(0)) { case '[': return internalTypeName; diff --git a/src/test/java/org/apache/bcel/generic/TypeTestCase.java b/src/test/java/org/apache/bcel/generic/TypeTestCase.java index 718d93322c..dc05df81ed 100644 --- a/src/test/java/org/apache/bcel/generic/TypeTestCase.java +++ b/src/test/java/org/apache/bcel/generic/TypeTestCase.java @@ -86,4 +86,16 @@ public void visitLDC(LDC obj) { } } } + + @Test + public void testInternalTypeNametoSignature() { + assertEquals(null, Type.internalTypeNametoSignature(null)); + assertEquals("", Type.internalTypeNametoSignature("")); + assertEquals("TT;", Type.internalTypeNametoSignature("TT;")); + assertEquals("Ljava/lang/String;", Type.internalTypeNametoSignature("Ljava/lang/String;")); + assertEquals("[Ljava/lang/String;", Type.internalTypeNametoSignature("[Ljava/lang/String;")); + assertEquals("Ljava/lang/String;", Type.internalTypeNametoSignature("java/lang/String")); + assertEquals("I", Type.internalTypeNametoSignature("I")); + assertEquals("LA;", Type.internalTypeNametoSignature("A")); + } } From e8f89f0aead138c86dee90e5bef1c255cff3d0a9 Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 22:28:23 +0200 Subject: [PATCH 4/8] fix coverage --- src/main/java/org/apache/bcel/generic/Type.java | 2 +- src/test/java/org/apache/bcel/generic/TypeTestCase.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index 00e2a1aca9..0ece04b702 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -367,7 +367,7 @@ public int hashCode() { } static String internalTypeNametoSignature(final String internalTypeName) { - if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, "B", "C", "D", "F", "I", "J", "S", "Z")) { + if (StringUtils.equalsAny(internalTypeName, "", "B", "C", "D", "F", "I", "J", "S", "Z")) { return internalTypeName; } switch(internalTypeName.charAt(0)) { diff --git a/src/test/java/org/apache/bcel/generic/TypeTestCase.java b/src/test/java/org/apache/bcel/generic/TypeTestCase.java index dc05df81ed..640ab73b61 100644 --- a/src/test/java/org/apache/bcel/generic/TypeTestCase.java +++ b/src/test/java/org/apache/bcel/generic/TypeTestCase.java @@ -96,6 +96,6 @@ public void testInternalTypeNametoSignature() { assertEquals("[Ljava/lang/String;", Type.internalTypeNametoSignature("[Ljava/lang/String;")); assertEquals("Ljava/lang/String;", Type.internalTypeNametoSignature("java/lang/String")); assertEquals("I", Type.internalTypeNametoSignature("I")); - assertEquals("LA;", Type.internalTypeNametoSignature("A")); + assertEquals("LT;", Type.internalTypeNametoSignature("T")); } } From 8ad6cc4cc26c216416a234e918d2039fce78e1fb Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 22:31:18 +0200 Subject: [PATCH 5/8] fix: use StringUtils.isEmpty(internalTypeName) --- src/main/java/org/apache/bcel/generic/Type.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index 0ece04b702..00e2a1aca9 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -367,7 +367,7 @@ public int hashCode() { } static String internalTypeNametoSignature(final String internalTypeName) { - if (StringUtils.equalsAny(internalTypeName, "", "B", "C", "D", "F", "I", "J", "S", "Z")) { + if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, "B", "C", "D", "F", "I", "J", "S", "Z")) { return internalTypeName; } switch(internalTypeName.charAt(0)) { From 19121c0f5504981e8d4b5ad68c672a9d65793646 Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 2 May 2023 22:48:05 +0200 Subject: [PATCH 6/8] Fix name of method to camel case: internalTypeNameToSignature --- src/main/java/org/apache/bcel/generic/LDC.java | 2 +- src/main/java/org/apache/bcel/generic/Type.java | 2 +- .../org/apache/bcel/generic/TypeTestCase.java | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/apache/bcel/generic/LDC.java b/src/main/java/org/apache/bcel/generic/LDC.java index e0cb408160..eba856e946 100644 --- a/src/main/java/org/apache/bcel/generic/LDC.java +++ b/src/main/java/org/apache/bcel/generic/LDC.java @@ -108,7 +108,7 @@ public Object getValue(final ConstantPoolGen cpg) { case org.apache.bcel.Const.CONSTANT_Class: final int nameIndex = ((org.apache.bcel.classfile.ConstantClass) c).getNameIndex(); c = cpg.getConstantPool().getConstant(nameIndex); - return Type.getType(Type.internalTypeNametoSignature(((org.apache.bcel.classfile.ConstantUtf8) c).getBytes())); + return Type.getType(Type.internalTypeNameToSignature(((org.apache.bcel.classfile.ConstantUtf8) c).getBytes())); default: // Never reached throw new IllegalArgumentException("Unknown or invalid constant type at " + super.getIndex()); } diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index 00e2a1aca9..7ed01fdaaa 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -366,7 +366,7 @@ public int hashCode() { return type ^ signature.hashCode(); } - static String internalTypeNametoSignature(final String internalTypeName) { + static String internalTypeNameToSignature(final String internalTypeName) { if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, "B", "C", "D", "F", "I", "J", "S", "Z")) { return internalTypeName; } diff --git a/src/test/java/org/apache/bcel/generic/TypeTestCase.java b/src/test/java/org/apache/bcel/generic/TypeTestCase.java index 640ab73b61..242e1004cb 100644 --- a/src/test/java/org/apache/bcel/generic/TypeTestCase.java +++ b/src/test/java/org/apache/bcel/generic/TypeTestCase.java @@ -89,13 +89,13 @@ public void visitLDC(LDC obj) { @Test public void testInternalTypeNametoSignature() { - assertEquals(null, Type.internalTypeNametoSignature(null)); - assertEquals("", Type.internalTypeNametoSignature("")); - assertEquals("TT;", Type.internalTypeNametoSignature("TT;")); - assertEquals("Ljava/lang/String;", Type.internalTypeNametoSignature("Ljava/lang/String;")); - assertEquals("[Ljava/lang/String;", Type.internalTypeNametoSignature("[Ljava/lang/String;")); - assertEquals("Ljava/lang/String;", Type.internalTypeNametoSignature("java/lang/String")); - assertEquals("I", Type.internalTypeNametoSignature("I")); - assertEquals("LT;", Type.internalTypeNametoSignature("T")); + assertEquals(null, Type.internalTypeNameToSignature(null)); + assertEquals("", Type.internalTypeNameToSignature("")); + assertEquals("TT;", Type.internalTypeNameToSignature("TT;")); + assertEquals("Ljava/lang/String;", Type.internalTypeNameToSignature("Ljava/lang/String;")); + assertEquals("[Ljava/lang/String;", Type.internalTypeNameToSignature("[Ljava/lang/String;")); + assertEquals("Ljava/lang/String;", Type.internalTypeNameToSignature("java/lang/String")); + assertEquals("I", Type.internalTypeNameToSignature("I")); + assertEquals("LT;", Type.internalTypeNameToSignature("T")); } } From c1e615d8b9838c887219b72a9b66f27a93f4e75e Mon Sep 17 00:00:00 2001 From: nbauma109 Date: Tue, 29 Aug 2023 21:58:43 +0200 Subject: [PATCH 7/8] Make Const.SHORT_TYPE_NAMES public for Type.internalTypeNameToSignature --- src/main/java/org/apache/bcel/Const.java | 2 +- src/main/java/org/apache/bcel/generic/Type.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/bcel/Const.java b/src/main/java/org/apache/bcel/Const.java index ba5acc9d71..1729403ea1 100644 --- a/src/main/java/org/apache/bcel/Const.java +++ b/src/main/java/org/apache/bcel/Const.java @@ -2834,7 +2834,7 @@ public final class Const { /** * The signature characters corresponding to primitive types, e.g., SHORT_TYPE_NAMES[T_INT] = "I" */ - private static final String[] SHORT_TYPE_NAMES = {ILLEGAL_TYPE, ILLEGAL_TYPE, ILLEGAL_TYPE, ILLEGAL_TYPE, "Z", "C", "F", "D", "B", "S", "I", "J", "V", + public static final String[] SHORT_TYPE_NAMES = {ILLEGAL_TYPE, ILLEGAL_TYPE, ILLEGAL_TYPE, ILLEGAL_TYPE, "Z", "C", "F", "D", "B", "S", "I", "J", "V", ILLEGAL_TYPE, ILLEGAL_TYPE, ILLEGAL_TYPE}; /** diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index 7ed01fdaaa..de8d7990cb 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -367,7 +367,7 @@ public int hashCode() { } static String internalTypeNameToSignature(final String internalTypeName) { - if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, "B", "C", "D", "F", "I", "J", "S", "Z")) { + if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, Const.SHORT_TYPE_NAMES)) { return internalTypeName; } switch(internalTypeName.charAt(0)) { From 7baad01732eea4a7b81b760e43fffc9f839b1452 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 15 Sep 2023 17:08:17 -0400 Subject: [PATCH 8/8] Checkstyle WhitespaceAround --- src/main/java/org/apache/bcel/generic/Type.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/bcel/generic/Type.java b/src/main/java/org/apache/bcel/generic/Type.java index de8d7990cb..34798c3482 100644 --- a/src/main/java/org/apache/bcel/generic/Type.java +++ b/src/main/java/org/apache/bcel/generic/Type.java @@ -370,7 +370,7 @@ static String internalTypeNameToSignature(final String internalTypeName) { if (StringUtils.isEmpty(internalTypeName) || StringUtils.equalsAny(internalTypeName, Const.SHORT_TYPE_NAMES)) { return internalTypeName; } - switch(internalTypeName.charAt(0)) { + switch (internalTypeName.charAt(0)) { case '[': return internalTypeName; case 'L':