RFR: 8261400: Reflection member filtering registration might be flawed
Chen Liang
liach at openjdk.org
Tue Jul 16 14:26:51 UTC 2024
On Tue, 16 Jul 2024 13:35:50 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Please review this patch that address the reflection member filtering flaws.
>>
>> 1. Remove a pointless bootstrap check to ensure no filter is accidentally bypassed due to class-loading order.
>> 2. Clarify the scenarios for filtering, and the correct filter actions for each scenario.
>> 3. Remove the filter in `sun.misc.Unsafe`; users are already using other ways to steal this instance, bypassing the filtered getter. The `jdk.internal.misc.Unsafe`'s filter was already removed in JDK-8161129, so this filter is meaningless.
>
> src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 58:
>
>> 56: // 3 filter scenarios:
>> 57: // 1. Classes loaded before Reflection, may (System) or may not
>> 58: // (ConstantPool) be initialized before main call: below
>
> This comment is confusing. I assume you want to say that these classes may or may not be loaded before the application main method. It may be simpler to just drop the proposed comments as they beg too many questions.
Let's put these 4 points in time: class A load, A init, Reflection init, main run. (If an event is after "main run", it may be completely optional)
There can be these arrangements:
1. A load, Reflection init, main run; subcases include:
a. A load, A init, Reflection init, main run (System)
b. A load, Reflection init, A init, main run (none known yet)
c. A load, Reflection init, main run, A init (ConstantPool)
Handling: register in Reflection static block
2. Reflection init, A load, A init, main run (MethodHandles.Lookup)
Handling: register in their own static block
3. Reflection init, main run, A init; subcases incldue:
a. Reflection init, A load, main run, A init (none known yet)
b. Reflection init, main run, A load, A init (sun.misc.Unsafe)
Handling: Quite hard, easily causing extra class loading
How should I phrase my comments?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20058#discussion_r1679515927
More information about the core-libs-dev
mailing list