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