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

Mandy Chung mchung at openjdk.org
Sat Nov 2 00:16:41 UTC 2024


On Fri, 1 Nov 2024 23:57:20 GMT, Chen Liang <liach at openjdk.org> wrote:

>> 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?

yes.  since you already made some refactoring, can improve further.

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

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


More information about the core-libs-dev mailing list