This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
The following commit(s) were added to refs/heads/master by this push:
new 3e6dfa13 Variation on PR "Various bug fixes in the verifier #181"
3e6dfa13 is described below
commit 3e6dfa134dab153bb0f42cd1aef3004f464c48b8
Author: nbauma109 <nbauma...@github.com>
AuthorDate: Sun Apr 9 16:11:06 2023 -0400
Variation on PR "Various bug fixes in the verifier #181"
Original PR comment:
Field not found -> search field in both super class and implemented
interfaces (5x duplicated code to find field by name and type is
refactored to a new method and now supports package-private)
Invoke on array types (i.e. array.clone())
AssertionFailedError before reporting INVOKEDYNAMIC is unsupported
---
.../java/org/apache/bcel/classfile/JavaClass.java | 38 ++++++++++
.../bcel/verifier/statics/Pass3aVerifier.java | 87 ++++------------------
.../structurals/InstConstraintVisitor.java | 56 +-------------
.../org/apache/bcel/verifier/VerifierTestCase.java | 27 +++++++
.../verifier/input/FieldVerifierChildClass.java | 49 ++++++++++++
.../verifier/input/FieldVerifierSuperClass.java | 37 +++++++++
.../input/StaticFieldVerifierChildClass.java | 50 +++++++++++++
.../input/StaticFieldVerifierSuperClass.java | 38 ++++++++++
8 files changed, 258 insertions(+), 124 deletions(-)
diff --git a/src/main/java/org/apache/bcel/classfile/JavaClass.java
b/src/main/java/org/apache/bcel/classfile/JavaClass.java
index e5464051..a91b6513 100644
--- a/src/main/java/org/apache/bcel/classfile/JavaClass.java
+++ b/src/main/java/org/apache/bcel/classfile/JavaClass.java
@@ -394,6 +394,44 @@ public class JavaClass extends AccessFlags implements
Cloneable, Node, Comparabl
return bcelComparator.equals(this, obj);
}
+ /**
+ * Finds a visible field by name and type in this class and its super
classes.
+ *
+ * @param fieldName the field name to find
+ * @param fieldType the field type to find
+ * @return field matching given name and type, null if field is not found
or not accessible from this class.
+ * @throws ClassNotFoundException
+ * @since 6.8.0
+ */
+ public Field findField(final String fieldName, final Type fieldType)
throws ClassNotFoundException {
+ for (final Field field : fields) {
+ if (field.getName().equals(fieldName)) {
+ final Type fType = Type.getType(field.getSignature());
+ // TODO: Check if assignment compatibility is sufficient.
What does Sun do?
+ if (fType.equals(fieldType)) {
+ return field;
+ }
+ }
+ }
+ final JavaClass superclass = getSuperClass();
+ if (superclass != null &&
!"java.lang.Object".equals(superclass.getClassName())) {
+ final Field f = superclass.findField(fieldName, fieldType);
+ if (f != null && (f.isPublic() || f.isProtected() ||
!f.isPrivate() && packageName.equals(superclass.getPackageName()))) {
+ return f;
+ }
+ }
+ JavaClass[] implementedInterfaces = getInterfaces();
+ if (implementedInterfaces != null) {
+ for (JavaClass implementedInterface : implementedInterfaces) {
+ final Field f = implementedInterface.findField(fieldName,
fieldType);
+ if (f != null) {
+ return f;
+ }
+ }
+ }
+ return null;
+ }
+
/**
* Get all interfaces implemented by this JavaClass (transitively).
*
diff --git a/src/main/java/org/apache/bcel/verifier/statics/Pass3aVerifier.java
b/src/main/java/org/apache/bcel/verifier/statics/Pass3aVerifier.java
index 2d8db459..61e555d7 100644
--- a/src/main/java/org/apache/bcel/verifier/statics/Pass3aVerifier.java
+++ b/src/main/java/org/apache/bcel/verifier/statics/Pass3aVerifier.java
@@ -25,12 +25,14 @@ import org.apache.bcel.classfile.ClassFormatException;
import org.apache.bcel.classfile.Code;
import org.apache.bcel.classfile.CodeException;
import org.apache.bcel.classfile.Constant;
+import org.apache.bcel.classfile.ConstantCP;
import org.apache.bcel.classfile.ConstantClass;
import org.apache.bcel.classfile.ConstantDouble;
import org.apache.bcel.classfile.ConstantFieldref;
import org.apache.bcel.classfile.ConstantFloat;
import org.apache.bcel.classfile.ConstantInteger;
import org.apache.bcel.classfile.ConstantInterfaceMethodref;
+import org.apache.bcel.classfile.ConstantInvokeDynamic;
import org.apache.bcel.classfile.ConstantLong;
import org.apache.bcel.classfile.ConstantMethodref;
import org.apache.bcel.classfile.ConstantNameAndType;
@@ -320,59 +322,9 @@ public final class Pass3aVerifier extends PassVerifier {
final String fieldName = o.getFieldName(constantPoolGen);
final JavaClass jc =
Repository.lookupClass(getObjectType(o).getClassName());
- Field[] fields = jc.getFields();
- Field f = null;
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- final Type fType = Type.getType(field.getSignature());
- final Type oType = o.getType(constantPoolGen);
- /*
- * TODO: Check if assignment compatibility is
sufficient. What does Sun do?
- */
- if (fType.equals(oType)) {
- f = field;
- break;
- }
- }
- }
+ final Field f = jc.findField(fieldName,
o.getType(constantPoolGen));
if (f == null) {
- final JavaClass[] superclasses = jc.getSuperClasses();
- outer: for (final JavaClass superclass : superclasses) {
- fields = superclass.getFields();
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- final Type fType =
Type.getType(field.getSignature());
- final Type oType = o.getType(constantPoolGen);
- if (fType.equals(oType)) {
- f = field;
- if ((f.getAccessFlags() &
(Const.ACC_PUBLIC | Const.ACC_PROTECTED)) == 0) {
- f = null;
- }
- break outer;
- }
- }
- }
- }
- if (f == null) {
- constraintViolated(o, "Referenced field '" + fieldName
+ "' does not exist in class '" + jc.getClassName() + "'.");
- }
- } else {
- /*
- * TODO: Check if assignment compatibility is sufficient.
What does Sun do?
- */
- Type.getType(f.getSignature());
- o.getType(constantPoolGen);
-// Type f_type = Type.getType(f.getSignature());
-// Type o_type = o.getType(cpg);
-
- // Argh. Sun's implementation allows us to have multiple
fields of
- // the same name but with a different signature.
- // if (! f_type.equals(o_type)) {
- // constraintViolated(o,
- // "Referenced field '"+field_name+"' has type
'"+f_type+"' instead of '"+o_type+"' as expected.");
- // }
-
- /* TODO: Check for access modifiers here. */
+ constraintViolated(o, "Referenced field '" + fieldName +
"' does not exist in class '" + jc.getClassName() + "'.");
}
} catch (final ClassNotFoundException e) {
// FIXME: maybe not the best way to handle this
@@ -414,14 +366,7 @@ public final class Pass3aVerifier extends PassVerifier {
try {
final String fieldName = o.getFieldName(constantPoolGen);
final JavaClass jc =
Repository.lookupClass(getObjectType(o).getClassName());
- final Field[] fields = jc.getFields();
- Field f = null;
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- f = field;
- break;
- }
- }
+ final Field f = jc.findField(fieldName,
o.getType(constantPoolGen));
if (f == null) {
throw new AssertionViolatedException("Field '" + fieldName
+ "' not found in " + jc.getClassName());
}
@@ -501,8 +446,8 @@ public final class Pass3aVerifier extends PassVerifier {
}
} else {
final Constant c = constantPoolGen.getConstant(o.getIndex());
- if (!(c instanceof ConstantInterfaceMethodref)) {
- constraintViolated(o, "Indexing a constant that's not a
CONSTANT_InterfaceMethodref but a '" + tostring(c) + "'.");
+ if (!(c instanceof ConstantInterfaceMethodref) && !(c
instanceof ConstantInvokeDynamic)) {
+ constraintViolated(o, "Indexing a constant that's not a
CONSTANT_InterfaceMethodref/InvokeDynamic but a '" + tostring(c) + "'.");
}
// TODO: From time to time check if BCEL allows to detect if
the
// 'count' operand is consistent with the information in the
@@ -510,7 +455,7 @@ public final class Pass3aVerifier extends PassVerifier {
// By now, BCEL hides those two operands because they're
superfluous.
// Invoked method must not be <init> or <clinit>
- final ConstantNameAndType cnat = (ConstantNameAndType)
constantPoolGen.getConstant(((ConstantInterfaceMethodref)
c).getNameAndTypeIndex());
+ final ConstantNameAndType cnat = (ConstantNameAndType)
constantPoolGen.getConstant(((ConstantCP) c).getNameAndTypeIndex());
final String name = ((ConstantUtf8)
constantPoolGen.getConstant(cnat.getNameIndex())).getBytes();
if (name.equals(Const.CONSTRUCTOR_NAME)) {
constraintViolated(o, "Method to invoke must not be '" +
Const.CONSTRUCTOR_NAME + "'.");
@@ -662,7 +607,12 @@ public final class Pass3aVerifier extends PassVerifier {
// INVOKEVIRTUAL is an InvokeInstruction, the argument and
return types are resolved/verified,
// too. So are the allowed method names.
final String className = o.getClassName(constantPoolGen);
- final JavaClass jc = Repository.lookupClass(className);
+ JavaClass jc;
+ if (className.charAt(0) == '[') { // array type, e.g. invoke
can be someArray.clone()
+ jc = Repository.lookupClass("java.lang.Object");
+ } else {
+ jc = Repository.lookupClass(className);
+ }
final Method m = getMethodRecursive(jc, o);
if (m == null) {
constraintViolated(o, "Referenced method '" +
o.getMethodName(constantPoolGen) + "' with expected signature '"
@@ -857,14 +807,7 @@ public final class Pass3aVerifier extends PassVerifier {
try {
final String fieldName = o.getFieldName(constantPoolGen);
final JavaClass jc =
Repository.lookupClass(getObjectType(o).getClassName());
- final Field[] fields = jc.getFields();
- Field f = null;
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- f = field;
- break;
- }
- }
+ final Field f = jc.findField(fieldName,
o.getType(constantPoolGen));
if (f == null) {
throw new AssertionViolatedException("Field '" + fieldName
+ "' not found in " + jc.getClassName());
}
diff --git
a/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
b/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
index 1c92576d..ec6d6743 100644
---
a/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
+++
b/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
@@ -918,21 +918,7 @@ public class InstConstraintVisitor extends EmptyVisitor {
private Field visitFieldInstructionInternals(final FieldInstruction o)
throws ClassNotFoundException {
final String fieldName = o.getFieldName(cpg);
final JavaClass jc =
Repository.lookupClass(getObjectType(o).getClassName());
- final Field[] fields = jc.getFields();
- Field f = null;
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- final Type fType = Type.getType(field.getSignature());
- final Type oType = o.getType(cpg);
- /*
- * TODO: Check if assignment compatibility is sufficient. What
does Sun do?
- */
- if (fType.equals(oType)) {
- f = field;
- break;
- }
- }
- }
+ final Field f = jc.findField(fieldName, o.getType(cpg));
if (f == null) {
throw new AssertionViolatedException("Field '" + fieldName + "'
not found in " + jc.getClassName());
}
@@ -1054,43 +1040,9 @@ public class InstConstraintVisitor extends EmptyVisitor {
final String fieldName = o.getFieldName(cpg);
final JavaClass jc =
Repository.lookupClass(getObjectType(o).getClassName());
- Field[] fields = jc.getFields();
- Field f = null;
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- final Type fType = Type.getType(field.getSignature());
- final Type oType = o.getType(cpg);
- /*
- * TODO: Check if assignment compatibility is sufficient.
What does Sun do?
- */
- if (fType.equals(oType)) {
- f = field;
- break;
- }
- }
- }
-
+ final Field f = jc.findField(fieldName, o.getType(cpg));
if (f == null) {
- final JavaClass[] superclasses = jc.getSuperClasses();
- outer: for (final JavaClass superclass : superclasses) {
- fields = superclass.getFields();
- for (final Field field : fields) {
- if (field.getName().equals(fieldName)) {
- final Type fType =
Type.getType(field.getSignature());
- final Type oType = o.getType(cpg);
- if (fType.equals(oType)) {
- f = field;
- if ((f.getAccessFlags() & (Const.ACC_PUBLIC |
Const.ACC_PROTECTED)) == 0) {
- f = null;
- }
- break outer;
- }
- }
- }
- }
- if (f == null) {
- throw new AssertionViolatedException("Field '" + fieldName
+ "' not found in " + jc.getClassName());
- }
+ throw new AssertionViolatedException("Field '" + fieldName +
"' not found in " + jc.getClassName());
}
if (f.isProtected()) {
@@ -1818,7 +1770,7 @@ public class InstConstraintVisitor extends EmptyVisitor {
final String theClass = o.getClassName(cpg);
- if (!Repository.instanceOf(objRefClassName, theClass)) {
+ if (objref != GENERIC_ARRAY &&
!Repository.instanceOf(objRefClassName, theClass)) {
constraintViolated(o, "The 'objref' item '" + objref + "' does
not implement '" + theClass + "' as expected.");
}
} catch (final ClassNotFoundException e) {
diff --git a/src/test/java/org/apache/bcel/verifier/VerifierTestCase.java
b/src/test/java/org/apache/bcel/verifier/VerifierTestCase.java
index ec04b9ec..b17105a0 100644
--- a/src/test/java/org/apache/bcel/verifier/VerifierTestCase.java
+++ b/src/test/java/org/apache/bcel/verifier/VerifierTestCase.java
@@ -20,6 +20,7 @@ package org.apache.bcel.verifier;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.File;
@@ -33,10 +34,14 @@ import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.NestHost;
import org.apache.bcel.classfile.Utility;
import org.apache.bcel.verifier.exc.AssertionViolatedException;
+import org.apache.bcel.verifier.input.FieldVerifierChildClass;
+import org.apache.bcel.verifier.input.StaticFieldVerifierChildClass;
import org.apache.bcel.verifier.statics.StringRepresentation;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledForJreRange;
+import org.junit.jupiter.api.condition.JRE;
public class VerifierTestCase {
@@ -106,6 +111,16 @@ public class VerifierTestCase {
VerifierFactory.clear();
}
+ @Test
+ public void testPackagePrivateField() throws ClassNotFoundException {
+ testDefaultMethodValidation(FieldVerifierChildClass.class.getName());
+ }
+
+ @Test
+ public void testPackagePrivateStaticField() throws ClassNotFoundException {
+
testDefaultMethodValidation(StaticFieldVerifierChildClass.class.getName());
+ }
+
@Test
public void testArrayUtils() throws ClassNotFoundException {
testNestHostWithJavaVersion("org.apache.commons.lang.ArrayUtils");
@@ -126,6 +141,18 @@ public class VerifierTestCase {
testNestHostWithJavaVersion("com.ibm.wsdl.DefinitionImpl");
}
+ @Test
+ @DisabledForJreRange(min = JRE.JAVA_9)
+ public void testObjectInputStreamJDK8() {
+ assertThrowsExactly(UnsupportedOperationException.class, () ->
testNestHostWithJavaVersion("java.io.ObjectInputStream"));
+ }
+
+ @Test
+ @DisabledForJreRange(max = JRE.JAVA_8)
+ public void testObjectInputStream() throws ClassNotFoundException {
+ testNestHostWithJavaVersion("java.io.ObjectInputStream");
+ }
+
@Test
public void testJvmOpCodes() throws ClassNotFoundException {
testDefaultMethodValidation("org.apache.bcel.verifier.tests.JvmOpCodes");
diff --git
a/src/test/java/org/apache/bcel/verifier/input/FieldVerifierChildClass.java
b/src/test/java/org/apache/bcel/verifier/input/FieldVerifierChildClass.java
new file mode 100644
index 00000000..99c020ce
--- /dev/null
+++ b/src/test/java/org/apache/bcel/verifier/input/FieldVerifierChildClass.java
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.bcel.verifier.input;
+
+public class FieldVerifierChildClass extends FieldVerifierSuperClass {
+
+ public FieldVerifierChildClass(final FieldVerifierChildClass c) {
+ super(c.publicField, c.getPrivateField(), c.protectedField,
c.packagePrivateField);
+ }
+
+ public int getPublicField() {
+ return publicField;
+ }
+
+ public void setPublicField(final int publicField) {
+ this.publicField = publicField;
+ }
+
+ public int getProtectedField() {
+ return protectedField;
+ }
+
+ public void setProtectedField(final int protectedField) {
+ this.protectedField = protectedField;
+ }
+
+ public int getPackagePrivateField() {
+ return packagePrivateField;
+ }
+
+ public void setPackagePrivateField(final int packagePrivateField) {
+ this.packagePrivateField = packagePrivateField;
+ }
+}
diff --git
a/src/test/java/org/apache/bcel/verifier/input/FieldVerifierSuperClass.java
b/src/test/java/org/apache/bcel/verifier/input/FieldVerifierSuperClass.java
new file mode 100644
index 00000000..be336897
--- /dev/null
+++ b/src/test/java/org/apache/bcel/verifier/input/FieldVerifierSuperClass.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.bcel.verifier.input;
+
+public class FieldVerifierSuperClass {
+
+ public int publicField;
+ private final int privateField;
+ protected int protectedField;
+ int packagePrivateField;
+
+ public FieldVerifierSuperClass(final int publicField, final int
privateField, final int protectedField, final int packagePrivateField) {
+ this.publicField = publicField;
+ this.privateField = privateField;
+ this.protectedField = protectedField;
+ this.packagePrivateField = packagePrivateField;
+ }
+
+ public int getPrivateField() {
+ return privateField;
+ }
+}
diff --git
a/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierChildClass.java
b/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierChildClass.java
new file mode 100644
index 00000000..6c804f08
--- /dev/null
+++
b/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierChildClass.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.bcel.verifier.input;
+
+public class StaticFieldVerifierChildClass extends
StaticFieldVerifierSuperClass {
+
+ @SuppressWarnings("static-access")
+ public StaticFieldVerifierChildClass(final StaticFieldVerifierChildClass
c) {
+ super(c.publicField, c.getPrivateField(), c.protectedField,
c.packagePrivateField);
+ }
+
+ public static int getPublicField() {
+ return publicField;
+ }
+
+ public static void setPublicField(final int publicField) {
+ StaticFieldVerifierChildClass.publicField = publicField;
+ }
+
+ public static int getProtectedField() {
+ return protectedField;
+ }
+
+ public static void setProtectedField(final int protectedField) {
+ StaticFieldVerifierChildClass.protectedField = protectedField;
+ }
+
+ public static int getPackagePrivateField() {
+ return packagePrivateField;
+ }
+
+ public static void setPackagePrivateField(final int packagePrivateField) {
+ StaticFieldVerifierChildClass.packagePrivateField =
packagePrivateField;
+ }
+}
diff --git
a/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierSuperClass.java
b/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierSuperClass.java
new file mode 100644
index 00000000..88eb7839
--- /dev/null
+++
b/src/test/java/org/apache/bcel/verifier/input/StaticFieldVerifierSuperClass.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.bcel.verifier.input;
+
+public class StaticFieldVerifierSuperClass {
+
+ public static int publicField;
+ private static int privateField;
+ protected static int protectedField;
+ static int packagePrivateField;
+
+ @SuppressWarnings("static-access")
+ public StaticFieldVerifierSuperClass(final int publicField, final int
privateField, final int protectedField, final int packagePrivateField) {
+ this.publicField = publicField;
+ this.privateField = privateField;
+ this.protectedField = protectedField;
+ this.packagePrivateField = packagePrivateField;
+ }
+
+ public int getPrivateField() {
+ return privateField;
+ }
+}
