On Fri, 31 Oct 2025 00:00:02 GMT, Chen Liang <[email protected]> wrote:
> There were a few other holes in the recent migration of BytecodeDescriptor,
> most notably:
>
> 1. BytecodeDescriptor is missing checks for `.`, `[` characters, leading,
> trailing, consecutive `/`, or empty name.
> 2. EnclosingMethod is only validated by hotspot to carry either field or
> method type. We still need to check for field types
>
> I have written up the behavioral changes in the CSR. In addition, I have
> added a few more tests to ensure the failure case behaviors of the migrated
> use sites.
src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java line 160:
> 158: | (1L << ('/' - CHECK_OFFSET))
> 159: | (1L << (';' - CHECK_OFFSET))
> 160: | (1L << ('[' - CHECK_OFFSET));
Are we sure that these are the only 4 non-identifier chars we can see in the
string?
src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java line 166:
> 164: int check = str.charAt(index) - CHECK_OFFSET;
> 165: if ((check & -Long.SIZE) == 0 && (NON_IDENTIFIER_MASK & (1L
> << check)) != 0) {
> 166: break;
Maybe this is a little clearer:
Suggestion:
if (check < 64 && (NON_IDENTIFIER_MASK & (1L << check)) != 0) {
break;
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28079#discussion_r2546556027
PR Review Comment: https://git.openjdk.org/jdk/pull/28079#discussion_r2546501723
