8368050: Validation missing in ClassFile signature factories

liach · liach · commit f9b91a783676 · 2025-09-23T12:47:56.000Z
Reviewed-by: asotona
diff --git a/src/java.base/share/classes/java/lang/classfile/MethodSignature.java b/src/java.base/share/classes/java/lang/classfile/MethodSignature.java
@@ -113,13 +113,14 @@ public static MethodSignature of(MethodTypeDesc methodDescriptor) {
      *
      * @param result signature for the return type
      * @param arguments signatures for the method parameters
+     * @throws IllegalArgumentException if any of {@code arguments} is void
      */
     public static MethodSignature of(Signature result,
                                      Signature... arguments) {
         return new SignaturesImpl.MethodSignatureImpl(List.of(),
                                                       List.of(),
                                                       requireNonNull(result),
-                                                      List.of(arguments));
+                                                      SignaturesImpl.validateArgumentList(arguments));
     }
 
     /**
@@ -131,6 +132,7 @@ public static MethodSignature of(Signature result,
      * @param exceptions signatures for the exceptions
      * @param result signature for the return type
      * @param arguments signatures for the method parameters
+     * @throws IllegalArgumentException if any of {@code arguments} is void
      */
     public static MethodSignature of(List<Signature.TypeParam> typeParameters,
                                      List<Signature.ThrowableSig> exceptions,
@@ -140,7 +142,7 @@ public static MethodSignature of(List<Signature.TypeParam> typeParameters,
                 List.copyOf(requireNonNull(typeParameters)),
                 List.copyOf(requireNonNull(exceptions)),
                 requireNonNull(result),
-                List.of(arguments));
+                SignaturesImpl.validateArgumentList(arguments));
     }
 
     /**
diff --git a/src/java.base/share/classes/java/lang/classfile/Signature.java b/src/java.base/share/classes/java/lang/classfile/Signature.java
@@ -43,6 +43,11 @@
 
 /**
  * Models generic Java type signatures, as defined in JVMS {@jvms 4.7.9.1}.
+ * <p>
+ * Names in signatures are <dfn id="identifier">identifiers</dfn>, which must
+ * not be empty and must not contain any of the ASCII characters {@code
+ * . ; [ / < > :}.  Top-level class and interface names are denoted by
+ * slash-separated identifiers.
  *
  * @see Type
  * @see SignatureAttribute
@@ -73,6 +78,8 @@ public static Signature parseFrom(String javaTypeSignature) {
      * signature represents a reifiable type (JLS {@jls 4.7}).
      *
      * @param classDesc the symbolic description of the Java type
+     * @throws IllegalArgumentException if the field descriptor cannot be
+     *         {@linkplain ##identifier denoted}
      */
     public static Signature of(ClassDesc classDesc) {
         requireNonNull(classDesc);
@@ -139,6 +146,31 @@ public sealed interface RefTypeSig
 
     /**
      * Models the signature of a possibly-parameterized class or interface type.
+     * <p>
+     * These are examples of class type signatures:
+     * <ul>
+     * <li>{@code Lcom/example/Outer;} for {@code Outer}
+     * <br>Has class name {@code com/example/Outer} and no outer type or type
+     *     argument.
+     * <li>{@code Lcom/example/Outer$Nested<TA;>;} for {@code Outer.Nested<A>}
+     * <br>Has class name {@code com/example/Outer$Nested} representing a nested
+     *     class, no outer type, and a single type argument of type variable
+     *     {@code A}.
+     * <li>{@code Lcom/example/GenericOuter<TA;>.Inner;} for {@code
+     *     GenericOuter<A>.Inner}
+     * <br>Has class name {@code Inner}, a simple class name, outer type
+     *     {@code Lcom/example/GenericOuter<TA;>;} for {@code GenericOuter<A>},
+     *     and no type argument.
+     * </ul>
+     * <p>
+     * If the {@linkplain #outerType() outer type} exists, the {@linkplain
+     * #className() class name} is the simple name of the nested type.
+     * Otherwise, it is a {@linkplain ClassEntry##internalname binary name in
+     * internal form} (separated by {@code /}).
+     * <p>
+     * If a nested type does not have any enclosing parameterization, it may
+     * be represented without an outer type and as an internal binary name,
+     * in which nesting is represented by {@code $} instead of {@code .}.
      *
      * @see Type
      * @see ParameterizedType
@@ -152,7 +184,8 @@ public sealed interface ClassTypeSig
         /**
          * {@return the signature of the class that this class is a member of,
          * only if this is a member class}  Note that the outer class may be
-         * absent if it is not a parameterized type.
+         * absent if this is a member class without any parameterized enclosing
+         * type.
          *
          * @jls 4.5 Parameterized Types
          */
@@ -161,7 +194,8 @@ public sealed interface ClassTypeSig
         /**
          * {@return the class or interface name; includes the {@linkplain
          * ClassEntry##internalname slash-separated} package name if there is no
-         * outer type}
+         * outer type}  Note this may indicate a nested class name with {@code $}
+         * separators if there is no parameterized enclosing type.
          */
         String className();
 
@@ -188,10 +222,11 @@ default ClassDesc classDesc() {
          * @param className the name of the class or interface
          * @param typeArgs the type arguments
          * @throws IllegalArgumentException if {@code className} does not
-         *         represent a class or interface
+         *         represent a class or interface, or if it cannot be
+         *         {@linkplain Signature##identifier denoted}
          */
         public static ClassTypeSig of(ClassDesc className, TypeArg... typeArgs) {
-            return of(null, className, typeArgs);
+            return of(null, Util.toInternalName(className), typeArgs);
         }
 
         /**
@@ -201,8 +236,15 @@ public static ClassTypeSig of(ClassDesc className, TypeArg... typeArgs) {
          * @param className the name of this class or interface
          * @param typeArgs the type arguments
          * @throws IllegalArgumentException if {@code className} does not
-         *         represent a class or interface
+         *         represent a class or interface, or if it cannot be
+         *         {@linkplain Signature##identifier denoted}
+         * @deprecated
+         * The resulting signature does not denote the class represented by
+         * {@code className} when {@code outerType} is not null.  Use {@link
+         * #of(ClassTypeSig, String, TypeArg...) of(ClassTypeSig, String, TypeArg...)}
+         * instead.
          */
+        @Deprecated(since = "26", forRemoval = true)
         public static ClassTypeSig of(ClassTypeSig outerType, ClassDesc className, TypeArg... typeArgs) {
             requireNonNull(className);
             return of(outerType, Util.toInternalName(className), typeArgs);
@@ -211,8 +253,11 @@ public static ClassTypeSig of(ClassTypeSig outerType, ClassDesc className, TypeA
         /**
          * {@return a class or interface signature without an outer type}
          *
-         * @param className the name of the class or interface
+         * @param className the name of the class or interface, may use
+         *                  {@code /} to separate
          * @param typeArgs the type arguments
+         * @throws IllegalArgumentException if {@code className} cannot be
+         *         {@linkplain Signature##identifier denoted}
          */
         public static ClassTypeSig of(String className, TypeArg... typeArgs) {
             return of(null, className, typeArgs);
@@ -222,12 +267,19 @@ public static ClassTypeSig of(String className, TypeArg... typeArgs) {
          * {@return a class type signature}
          *
          * @param outerType signature of the outer type, may be {@code null}
-         * @param className the name of this class or interface
+         * @param className the name of this class or interface, may use
+         *                  {@code /} to separate if outer type is absent
          * @param typeArgs the type arguments
+         * @throws IllegalArgumentException if {@code className} cannot be
+         *         {@linkplain Signature##identifier denoted}
          */
         public static ClassTypeSig of(ClassTypeSig outerType, String className, TypeArg... typeArgs) {
-            requireNonNull(className);
-            return new SignaturesImpl.ClassTypeSigImpl(Optional.ofNullable(outerType), className.replace(".", "/"), List.of(typeArgs));
+            if (outerType != null) {
+                SignaturesImpl.validateIdentifier(className);
+            } else {
+                SignaturesImpl.validatePackageSpecifierPlusIdentifier(className);
+            }
+            return new SignaturesImpl.ClassTypeSigImpl(Optional.ofNullable(outerType), className, List.of(typeArgs));
         }
     }
 
@@ -383,9 +435,11 @@ public sealed interface TypeVarSig
          * {@return a signature for a type variable}
          *
          * @param identifier the name of the type variable
+         * @throws IllegalArgumentException if the name cannot be {@linkplain
+         *         Signature##identifier denoted}
          */
         public static TypeVarSig of(String identifier) {
-            return new SignaturesImpl.TypeVarSigImpl(requireNonNull(identifier));
+            return new SignaturesImpl.TypeVarSigImpl(SignaturesImpl.validateIdentifier(identifier));
         }
     }
 
@@ -408,20 +462,22 @@ public sealed interface ArrayTypeSig
         /**
          * {@return an array type with the given component type}
          * @param componentSignature the component type
+         * @throws IllegalArgumentException if the component type is void
          */
         public static ArrayTypeSig of(Signature componentSignature) {
-            return of(1, requireNonNull(componentSignature));
+            return of(1, SignaturesImpl.validateNonVoid(componentSignature));
         }
 
         /**
          * {@return a signature for an array type}
          * @param dims the dimension of the array
          * @param componentSignature the component type
          * @throws IllegalArgumentException if {@code dims < 1} or the
-         *         resulting array type exceeds 255 dimensions
+         *         resulting array type exceeds 255 dimensions or the component
+         *         type is void
          */
         public static ArrayTypeSig of(int dims, Signature componentSignature) {
-            requireNonNull(componentSignature);
+            SignaturesImpl.validateNonVoid(componentSignature);
             if (componentSignature instanceof SignaturesImpl.ArrayTypeSigImpl arr) {
                 if (dims < 1 || dims > 255 - arr.arrayDepth())
                     throw new IllegalArgumentException("illegal array depth value");
@@ -469,10 +525,12 @@ public sealed interface TypeParam
          * @param identifier the name of the type parameter
          * @param classBound the class bound of the type parameter, may be {@code null}
          * @param interfaceBounds the interface bounds of the type parameter
+         * @throws IllegalArgumentException if the name cannot be {@linkplain
+         *         Signature##identifier denoted}
          */
         public static TypeParam of(String identifier, RefTypeSig classBound, RefTypeSig... interfaceBounds) {
             return new SignaturesImpl.TypeParamImpl(
-                    requireNonNull(identifier),
+                    SignaturesImpl.validateIdentifier(identifier),
                     Optional.ofNullable(classBound),
                     List.of(interfaceBounds));
         }
@@ -483,10 +541,12 @@ public static TypeParam of(String identifier, RefTypeSig classBound, RefTypeSig.
          * @param identifier the name of the type parameter
          * @param classBound the optional class bound of the type parameter
          * @param interfaceBounds the interface bounds of the type parameter
+         * @throws IllegalArgumentException if the name cannot be {@linkplain
+         *         Signature##identifier denoted}
          */
         public static TypeParam of(String identifier, Optional<RefTypeSig> classBound, RefTypeSig... interfaceBounds) {
             return new SignaturesImpl.TypeParamImpl(
-                    requireNonNull(identifier),
+                    SignaturesImpl.validateIdentifier(identifier),
                     requireNonNull(classBound),
                     List.of(interfaceBounds));
         }
diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2025, 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
@@ -309,7 +309,7 @@ <S extends Signature> S mapSignature(S signature) {
             case Signature.ClassTypeSig cts ->
                 Signature.ClassTypeSig.of(
                         cts.outerType().map(this::mapSignature).orElse(null),
-                        map(cts.classDesc()),
+                        Util.toInternalName(map(cts.classDesc())),
                         cts.typeArgs().stream().map(ta -> switch (ta) {
                             case Signature.TypeArg.Unbounded u -> u;
                             case Signature.TypeArg.Bounded bta -> Signature.TypeArg.bounded(
diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/SignaturesImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/SignaturesImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2025, 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
@@ -73,7 +73,7 @@ public MethodSignature parseMethodSignature() {
             while (!match(')')) {
                 if (paramTypes == null)
                     paramTypes = new ArrayList<>();
-                paramTypes.add(typeSig());
+                paramTypes.add(validateNonVoid(typeSig()));
             }
             Signature returnType = typeSig();
             ArrayList<ThrowableSig> throwsTypes = null;
@@ -226,13 +226,9 @@ private void require(char c) {
      */
     private int requireIdentifier() {
         int start = sigp;
-        l:
         while (sigp < sig.length()) {
-            switch (sig.charAt(sigp)) {
-                case '.', ';', '[', '/', '<', '>', ':' -> {
-                    break l;
-                }
-            }
+            if (isNonIdentifierChar(sig.charAt(sigp)))
+                break;
             sigp++;
         }
         if (start == sigp) {
@@ -241,6 +237,77 @@ private int requireIdentifier() {
         return sigp;
     }
 
+    // Non-identifier chars in ascii 0 to 63, note [ is larger
+    private static final long SMALL_NON_IDENTIFIER_CHARS_SET = (1L << '.')
+            | (1L << ';')
+            | (1L << '/')
+            | (1L << '<')
+            | (1L << '>')
+            | (1L << ':');
+
+    private static boolean isNonIdentifierChar(char c) {
+        return c < Long.SIZE ? (SMALL_NON_IDENTIFIER_CHARS_SET & (1L << c)) != 0 : c == '[';
+    }
+
+    /// {@return exclusive end of the next identifier}
+    public static int nextIdentifierEnd(String st, int start) {
+        int end = st.length();
+        for (int i = start; i < end; i++) {
+            if (isNonIdentifierChar(st.charAt(i))) {
+                return i;
+            }
+        }
+        return end;
+    }
+
+    /// Validates this string as a simple identifier.
+    public static String validateIdentifier(String st) {
+        var len = st.length(); // implicit null check
+        if (len == 0 || nextIdentifierEnd(st, 0) != len) {
+            throw new IllegalArgumentException("Not a valid identifier: " + st);
+        }
+        return st;
+    }
+
+    /// Validates this string as slash-separated one or more identifiers.
+    public static String validatePackageSpecifierPlusIdentifier(String st) {
+        int nextIdentifierStart = 0;
+        int len = st.length();
+        while (nextIdentifierStart < len) {
+            int end = nextIdentifierEnd(st, nextIdentifierStart);
+            if (end == len)
+                return st;
+            if (end == nextIdentifierStart || st.charAt(end) != '/')
+                throw new IllegalArgumentException("Not a class name: " + st);
+            nextIdentifierStart = end + 1;
+        }
+        // Couldn't get an identifier initially or after a separator.
+        throw new IllegalArgumentException("Not a class name: " + st);
+    }
+
+    /// Validates the signature to be non-void (a valid field type).
+    public static Signature validateNonVoid(Signature incoming) {
+        Objects.requireNonNull(incoming);
+        if (incoming instanceof Signature.BaseTypeSig baseType && baseType.baseType() == 'V')
+            throw new IllegalArgumentException("void");
+        return incoming;
+    }
+
+    /// Returns the validated immutable argument list or fails with IAE.
+    public static List<Signature> validateArgumentList(Signature[] signatures) {
+        return validateArgumentList(List.of(signatures));
+    }
+
+    /// Returns the validated immutable argument list or fails with IAE.
+    public static List<Signature> validateArgumentList(List<Signature> signatures) {
+        var res = List.copyOf(signatures); // deep null checks
+        for (var sig : signatures) {
+            if (sig instanceof Signature.BaseTypeSig baseType && baseType.baseType() == 'V')
+                throw new IllegalArgumentException("void");
+        }
+        return res;
+    }
+
     public static record BaseTypeSigImpl(char baseType) implements Signature.BaseTypeSig {
 
         @Override
@@ -316,13 +383,13 @@ public static record TypeParamImpl(String identifier, Optional<RefTypeSig> class
 
     private static StringBuilder printTypeParameters(List<TypeParam> typeParameters) {
         var sb = new StringBuilder();
-        if (typeParameters != null && !typeParameters.isEmpty()) {
+        if (!typeParameters.isEmpty()) {
             sb.append('<');
             for (var tp : typeParameters) {
                 sb.append(tp.identifier()).append(':');
                 if (tp.classBound().isPresent())
                     sb.append(tp.classBound().get().signatureString());
-                if (tp.interfaceBounds() != null) for (var is : tp.interfaceBounds())
+                for (var is : tp.interfaceBounds())
                     sb.append(':').append(is.signatureString());
             }
             sb.append('>');
@@ -337,7 +404,7 @@ public static record ClassSignatureImpl(List<TypeParam> typeParameters, ClassTyp
         public String signatureString() {
             var sb = printTypeParameters(typeParameters);
             sb.append(superclassSignature.signatureString());
-            if (superinterfaceSignatures != null) for (var in : superinterfaceSignatures)
+            for (var in : superinterfaceSignatures)
                 sb.append(in.signatureString());
             return sb.toString();
         }
diff --git a/test/jdk/jdk/classfile/SignaturesTest.java b/test/jdk/jdk/classfile/SignaturesTest.java
