RFR: 8343437: ClassDesc.of incorrectly permitting empty names [v2]

[Chen Liang]

On Fri, 1 Nov 2024 21:32:23 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Comments to clarify, also align skipOverFieldSignature
>
> src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java line 258:
> 
>> 256:         if (name.isEmpty())
>> 257:             return name;
>> 258:         return validateBinaryClassName(name);
> 
> Perhaps have a utility method to specify if empty name is allowed.   `validateBinaryClassName` and `validateInternalClassName` only differ in the separator charactor.   They can be refactored to call one single name validation method something like this:
> 
> 
>     private static String validateName(String name, boolean internalName, boolean allowEmptyName) {
>         if (!allowEmptyName && name.isEmpty())
>               throw invalidClassName(name);
>               
>         // state variable for detection of illegal states, such as:
>         // empty unqualified name, consecutive, leading, or trailing separators
>         int afterSeparator = 0;
>         int len = name.length();
>         for (int i = 0; i < len; i++) {
>             char ch = name.charAt(i);
>             // reject ';' or '[' or other form's separator
>             if (ch == ';' || ch == '[' || (internalName && ch == '.') || (!internalName && ch == '/'))
>                 throw invalidClassName(name);
>             if ((internalName && ch == '/') || (!internalName && ch == '.')) {
>                 // illegal state when received separator indicates consecutive
>                 // or leading separators
>                 if (i == afterSeparator)
>                     throw invalidClassName(name);
>                 afterSeparator = i + 1;
>             }
>         }
>         // reject empty unqualified name or trailing separators
>         if (len == afterSeparator)
>             throw invalidClassName(name);
>         return name;
>     }

I assume I will still keep the 4 public methods to call this implementation method, as the boolean arguments will be otherwise confusing.  Is that what you intend?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21830#discussion_r1826385667