8023278: Reflection API methods do not throw AnnotationFormatError in case of...

8023278: Reflection API methods do not throw AnnotationFormatError in case of malformed Runtime[In]VisibleTypeAnnotations attribute
Reviewed-by: darcy

src/share/classes/sun/reflect/annotation/TypeAnnotation.java

@@ -147,13 +147,13 @@ public final class TypeAnnotation {
         public static final LocationInfo BASE_LOCATION = new LocationInfo();
 
         public static LocationInfo parseLocationInfo(ByteBuffer buf) {
-            int depth = buf.get();
+            int depth = buf.get() & 0xFF;
             if (depth == 0)
                 return BASE_LOCATION;
             Location[] locations = new Location[depth];
             for (int i = 0; i < depth; i++) {
                 byte tag = buf.get();
-                byte index = buf.get();
+                short index = (short)(buf.get() & 0xFF);
                 if (!(tag == 0 || tag == 1 | tag == 2 || tag == 3))
                     throw new AnnotationFormatError("Bad Location encoding in Type Annotation");
                 if (tag != 3 && index != 0)
@@ -164,26 +164,26 @@ public final class TypeAnnotation {
         }
 
         public LocationInfo pushArray() {
-            return pushLocation((byte)0, (byte)0);
+            return pushLocation((byte)0, (short)0);
         }
 
         public LocationInfo pushInner() {
-            return pushLocation((byte)1, (byte)0);
+            return pushLocation((byte)1, (short)0);
         }
 
         public LocationInfo pushWildcard() {
-            return pushLocation((byte) 2, (byte) 0);
+            return pushLocation((byte) 2, (short) 0);
         }
 
-        public LocationInfo pushTypeArg(byte index) {
+        public LocationInfo pushTypeArg(short index) {
             return pushLocation((byte) 3, index);
         }
 
-        public LocationInfo pushLocation(byte tag, byte index) {
+        public LocationInfo pushLocation(byte tag, short index) {
             int newDepth = this.depth + 1;
             Location[] res = new Location[newDepth];
             System.arraycopy(this.locations, 0, res, 0, depth);
-            res[newDepth - 1] = new Location(tag, index);
+            res[newDepth - 1] = new Location(tag, (short)(index & 0xFF));
             return new LocationInfo(newDepth, res);
         }
 
@@ -207,13 +207,13 @@ public final class TypeAnnotation {
 
         public static final class Location {
             public final byte tag;
-            public final byte index;
+            public final short index;
 
             boolean isSameLocation(Location other) {
                 return tag == other.tag && index == other.index;
             }
 
-            public Location(byte tag, byte index) {
+            public Location(byte tag, short index) {
                 this.tag = tag;
                 this.index = index;
             }

src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java

@@ -394,20 +394,25 @@ public final class TypeAnnotationParser {
             ConstantPool cp,
             AnnotatedElement baseDecl,
             Class<?> container) {
-        TypeAnnotationTargetInfo ti = parseTargetInfo(buf);
-        LocationInfo locationInfo = LocationInfo.parseLocationInfo(buf);
-        Annotation a = AnnotationParser.parseAnnotation(buf, cp, container, false);
-        if (ti == null) // Inside a method for example
-            return null;
-        return new TypeAnnotation(ti, locationInfo, a, baseDecl);
+        try {
+            TypeAnnotationTargetInfo ti = parseTargetInfo(buf);
+            LocationInfo locationInfo = LocationInfo.parseLocationInfo(buf);
+            Annotation a = AnnotationParser.parseAnnotation(buf, cp, container, false);
+            if (ti == null) // Inside a method for example
+                return null;
+            return new TypeAnnotation(ti, locationInfo, a, baseDecl);
+        } catch (IllegalArgumentException | // Bad type in const pool at specified index
+                BufferUnderflowException e) {
+            throw new AnnotationFormatError(e);
+        }
     }
 
     private static TypeAnnotationTargetInfo parseTargetInfo(ByteBuffer buf) {
-        byte posCode = buf.get();
+        int posCode = buf.get() & 0xFF;
         switch(posCode) {
         case CLASS_TYPE_PARAMETER:
         case METHOD_TYPE_PARAMETER: {
-            byte index = buf.get();
+            int index = buf.get() & 0xFF;
             TypeAnnotationTargetInfo res;
             if (posCode == CLASS_TYPE_PARAMETER)
                 res = new TypeAnnotationTargetInfo(TypeAnnotationTarget.CLASS_TYPE_PARAMETER,
@@ -418,7 +423,7 @@ public final class TypeAnnotationParser {
             return res;
             } // unreachable break;
         case CLASS_EXTENDS: {
-            short index = buf.getShort();
+            short index = buf.getShort(); //needs to be signed
             if (index == -1) {
                 return new TypeAnnotationTargetInfo(TypeAnnotationTarget.CLASS_EXTENDS);
             } else if (index >= 0) {
@@ -437,7 +442,7 @@ public final class TypeAnnotationParser {
         case METHOD_RECEIVER:
             return new TypeAnnotationTargetInfo(TypeAnnotationTarget.METHOD_RECEIVER);
         case METHOD_FORMAL_PARAMETER: {
-            byte index = buf.get();
+            int index = buf.get() & 0xFF;
             return new TypeAnnotationTargetInfo(TypeAnnotationTarget.METHOD_FORMAL_PARAMETER,
                     index);
             } //unreachable break;
@@ -486,12 +491,12 @@ public final class TypeAnnotationParser {
     }
 
     private static TypeAnnotationTargetInfo parseShortTarget(TypeAnnotationTarget target, ByteBuffer buf) {
-        short index = buf.getShort();
+        int index = buf.getShort() & 0xFFFF;
         return new TypeAnnotationTargetInfo(target, index);
     }
     private static TypeAnnotationTargetInfo parse2ByteTarget(TypeAnnotationTarget target, ByteBuffer buf) {
-        byte count = buf.get();
-        byte secondaryIndex = buf.get();
+        int count = buf.get() & 0xFF;
+        int secondaryIndex = buf.get() & 0xFF;
         return new TypeAnnotationTargetInfo(target,
                                             count,
                                             secondaryIndex);

test/java/lang/annotation/typeAnnotations/BadCPIndex.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8023878
+ * @summary Test that the right kind of exception is thrown from the type
+ *          annotation reflection code.
+ * @run testng BadCPIndex
+ */
+
+import java.lang.annotation.*;
+import java.util.Base64;
+import java.util.function.Function;
+
+import org.testng.annotations.Test;
+import org.testng.annotations.DataProvider;
+
+public class BadCPIndex {
+    private static final  MyLoader loader = new MyLoader(BadCPIndex.class.getClassLoader());
+
+    // Blueprint for broken C
+    //public static class C extends @BadCPIndex.A Object {}
+    private static final String encodedBrokenC = "yv66vgAAADQAFgoAAwAPBwARBwATAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAA9CYWRDUEluZGV4LmphdmEBAB1SdW50aW1lVmlzaWJsZVR5cGVBbm5vdGF0aW9ucwcAFAEAAUEBAAxJbm5lckNsYXNzZXMBAA5MQmFkQ1BJbmRleCRBOwwABAAFBwAVAQAMQmFkQ1BJbmRleCRDAQABQwEAEGphdmEvbGFuZy9PYmplY3QBAAxCYWRDUEluZGV4JEEBAApCYWRDUEluZGV4ACEAAgADAAAAAAABAAEABAAFAAEABgAAAB0AAQABAAAABSq3AAGxAAAAAQAHAAAABgABAAAAKQADAAgAAAACAAkACgAAAAoAARD//wAADwAAAA0AAAASAAIACwAQAAwmCQACABAAEgAJ";
+
+    // Blueprint for broken D
+    //public static class D<@BadCPIndex.B U> {}
+    private static final String encodedBrokenD = "yv66vgAAADQAGAoAAwARBwATBwAVAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEACVNpZ25hdHVyZQEAKDxVOkxqYXZhL2xhbmcvT2JqZWN0Oz5MamF2YS9sYW5nL09iamVjdDsBAApTb3VyY2VGaWxlAQAPQmFkQ1BJbmRleC5qYXZhAQAdUnVudGltZVZpc2libGVUeXBlQW5ub3RhdGlvbnMHABYBAAFCAQAMSW5uZXJDbGFzc2VzAQAOTEJhZENQSW5kZXgkQjsMAAQABQcAFwEADEJhZENQSW5kZXgkRAEAAUQBABBqYXZhL2xhbmcvT2JqZWN0AQAMQmFkQ1BJbmRleCRCAQAKQmFkQ1BJbmRleAAhAAIAAwAAAAAAAQABAAQABQABAAYAAAAdAAEAAQAAAAUqtwABsQAAAAEABwAAAAYAAQAAAEAABAAIAAAAAgAJAAoAAAACAAsADAAAAAkAAQAAAAARAAAADwAAABIAAgANABIADiYJAAIAEgAUAAk=";
+
+    // Blueprint for broken E
+    //public static class E extends @BadCPIndex.A Object {}
+    private static final String encodedBrokenE = "yv66vgAAADQAFgoAAwAPBwARBwATAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAA9CYWRDUEluZGV4LmphdmEBAB1SdW50aW1lVmlzaWJsZVR5cGVBbm5vdGF0aW9ucwcAFAEAAUEBAAxJbm5lckNsYXNzZXMBAA5MQmFkQ1BJbmRleCRBOwwABAAFBwAVAQAMQmFkQ1BJbmRleCRFAQABRQEAEGphdmEvbGFuZy9PYmplY3QBAAxCYWRDUEluZGV4JEEBAApCYWRDUEluZGV4ACEAAgADAAAAAAABAAEABAAFAAEABgAAAB0AAQABAAAABSq3AAGxAAAAAQAHAAAABgABAAAARgADAAgAAAACAAkACgAAAAoAARD//wAADgAKAA0AAAASAAIACwAQAAwmCQACABAAEgAJ";
+
+    private static final Object[][] cases = {
+        { new Case("BadCPIndex$C", encodedBrokenC, Class::getAnnotatedSuperclass) },
+        { new Case("BadCPIndex$D", encodedBrokenD, (c -> c.getTypeParameters()[0].getAnnotations()))},
+        { new Case("BadCPIndex$E", encodedBrokenE, Class::getAnnotatedSuperclass) },
+    };
+
+    @DataProvider
+    public static Object[][] data() { return cases; }
+
+    @Test(dataProvider="data")
+    public static void testOpThrowsAFE(Case testCase) {
+        Class<?> c = loader.defineClass(testCase.name, Base64.getDecoder().decode(testCase.encoding));
+        try {
+            System.out.println("Testing: " + c);
+            testCase.trigger.apply(c);
+            throw new RuntimeException("Expecting AnnotationFormatError here");
+        } catch (AnnotationFormatError e) {
+            ; //ok
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    private static class MyLoader extends ClassLoader {
+        public MyLoader(ClassLoader parent) {
+            super(parent);
+        }
+
+        public Class<?> defineClass(String name, byte[] bytes) {
+            return defineClass(name, bytes, 0, bytes.length);
+        }
+    }
+
+    private static class Case {
+        public String name;
+        public String encoding;
+        public Function<Class<?>, Object> trigger;
+
+        public Case(String name, String encoding, Function<Class<?>, Object> trigger) {
+            this.name = name;
+            this.encoding = encoding;
+            this.trigger = trigger;
+        }
+    }
+
+    @Target(ElementType.TYPE_USE)
+    @Retention(RetentionPolicy.RUNTIME)
+    public static @interface A {}
+
+    @Target(ElementType.TYPE_PARAMETER)
+    @Retention(RetentionPolicy.RUNTIME)
+    public static @interface B {}
+}