8215510: j.l.c.ClassDesc is accepting descriptors not allowed by the spec

Vicente Romero · Vicente Romero · commit eed3a536c0fc · 2019-01-09T08:07:23.000-05:00
Reviewed-by: goetz
diff --git a/src/java.base/share/classes/java/lang/constant/ClassDesc.java b/src/java.base/share/classes/java/lang/constant/ClassDesc.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -97,7 +97,10 @@ static ClassDesc of(String name) {
      */
     static ClassDesc of(String packageName, String className) {
         ConstantUtils.validateBinaryClassName(requireNonNull(packageName));
-        validateMemberName(requireNonNull(className));
+        if (packageName.isEmpty()) {
+            return of(className);
+        }
+        validateMemberName(requireNonNull(className), false);
         return ofDescriptor(String.format("L%s%s%s;",
                                           binaryToInternal(packageName),
                                           (packageName.length() > 0 ? "/" : ""),
@@ -130,6 +133,9 @@ static ClassDesc of(String packageName, String className) {
      */
     static ClassDesc ofDescriptor(String descriptor) {
         requireNonNull(descriptor);
+        if (descriptor.isEmpty()) {
+            throw new IllegalArgumentException(String.format("not a valid reference type descriptor: %s", descriptor));
+        }
         int depth = ConstantUtils.arrayDepth(descriptor);
         if (depth > ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS) {
             throw new IllegalArgumentException(String.format("Cannot create an array type descriptor with more than %d dimensions",
@@ -192,7 +198,7 @@ default ClassDesc arrayType(int rank) {
      * @throws IllegalArgumentException if the nested class name is invalid
      */
     default ClassDesc nested(String nestedName) {
-        validateMemberName(nestedName);
+        validateMemberName(nestedName, false);
         if (!isClassOrInterface())
             throw new IllegalStateException("Outer class is not a class or interface type");
         return ClassDesc.ofDescriptor(String.format("%s$%s;", dropLastChar(descriptorString()), nestedName));
diff --git a/src/java.base/share/classes/java/lang/constant/ConstantUtils.java b/src/java.base/share/classes/java/lang/constant/ConstantUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -65,15 +65,15 @@ static String validateBinaryClassName(String name) {
      * @return the name passed if valid
      * @throws IllegalArgumentException if the member name is invalid
      */
-    public static String validateMemberName(String name) {
+    public static String validateMemberName(String name, boolean method) {
         requireNonNull(name);
         if (name.length() == 0)
             throw new IllegalArgumentException("zero-length member name");
         for (int i=0; i<name.length(); i++) {
             char ch = name.charAt(i);
             if (ch == '.' || ch == ';' || ch == '[' || ch == '/')
                 throw new IllegalArgumentException("Invalid member name: " + name);
-            if (ch == '<' || ch == '>') {
+            if (method && (ch == '<' || ch == '>')) {
                 if (!pointyNames.contains(name))
                     throw new IllegalArgumentException("Invalid member name: " + name);
             }
@@ -126,8 +126,8 @@ static List<String> parseMethodDescriptor(String descriptor) {
 
         ++cur;  // skip '('
         while (cur < end && descriptor.charAt(cur) != ')') {
-            int len = matchSig(descriptor, cur, end);
-            if (len == 0 || descriptor.charAt(cur) == 'V')
+            int len = skipOverFieldSignature(descriptor, cur, end, false);
+            if (len == 0)
                 throw new IllegalArgumentException("Bad method descriptor: " + descriptor);
             ptypes.add(descriptor.substring(cur, cur + len));
             cur += len;
@@ -136,41 +136,103 @@ static List<String> parseMethodDescriptor(String descriptor) {
             throw new IllegalArgumentException("Bad method descriptor: " + descriptor);
         ++cur;  // skip ')'
 
-        int rLen = matchSig(descriptor, cur, end);
+        int rLen = skipOverFieldSignature(descriptor, cur, end, true);
         if (rLen == 0 || cur + rLen != end)
             throw new IllegalArgumentException("Bad method descriptor: " + descriptor);
         ptypes.add(0, descriptor.substring(cur, cur + rLen));
         return ptypes;
     }
 
+    private static final char JVM_SIGNATURE_ARRAY = '[';
+    private static final char JVM_SIGNATURE_BYTE = 'B';
+    private static final char JVM_SIGNATURE_CHAR = 'C';
+    private static final char JVM_SIGNATURE_CLASS = 'L';
+    private static final char JVM_SIGNATURE_ENDCLASS = ';';
+    private static final char JVM_SIGNATURE_ENUM = 'E';
+    private static final char JVM_SIGNATURE_FLOAT = 'F';
+    private static final char JVM_SIGNATURE_DOUBLE = 'D';
+    private static final char JVM_SIGNATURE_FUNC = '(';
+    private static final char JVM_SIGNATURE_ENDFUNC = ')';
+    private static final char JVM_SIGNATURE_INT = 'I';
+    private static final char JVM_SIGNATURE_LONG = 'J';
+    private static final char JVM_SIGNATURE_SHORT = 'S';
+    private static final char JVM_SIGNATURE_VOID = 'V';
+    private static final char JVM_SIGNATURE_BOOLEAN = 'Z';
+
     /**
      * Validates that the characters at [start, end) within the provided string
      * describe a valid field type descriptor.
-     *
-     * @param str the descriptor string
+     * @param descriptor the descriptor string
      * @param start the starting index into the string
      * @param end the ending index within the string
+     * @param voidOK is void acceptable?
      * @return the length of the descriptor, or 0 if it is not a descriptor
      * @throws IllegalArgumentException if the descriptor string is not valid
      */
-    static int matchSig(String str, int start, int end) {
-        if (start >= end || start >= str.length() || end > str.length())
-            return 0;
-        char c = str.charAt(start);
-        if (c == 'L') {
-            int endc = str.indexOf(';', start);
-            int badc = str.indexOf('.', start);
-            if (badc >= 0 && badc < endc)
-                return 0;
-            badc = str.indexOf('[', start);
-            if (badc >= 0 && badc < endc)
-                return 0;
-            return (endc < 0) ? 0 : endc - start + 1;
-        } else if (c == '[') {
-            int t = matchSig(str, start+1, end);
-            return (t > 0) ? t + 1 : 0;
-        } else {
-            return ("IJCSBFDZV".indexOf(c) >= 0) ? 1 : 0;
+    @SuppressWarnings("fallthrough")
+    static int skipOverFieldSignature(String descriptor, int start, int end, boolean voidOK) {
+        int arrayDim = 0;
+        int index = start;
+        while (index < end) {
+            switch (descriptor.charAt(index)) {
+                case JVM_SIGNATURE_VOID: if (!voidOK) { return index; }
+                case JVM_SIGNATURE_BOOLEAN:
+                case JVM_SIGNATURE_BYTE:
+                case JVM_SIGNATURE_CHAR:
+                case JVM_SIGNATURE_SHORT:
+                case JVM_SIGNATURE_INT:
+                case JVM_SIGNATURE_FLOAT:
+                case JVM_SIGNATURE_LONG:
+                case JVM_SIGNATURE_DOUBLE:
+                    return index - start + 1;
+                case JVM_SIGNATURE_CLASS:
+                    // Skip leading 'L' and ignore first appearance of ';'
+                    index++;
+                    int indexOfSemi = descriptor.indexOf(';', index);
+                    if (indexOfSemi != -1) {
+                        String unqualifiedName = descriptor.substring(index, indexOfSemi);
+                        boolean legal = verifyUnqualifiedClassName(unqualifiedName);
+                        if (!legal) {
+                            return 0;
+                        }
+                        return index - start + unqualifiedName.length() + 1;
+                    }
+                    return 0;
+                case JVM_SIGNATURE_ARRAY:
+                    arrayDim++;
+                    if (arrayDim > MAX_ARRAY_TYPE_DESC_DIMENSIONS) {
+                        throw new IllegalArgumentException(String.format("Cannot create an array type descriptor with more than %d dimensions",
+                                ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS));
+                    }
+                    // The rest of what's there better be a legal descriptor
+                    index++;
+                    voidOK = false;
+                    break;
+                default:
+                    return 0;
+            }
+        }
+        return 0;
+    }
+
+    static boolean verifyUnqualifiedClassName(String name) {
+        for (int index = 0; index < name.length(); index++) {
+            char ch = name.charAt(index);
+            if (ch < 128) {
+                if (ch == '.' || ch == ';' || ch == '[' ) {
+                    return false;   // do not permit '.', ';', or '['
+                }
+                if (ch == '/') {
+                    // check for '//' or leading or trailing '/' which are not legal
+                    // unqualified name must not be empty
+                    if (index == 0 || index + 1 >= name.length() || name.charAt(index + 1) == '/') {
+                        return false;
+                    }
+                }
+            } else {
+                index ++;
+            }
         }
+        return true;
     }
 }
diff --git a/src/java.base/share/classes/java/lang/constant/DirectMethodHandleDescImpl.java b/src/java.base/share/classes/java/lang/constant/DirectMethodHandleDescImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -68,7 +68,7 @@ final class DirectMethodHandleDescImpl implements DirectMethodHandleDesc {
 
         requireNonNull(kind);
         validateClassOrInterface(requireNonNull(owner));
-        validateMemberName(requireNonNull(name));
+        validateMemberName(requireNonNull(name), true);
         requireNonNull(type);
 
         switch (kind) {
diff --git a/src/java.base/share/classes/java/lang/constant/DynamicCallSiteDesc.java b/src/java.base/share/classes/java/lang/constant/DynamicCallSiteDesc.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -75,7 +75,7 @@ private DynamicCallSiteDesc(DirectMethodHandleDesc bootstrapMethod,
                                 String invocationName,
                                 MethodTypeDesc invocationType,
                                 ConstantDesc[] bootstrapArgs) {
-        this.invocationName = validateMemberName(requireNonNull(invocationName));
+        this.invocationName = validateMemberName(requireNonNull(invocationName), true);
         this.invocationType = requireNonNull(invocationType);
         this.bootstrapMethod = requireNonNull(bootstrapMethod);
         this.bootstrapArgs = requireNonNull(bootstrapArgs.clone());
diff --git a/src/java.base/share/classes/java/lang/constant/DynamicConstantDesc.java b/src/java.base/share/classes/java/lang/constant/DynamicConstantDesc.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -96,7 +96,7 @@ protected DynamicConstantDesc(DirectMethodHandleDesc bootstrapMethod,
                                   ClassDesc constantType,
                                   ConstantDesc... bootstrapArgs) {
         this.bootstrapMethod = requireNonNull(bootstrapMethod);
-        this.constantName = validateMemberName(requireNonNull(constantName));
+        this.constantName = validateMemberName(requireNonNull(constantName), true);
         this.constantType = requireNonNull(constantType);
         this.bootstrapArgs = requireNonNull(bootstrapArgs).clone();
 
diff --git a/src/java.base/share/classes/java/lang/constant/ReferenceClassDescImpl.java b/src/java.base/share/classes/java/lang/constant/ReferenceClassDescImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -49,7 +49,7 @@ final class ReferenceClassDescImpl implements ClassDesc {
      */
     ReferenceClassDescImpl(String descriptor) {
         requireNonNull(descriptor);
-        int len = ConstantUtils.matchSig(descriptor, 0, descriptor.length());
+        int len = ConstantUtils.skipOverFieldSignature(descriptor, 0, descriptor.length(), false);
         if (len == 0 || len == 1
             || len != descriptor.length())
             throw new IllegalArgumentException(String.format("not a valid reference type descriptor: %s", descriptor));
diff --git a/test/jdk/java/lang/constant/ClassDescTest.java b/test/jdk/java/lang/constant/ClassDescTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -42,6 +42,7 @@
 
 /**
  * @test
+ * @bug 8215510
  * @compile ClassDescTest.java
  * @run testng ClassDescTest
  * @summary unit tests for java.lang.constant.ClassDesc
@@ -215,7 +216,7 @@ public void testArrayClassDesc() throws ReflectiveOperationException {
     }
 
     public void testBadClassDescs() {
-        List<String> badDescriptors = List.of("II", "I;", "Q", "L",
+        List<String> badDescriptors = List.of("II", "I;", "Q", "L", "",
                                               "java.lang.String", "[]", "Ljava/lang/String",
                                               "Ljava.lang.String;", "java/lang/String");
 
diff --git a/test/jdk/java/lang/constant/NameValidationTest.java b/test/jdk/java/lang/constant/NameValidationTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -23,6 +23,7 @@
 
 /**
  * @test
+ * @bug 8215510
  * @compile NameValidationTest.java
  * @run testng NameValidationTest
  * @summary unit tests for verifying member names
@@ -45,8 +46,8 @@ public class NameValidationTest {
     private static final String[] badMemberNames = new String[] {"xx.xx", "zz;zz", "[l", "aa/aa", "<cinit>"};
     private static final String[] goodMemberNames = new String[] {"<clinit>", "<init>", "3", "~", "$", "qq"};
 
-    private static final String[] badClassNames = new String[] {"zz;zz", "[l", "aa/aa"};
-    private static final String[] goodClassNames = new String[] {"3", "~", "$", "qq", ".", "a.a"};
+    private static final String[] badClassNames = new String[] {"zz;zz", "[l", "aa/aa", ".", "a..b"};
+    private static final String[] goodClassNames = new String[] {"3", "~", "$", "qq", "a.a"};
 
     public void testMemberNames() {
         DirectMethodHandleDesc mh = MethodHandleDesc.of(Kind.VIRTUAL, CD_String, "isEmpty", "()Z");
diff --git a/test/jdk/java/lang/constant/boottest/java.base/java/lang/constant/ConstantUtilsTest.java b/test/jdk/java/lang/constant/boottest/java.base/java/lang/constant/ConstantUtilsTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -43,14 +43,14 @@ public class ConstantUtilsTest {
 
     public void testValidateMemberName() {
         try {
-            ConstantUtils.validateMemberName(null);
+            ConstantUtils.validateMemberName(null, false);
             fail("");
         } catch (NullPointerException e) {
             // good
         }
 
         try {
-            ConstantUtils.validateMemberName("");
+            ConstantUtils.validateMemberName("", false);
             fail("");
         } catch (IllegalArgumentException e) {
             // good
@@ -59,7 +59,7 @@ public void testValidateMemberName() {
         List<String> badNames = List.of(".", ";", "[", "/", "<", ">");
         for (String n : badNames) {
             try {
-                ConstantUtils.validateMemberName(n);
+                ConstantUtils.validateMemberName(n, true);
                 fail(n);
             } catch (IllegalArgumentException e) {
                 // good
