diff --git a/src/java.base/share/classes/java/lang/constant/ClassDesc.java b/src/java.base/share/classes/java/lang/constant/ClassDesc.java index 9f58a4f94da..51369652740 100644 --- a/src/java.base/share/classes/java/lang/constant/ClassDesc.java +++ b/src/java.base/share/classes/java/lang/constant/ClassDesc.java @@ -120,7 +120,7 @@ public sealed interface ClassDesc * not in the correct format */ static ClassDesc of(String packageName, String className) { - validateBinaryClassName(packageName); + validateBinaryPackageName(packageName); validateMemberName(className, false); if (packageName.isEmpty()) { return internalNameToDesc(className); diff --git a/src/java.base/share/classes/java/lang/constant/PackageDesc.java b/src/java.base/share/classes/java/lang/constant/PackageDesc.java index 2798d69d6da..4f2791b3b5c 100644 --- a/src/java.base/share/classes/java/lang/constant/PackageDesc.java +++ b/src/java.base/share/classes/java/lang/constant/PackageDesc.java @@ -55,7 +55,7 @@ public sealed interface PackageDesc * @see PackageDesc#ofInternalName(String) */ static PackageDesc of(String name) { - ConstantUtils.validateBinaryPackageName(requireNonNull(name)); + ConstantUtils.validateBinaryPackageName(name); return new PackageDescImpl(ConstantUtils.binaryToInternal(name)); } @@ -75,7 +75,7 @@ public sealed interface PackageDesc * @see PackageDesc#of(String) */ static PackageDesc ofInternalName(String name) { - ConstantUtils.validateInternalPackageName(requireNonNull(name)); + ConstantUtils.validateInternalPackageName(name); return new PackageDescImpl(name); } diff --git a/src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java b/src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java index 21791937649..875c75296ee 100644 --- a/src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java +++ b/src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java @@ -174,8 +174,52 @@ public final class ConstantUtils { } /** - * Validates the correctness of a binary class name. In particular checks for the presence of - * invalid characters in the name. + * Validates the correctness of a class or interface name or a package name. + * In particular checks for the presence of invalid characters, + * consecutive, leading, or trailing separator char, for both non-internal + * and internal forms, and the empty string for class or interface names. + * + * @param name the name + * @param slashSeparator {@code true} means {@code /} is the separator char + * (internal form); otherwise {@code .} is the separator char + * @param allowEmpty {@code true} means the empty string is a valid name + * @return the name passed if valid + * @throws IllegalArgumentException if the name is invalid + * @throws NullPointerException if name is {@code null} + */ + private static String validateClassOrPackageName(String name, boolean slashSeparator, boolean allowEmpty) { + int len = name.length(); // implicit null check + // empty name special rule + if (allowEmpty && len == 0) + return name; + // state variable for detection of illegal states of + // empty name, consecutive, leading, or trailing separators + int afterSeparator = 0; + for (int i = 0; i < len; i++) { + char ch = name.charAt(i); + // reject ';' or '[' + if (ch == ';' || ch == '[') + throw invalidClassName(name); + // encounter a separator + boolean foundSlash = ch == '/'; + if (foundSlash || ch == '.') { + // reject the other separator char + // reject consecutive or leading separators + if (foundSlash != slashSeparator || i == afterSeparator) + throw invalidClassName(name); + afterSeparator = i + 1; + } + } + // reject empty name or trailing separators + if (len == afterSeparator) + throw invalidClassName(name); + return name; + } + + /** + * Validates the correctness of a binary class name. + * In particular checks for the presence of invalid characters, empty + * name, consecutive, leading, or trailing {@code .}. * * @param name the class name * @return the class name passed if valid @@ -183,18 +227,13 @@ public final class ConstantUtils { * @throws NullPointerException if class name is {@code null} */ public static String validateBinaryClassName(String name) { - for (int i = 0; i < name.length(); i++) { - char ch = name.charAt(i); - if (ch == ';' || ch == '[' || ch == '/' - || ch == '.' && (i == 0 || i + 1 == name.length() || name.charAt(i - 1) == '.')) - throw invalidClassName(name); - } - return name; + return validateClassOrPackageName(name, false, false); } /** * Validates the correctness of an internal class name. - * In particular checks for the presence of invalid characters in the name. + * In particular checks for the presence of invalid characters, empty + * name, consecutive, leading, or trailing {@code /}. * * @param name the class name * @return the class name passed if valid @@ -202,19 +241,13 @@ public final class ConstantUtils { * @throws NullPointerException if class name is {@code null} */ public static String validateInternalClassName(String name) { - for (int i = 0; i < name.length(); i++) { - char ch = name.charAt(i); - if (ch == ';' || ch == '[' || ch == '.' - || ch == '/' && (i == 0 || i + 1 == name.length() || name.charAt(i - 1) == '/')) - throw invalidClassName(name); - } - return name; + return validateClassOrPackageName(name, true, false); } /** * Validates the correctness of a binary package name. - * In particular checks for the presence of invalid characters in the name. - * Empty package name is allowed. + * In particular checks for the presence of invalid characters, consecutive, + * leading, or trailing {@code .}. Allows empty strings for the unnamed package. * * @param name the package name * @return the package name passed if valid @@ -222,18 +255,13 @@ public final class ConstantUtils { * @throws NullPointerException if the package name is {@code null} */ public static String validateBinaryPackageName(String name) { - for (int i = 0; i < name.length(); i++) { - char ch = name.charAt(i); - if (ch == ';' || ch == '[' || ch == '/') - throw new IllegalArgumentException("Invalid package name: " + name); - } - return name; + return validateClassOrPackageName(name, false, true); } /** * Validates the correctness of an internal package name. - * In particular checks for the presence of invalid characters in the name. - * Empty package name is allowed. + * In particular checks for the presence of invalid characters, consecutive, + * leading, or trailing {@code /}. Allows empty strings for the unnamed package. * * @param name the package name * @return the package name passed if valid @@ -241,12 +269,7 @@ public final class ConstantUtils { * @throws NullPointerException if the package name is {@code null} */ public static String validateInternalPackageName(String name) { - for (int i = 0; i < name.length(); i++) { - char ch = name.charAt(i); - if (ch == ';' || ch == '[' || ch == '.') - throw new IllegalArgumentException("Invalid package name: " + name); - } - return name; + return validateClassOrPackageName(name, true, true); } /** @@ -423,26 +446,22 @@ public final class ConstantUtils { case JVM_SIGNATURE_DOUBLE: return index - start; case JVM_SIGNATURE_CLASS: - // state variable for detection of illegal states, such as: - // empty unqualified name, '//', leading '/', or trailing '/' - boolean legal = false; + // state variable for detection of illegal states of + // empty name, '//', leading '/', or trailing '/' + int afterSeparator = index + 1; // start of internal name while (index < end) { - switch (descriptor.charAt(index++)) { - case ';' -> { - // illegal state on parser exit indicates empty unqualified name or trailing '/' - return legal ? index - start : 0; - } - case '.', '[' -> { - // do not permit '.' or '[' + ch = descriptor.charAt(index++); + if (ch == ';') + // reject empty name or trailing '/' + return index == afterSeparator ? 0 : index - start; + // reject '.' or '[' + if (ch == '.' || ch == '[') + return 0; + if (ch == '/') { + // reject '//' or leading '/' + if (index == afterSeparator) return 0; - } - case '/' -> { - // illegal state when received '/' indicates '//' or leading '/' - if (!legal) return 0; - legal = false; - } - default -> - legal = true; + afterSeparator = index + 1; } } break; diff --git a/test/jdk/java/lang/constant/ClassDescTest.java b/test/jdk/java/lang/constant/ClassDescTest.java index 839de27b178..ee76d27e8d0 100644 --- a/test/jdk/java/lang/constant/ClassDescTest.java +++ b/test/jdk/java/lang/constant/ClassDescTest.java @@ -279,7 +279,8 @@ public class ClassDescTest extends SymbolicDescTest { public void testBadClassDescs() { List badDescriptors = List.of("II", "I;", "Q", "L", "", "java.lang.String", "[]", "Ljava/lang/String", - "Ljava.lang.String;", "java/lang/String"); + "Ljava.lang.String;", "java/lang/String", "L;", + "La//b;", "L/a;", "La/;"); for (String d : badDescriptors) { try { @@ -292,7 +293,7 @@ public class ClassDescTest extends SymbolicDescTest { } List badBinaryNames = List.of("I;", "[]", "Ljava/lang/String", - "Ljava.lang.String;", "java/lang/String"); + "Ljava.lang.String;", "java/lang/String", ""); for (String d : badBinaryNames) { try { ClassDesc constant = ClassDesc.of(d); @@ -303,7 +304,7 @@ public class ClassDescTest extends SymbolicDescTest { } List badInternalNames = List.of("I;", "[]", "[Ljava/lang/String;", - "Ljava.lang.String;", "java.lang.String"); + "Ljava.lang.String;", "java.lang.String", ""); for (String d : badInternalNames) { try { ClassDesc constant = ClassDesc.ofInternalName(d); diff --git a/test/jdk/java/lang/constant/PackageDescTest.java b/test/jdk/java/lang/constant/PackageDescTest.java index fe54a96eb8b..f7824c81a3b 100644 --- a/test/jdk/java/lang/constant/PackageDescTest.java +++ b/test/jdk/java/lang/constant/PackageDescTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2024, 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 @@ -35,13 +35,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows; class PackageDescTest { @ParameterizedTest - @ValueSource(strings = {"a/b.d", "a[]", "a;"}) + @ValueSource(strings = {"a/b.d", "a[]", "a;", "a..b", "a.b.", ".a.b"}) void testInvalidPackageNames(String pkg) { assertThrows(IllegalArgumentException.class, () -> PackageDesc.of(pkg)); } @ParameterizedTest - @ValueSource(strings = {"a/b.d", "a[]", "a;"}) + @ValueSource(strings = {"a/b.d", "a[]", "a;", "a//b", "a/b/", "/a/b"}) void testInvalidInternalPackageNames(String pkg) { assertThrows(IllegalArgumentException.class, () -> PackageDesc.ofInternalName(pkg)); }