On 3/28/19 8:46 AM, Peter Levart wrote:
On 3/28/19 4:08 PM, Alan Bateman wrote:
On 28/03/2019 14:48, Peter Levart wrote:
:
In addition, if access from null caller is granted and it is
performed to a member in a "concealed" package, there's no warning
displayed
The proposed check is that the package is exported unconditionally so
it will fail, no warning needed. I think that is okay. I could
imagine someone trying to argue that they run with `--add-exports
java.base/<concealed-package>=ALL-UNNAMED` and they expect their JNI
code to be able to reflect on the public members of public classes in
that package but it hardly seems wroth it as JNI doesn't do access
checks so it's pointless writing JNI code to use reflection.
Right, and it would require deep changes to
AccessibleObject#logIfExportedForIllegalAccess too, since it assumes
the presence of non-null caller...
Yes it assumes the non-null caller in the current implementation. There
are several options addressing this. I would prefer to throw
IllegalCallerException if AccessibleObject::setAccessible or
trySetAccessible is called from null caller but this needs to have a
careful investigation. As Alan mentions, we should also do an audit on
all @CS methods and handles them.
My intent is to keep JDK-8221530 to fix the regression w.r.t. existing
member access check. I realized the synopsis implies a bigger scope on
other @CS methods. So I have considered other @CS methods besides
reflective member access check is a separate issue.
Nevertheless the access checking logic could be encapsulated entirely
in the Reflection class (for null caller too) and then in
AccessibleObject, the logIfExportedForIllegalAccess call just skipped
for null caller... Else the logic is scattered between two classes and
would have to be repeated for other similar cases.
Reflection.verifyMemberAccess() is called not only from
AccessibleObject.slowVerifyAccess() but from elsewhere too.
As you see from the webrev, Reflection.verifyMemberAccess requires
Objects.requiresNonNull(currentClass) and
Objects.requiresNonNull(memberClass).
If caller is null, it should not call Reflection.verifyMemberAccess
since the caller does not necessarily allow publicly accessible member
and NPE forces the author to think through it. If needed later, we
For example, from ReflectUtil.ensureMemberAccess which is used in @CS
AtomicXxxFieldUpdater(s), or from @CS java.util.ServiceLoader.load...
ServiceLoader does not support null caller.
By encapsulating it in the common Reflection.verifyMemberAccess()
method, all those usages get handled at the same time.
The API calling Reflection.verifyMemberAccess() should clearly decide
what behavior it wants when there is no caller. Maybe the same behavior
as Field::get or maybe not.
Mandy
