diff --git a/src/java.base/share/classes/java/lang/Class.java b/src/java.base/share/classes/java/lang/Class.java index cfd2fc82235..f96ad9ff972 100644 --- a/src/java.base/share/classes/java/lang/Class.java +++ b/src/java.base/share/classes/java/lang/Class.java @@ -43,6 +43,7 @@ import java.lang.reflect.Executable; import java.lang.reflect.Field; import java.lang.reflect.GenericArrayType; import java.lang.reflect.GenericDeclaration; +import java.lang.reflect.GenericSignatureFormatError; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Member; import java.lang.reflect.Method; @@ -1445,7 +1446,6 @@ public final class Class implements java.io.Serializable, if (!enclosingInfo.isMethod()) return null; - // Descriptor already validated by VM List> types = BytecodeDescriptor.parseMethod(enclosingInfo.getDescriptor(), getClassLoader()); Class returnType = types.removeLast(); Class[] parameterClasses = types.toArray(EMPTY_CLASS_ARRAY); @@ -1533,8 +1533,15 @@ public final class Class implements java.io.Serializable, String getName() { return name; } - String getDescriptor() { return descriptor; } - + String getDescriptor() { + // hotspot validates this descriptor to be either a field or method + // descriptor as the "type" in a NameAndType in verification. + // So this can still be a field descriptor + if (descriptor.isEmpty() || descriptor.charAt(0) != '(') { + throw new GenericSignatureFormatError("Bad method signature: " + descriptor); + } + return descriptor; + } } private static Class toClass(Type o) { @@ -1567,7 +1574,6 @@ public final class Class implements java.io.Serializable, if (!enclosingInfo.isConstructor()) return null; - // Descriptor already validated by VM List> types = BytecodeDescriptor.parseMethod(enclosingInfo.getDescriptor(), getClassLoader()); types.removeLast(); Class[] parameterClasses = types.toArray(EMPTY_CLASS_ARRAY); diff --git a/src/java.base/share/classes/java/lang/invoke/MethodType.java b/src/java.base/share/classes/java/lang/invoke/MethodType.java index 3d15ce68710..afbe63709fd 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodType.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java @@ -1194,10 +1194,6 @@ class MethodType static MethodType fromDescriptor(String descriptor, ClassLoader loader) throws IllegalArgumentException, TypeNotPresentException { - if (!descriptor.startsWith("(") || // also generates NPE if needed - descriptor.indexOf(')') < 0 || - descriptor.indexOf('.') >= 0) - throw newIllegalArgumentException("not a method descriptor: "+descriptor); List> types = BytecodeDescriptor.parseMethod(descriptor, loader); Class rtype = types.remove(types.size() - 1); Class[] ptypes = listToArray(types); diff --git a/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java b/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java index bd5ea6d7635..b3671d19333 100644 --- a/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java +++ b/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java @@ -63,33 +63,24 @@ public class BytecodeDescriptor { /// @throws TypeNotPresentException if a reference type cannot be found by /// the loader (before the descriptor is found invalid) public static List> parseMethod(String descriptor, ClassLoader loader) { - return parseMethod(descriptor, 0, descriptor.length(), loader); - } - - /** - * @param loader the class loader in which to look up the types (null means - * bootstrap class loader) - */ - static List> parseMethod(String bytecodeSignature, - int start, int end, ClassLoader loader) { - String str = bytecodeSignature; - int[] i = {start}; + int end = descriptor.length(); // implicit null check + int[] i = {0}; var ptypes = new ArrayList>(); - if (i[0] < end && str.charAt(i[0]) == '(') { + if (i[0] < end && descriptor.charAt(i[0]) == '(') { ++i[0]; // skip '(' - while (i[0] < end && str.charAt(i[0]) != ')') { - Class pt = parseSig(str, i, end, loader); + while (i[0] < end && descriptor.charAt(i[0]) != ')') { + Class pt = parseSig(descriptor, i, end, loader); if (pt == null || pt == void.class) - parseError(str, "bad argument type"); + parseError(descriptor, "bad argument type"); ptypes.add(pt); } ++i[0]; // skip ')' } else { - parseError(str, "not a method type"); + parseError(descriptor, "not a method type"); } - Class rtype = parseSig(str, i, end, loader); + Class rtype = parseSig(descriptor, i, end, loader); if (rtype == null || i[0] != end) - parseError(str, "bad return type"); + parseError(descriptor, "bad return type"); ptypes.add(rtype); return ptypes; } @@ -115,8 +106,22 @@ public class BytecodeDescriptor { if (i[0] == end) return null; char c = str.charAt(i[0]++); if (c == 'L') { - int begc = i[0], endc = str.indexOf(';', begc); - if (endc < 0) return null; + int begc = i[0]; + int identifierStart = begc; + int endc; + while (true) { + int next = nextNonIdentifier(str, identifierStart, end); + if (identifierStart == next || next >= end) return null; // Empty name segment, or the end + char ch = str.charAt(next); + if (ch == ';') { + endc = next; + break; + } else if (ch == '/') { + identifierStart = next + 1; // Next name segment + } else { + return null; // Bad char [ or . + } + } i[0] = endc+1; String name = str.substring(begc, endc).replace('/', '.'); try { @@ -148,6 +153,23 @@ public class BytecodeDescriptor { } } + private static final int CHECK_OFFSET = 32; + private static final long NON_IDENTIFIER_MASK = (1L << ('.' - CHECK_OFFSET)) + | (1L << ('/' - CHECK_OFFSET)) + | (1L << (';' - CHECK_OFFSET)) + | (1L << ('[' - CHECK_OFFSET)); + + private static int nextNonIdentifier(String str, int index, int end) { + while (index < end) { + int check = str.charAt(index) - CHECK_OFFSET; + if ((check & -Long.SIZE) == 0 && (NON_IDENTIFIER_MASK & (1L << check)) != 0) { + break; + } + index++; + } + return index; + } + public static String unparse(Class type) { if (type == Object.class) { return "Ljava/lang/Object;"; diff --git a/test/jdk/java/lang/Class/getEnclosingMethod/BadEnclosingMethodTest.java b/test/jdk/java/lang/Class/getEnclosingMethod/BadEnclosingMethodTest.java index f673d01c835..dea1141d542 100644 --- a/test/jdk/java/lang/Class/getEnclosingMethod/BadEnclosingMethodTest.java +++ b/test/jdk/java/lang/Class/getEnclosingMethod/BadEnclosingMethodTest.java @@ -34,6 +34,7 @@ import org.junit.jupiter.api.Test; import java.lang.classfile.ClassFile; import java.lang.classfile.attribute.EnclosingMethodAttribute; +import java.lang.reflect.GenericSignatureFormatError; import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; @@ -93,8 +94,16 @@ class BadEnclosingMethodTest { */ @Test void testMalformedTypes() throws Exception { - assertThrows(ClassFormatError.class, () -> loadTestClass("methodName", "(L[;)V")); - assertThrows(ClassFormatError.class, () -> loadTestClass(INIT_NAME, "(L[;)V")); + assertThrowsExactly(ClassFormatError.class, () -> loadTestClass("methodName", "(L[;)V")); + assertThrowsExactly(ClassFormatError.class, () -> loadTestClass(INIT_NAME, "(L[;)V")); + assertThrowsExactly(ClassFormatError.class, () -> loadTestClass(INIT_NAME, "(Lmissing/;)V")); + assertThrowsExactly(ClassFormatError.class, () -> loadTestClass(INIT_NAME, "()L/Missing;")); + // only throw if the query type match the method/constructor type + assertDoesNotThrow(() -> loadTestClass(INIT_NAME, "Ljava/lang/Object;").getEnclosingMethod()); + assertDoesNotThrow(() -> loadTestClass("method", "[I").getEnclosingConstructor()); + // We have to manually intercept field-typed "methods" + assertThrows(GenericSignatureFormatError.class, () -> loadTestClass(INIT_NAME, "Ljava/lang/Object;").getEnclosingConstructor()); + assertThrows(GenericSignatureFormatError.class, () -> loadTestClass("method", "[I").getEnclosingMethod()); } /** diff --git a/test/jdk/java/lang/annotation/MalformedAnnotationTest.java b/test/jdk/java/lang/annotation/MalformedAnnotationTest.java index e7096000fb1..513deb8c7e9 100644 --- a/test/jdk/java/lang/annotation/MalformedAnnotationTest.java +++ b/test/jdk/java/lang/annotation/MalformedAnnotationTest.java @@ -31,6 +31,9 @@ import jdk.test.lib.ByteCodeLoader; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -41,6 +44,8 @@ import java.lang.classfile.ClassFile; import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; import java.lang.constant.ClassDesc; import java.lang.reflect.GenericSignatureFormatError; +import java.util.Arrays; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -56,22 +61,58 @@ class MalformedAnnotationTest { Class value(); } + static Stream badFieldDescriptors() { + return Arrays.stream(new String[] { + "Not a_descriptor", + "()V", + "Ljava/lang/Object", + "Ljava/", + "Ljava/util/Map.Entry;", + "[".repeat(256) + "I", + "Lbad.Name;", + "Lbad[Name;", + "L;", + "L/Missing;", + "Lmissing/;", + }); + } + /** * Ensures bad class descriptors in annotations lead to * {@link GenericSignatureFormatError} and the error message contains the * malformed descriptor string. */ - @Test - void testMalformedClassValue() throws Exception { - var badDescString = "Not a_descriptor"; + @ParameterizedTest + @MethodSource("badFieldDescriptors") + void testMalformedClassValue(String badDescString) throws Exception { + var cl = spinClass(badDescString); + var ex = assertThrows(GenericSignatureFormatError.class, () -> cl.getDeclaredAnnotation(ClassCarrier.class)); + assertTrue(ex.getMessage().contains(badDescString), () -> "Uninformative error: " + ex); + } + + private static Class spinClass(String desc) throws Exception { var bytes = ClassFile.of().build(ClassDesc.of("Test"), clb -> clb .with(RuntimeVisibleAnnotationsAttribute.of( Annotation.of(ClassCarrier.class.describeConstable().orElseThrow(), AnnotationElement.of("value", AnnotationValue.ofClass(clb - .constantPool().utf8Entry(badDescString)))) + .constantPool().utf8Entry(desc)))) ))); - var cl = new ByteCodeLoader("Test", bytes, MalformedAnnotationTest.class.getClassLoader()).loadClass("Test"); - var ex = assertThrows(GenericSignatureFormatError.class, () -> cl.getDeclaredAnnotation(ClassCarrier.class)); - assertTrue(ex.getMessage().contains(badDescString), () -> "Uninformative error: " + ex); + return new ByteCodeLoader("Test", bytes, ClassCarrier.class.getClassLoader()).loadClass("Test"); + } + + static Stream goodFieldDescriptors() { + return Arrays.stream(new String[] { + "Ljava/lang/Object<*>;", // previously MalformedParameterizedTypeException + "[Ljava/util/Optional<*>;", // previously ClassCastException + "Ljava/util/Map$Entry<**>;", // previously ClassCastException + }); + } + + @ParameterizedTest + @MethodSource("goodFieldDescriptors") + void testLegalClassValue(String goodDescString) throws Exception { + var cl = spinClass(goodDescString); + var anno = cl.getDeclaredAnnotation(ClassCarrier.class); + assertThrows(TypeNotPresentException.class, anno::value); } } diff --git a/test/jdk/java/lang/invoke/MethodTypeTest.java b/test/jdk/java/lang/invoke/MethodTypeTest.java index 70949236066..8ac03d1a7fd 100644 --- a/test/jdk/java/lang/invoke/MethodTypeTest.java +++ b/test/jdk/java/lang/invoke/MethodTypeTest.java @@ -230,6 +230,11 @@ public class MethodTypeTest { "(" + "[".repeat(256) + "J)I", "(java/lang/Object)V", "()java/lang/Object", + "()Lbad.Name;", + "()Lbad[Name;", + "(L;)V", + "(L/Missing;)I", + "(Lmissing/;)Z", }; } diff --git a/test/jdk/sun/invoke/util/BytecodeDescriptorTest.java b/test/jdk/sun/invoke/util/BytecodeDescriptorTest.java index fa99c20e644..2d07b08dcb7 100644 --- a/test/jdk/sun/invoke/util/BytecodeDescriptorTest.java +++ b/test/jdk/sun/invoke/util/BytecodeDescriptorTest.java @@ -79,7 +79,13 @@ class BytecodeDescriptorTest { assertSame(int.class, BytecodeDescriptor.parseClass("I", null), "primitive"); assertSame(long[][].class, BytecodeDescriptor.parseClass("[[J", null), "array"); assertSame(Object.class, BytecodeDescriptor.parseClass("Ljava/lang/Object;", null), "class or interface"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("java.lang.Object", null), "binary name"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("L[a;", null), "bad class or interface"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("Ljava.lang.Object;", null), "bad class or interface"); assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("java/lang/Object", null), "internal name"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("L;", null), "empty name"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("Lmissing/;", null), "empty name part"); + assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("L/Missing;", null), "empty name part"); assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("[V", null), "bad array"); assertSame(Class.forName("[".repeat(255) + "I"), BytecodeDescriptor.parseClass("[".repeat(255) + "I", null), "good array"); assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("[".repeat(256) + "I", null), "bad array"); @@ -110,6 +116,21 @@ class BytecodeDescriptorTest { assertThrows(IllegalArgumentException.class, () -> BytecodeDescriptor.parseClass("([".repeat(256) + "I)J", null), "bad arg"); + assertThrows(IllegalArgumentException.class, + () -> BytecodeDescriptor.parseMethod("(Ljava.lang.Object;)V", null), + "bad arg"); + assertThrows(IllegalArgumentException.class, + () -> BytecodeDescriptor.parseMethod("(Ljava/lang[Object;)V", null), + "bad arg"); + assertThrows(IllegalArgumentException.class, + () -> BytecodeDescriptor.parseMethod("(L;)V", null), + "bad arg"); + assertThrows(IllegalArgumentException.class, + () -> BytecodeDescriptor.parseMethod("(Ljava/lang/;)V", null), + "bad arg"); + assertThrows(IllegalArgumentException.class, + () -> BytecodeDescriptor.parseMethod("(L/Object;)V", null), + "bad arg"); assertEquals(List.of(foo1, bar1), BytecodeDescriptor.parseMethod("(" + FOO_DESC + ")" + BAR_DESC, cl1),