Me trying to explain the problem behind #ReflectiveAccessToNonExportedTypes

[David M. Lloyd]

On 7/15/16 3:18 AM, Remi Forax wrote:
> ----- Mail original -----
>> De: "David M. Lloyd" <david.lloyd at redhat.com>
>> À: jpms-spec-experts at openjdk.java.net
>> Envoyé: Jeudi 14 Juillet 2016 17:31:38
>> Objet: Re: Me trying to explain the problem behind	#ReflectiveAccessToNonExportedTypes
>>
>> On 07/14/2016 10:06 AM, Remi Forax wrote:
>>> Let me try to explain again the problem and the possible solution.
>>>
>>> Let say i have two modules, module1 and module2.
>>> In module1, I have two packages module1.exported and module1.nonexported,
>>> so module1 has this module-info.java
>>>
>>>     module module1 {
>>>       exports module1.exported;
>>>     }
>>>
>>> in module1.nonexported, I have a class A that's implement an interface I
>>> defined like this:
>>>     package module1.nonexported;
>>>
>>>     import module1.exported.I;
>>>
>>>     public class A implements I {
>>>       @Override
>>>       public void foo() {
>>>         System.out.println("foo");
>>>       }
>>>     }
>>>
>>> I is defined in module1.exported like this:
>>>     package module1.exported;
>>>
>>>     import module1.nonexported.A;
>>>
>>>     public interface I {
>>>       public void foo();
>>>
>>>       static I instance() {
>>>         return new A();
>>>       }
>>>     }
>>>
>>> Now, in module2, i want to use I so module2 requires module1, like this:
>>>     module module2 {
>>>       requires module1;
>>>     }
>>>
>>> and I have a main defined in the package modul2.main, like that:
>>>     package module2.main;
>>>
>>> import java.lang.reflect.Method;
>>>
>>> import module1.exported.I;
>>>
>>> public class Main {
>>>     public static void main(String[] args) throws
>>>     ReflectiveOperationException {
>>>       System.out.println(Class.forName("module1.nonexported.A"));
>>>
>>>       I i = I.instance();
>>>       Method m = i.getClass().getMethod("foo");
>>>       m.invoke(i);
>>>     }
>>> }
>>>
>>> If i run this code in with the two modules in the classpath,
>>> everything work and the code prints
>>>     class module1.nonexported.A
>>>     foo
>>>
>>> Now, if if the two modules are in the modulepath,
>>> Class.forName works but m.invoke() throws an IllegalAccessException because
>>> A is in a non exported package
>>>     java.lang.IllegalAccessException: class module2.main.Main (in module
>>>     module2) cannot access class module1.nonexported.A (in module module1)
>>>     because module module1 does not export module1.nonexported to module
>>>     module2
>>>
>>> If i try with m.setAccessible(true), before m.invoke(), setAccessible
>>> throws an exception InaccessibleObjectException
>>>     java.lang.reflect.InaccessibleObjectException: Unable to make member of
>>>     class module1.nonexported.A accessible:  module module1 does not export
>>>     module1.nonexported to module module2
>>>
>>> A way to solve this issue, is to use the interface I to call foo instead of
>>> using the class A,
>>>     Method m = I.class.getMethod("foo");
>>>     m.invoke(i);
>>>
>>> in that case, everything is Ok, because i don't try to use the method foo
>>> of A anymore.
>>>
>>> What Mark as proposed is to introduce a new "export dynamic" semantics
>>> which doesn't allow the package to be visible at compile time but to be
>>> visible at runtime.
>>> This solve the issue by supposing that everyone will write export dynamic
>>> on every non exported package to be backward compatible with the Java 8
>>> behavior.
>>>
>>> While i agree that "export dynamic" is a semantics that jigsaw should
>>> provide, i disagree with the syntax "export dynamic" because for me, the a
>>> non exported package should have this semantics in order to be backward
>>> compatible with existing code.
>>> So we should have 3 way to export or not a package,
>>>     1. don't export at compile time, don't export at runtime
>>>     2. don't export at compile time, export at runtime
>>>     3. export at compile time, export at runtime.
>>>
>>> 2 should be the default, 1 should be use by the package of the JDK (or any
>>> other libs) that want strong security, 3 is for the package that defines
>>> API (that will be maintained forever).
>>
>> This is a good summary and I think matches my understanding.  But the
>> basic problem is that if everyone is doing this "export dynamic" then
>> the safety proposed by restricting "public" is a false promise: we are
>> providing the feature only to take it away again, resulting in the exact
>> same situation we have today, but with no alternatives.  If we have to
>> do this then I think the feature itself needs to be re-examined.
>>
>> What I was suggesting was that we re-separate the notion of exports from
>> accessibility altogether, so you again have three different situations
>> rather than two:
>>
>> A. Compile time
>> B. Class link at run time
>> C. Reflection at run time
>
> B and C may be indistinguishable. For the OpenJdk + Hotspot, you can distinguish between access control done by the VM in C++ and access control done by the reflection API done at runtime. For OpenJDK + IBM J9, there is only one entry point for the the access checks. When specifying the JSR292 (java.lang.invoke + invokedynamic), we spend a lot of time to be sure that reflection behavior and bytecode behavior were aligned. I don't think that trying to separate B and C is possible that late in the game.

How could they possibly be indistinguishable?  All the code for causing 
public to be restricted in any way has been added for Jigsaw.

>> This changes the basic "export" back to mean "make available for
>> linkage", with no bearing on accessibility.  Then you have (at minimum)
>> the following ways to export (or not) a package:
>>
>> 1. No export at compile time, no export but reflection-only at run time
>> (but only for public types & members)
>> 2. Yes export at compile time, yes export at run time (but only for
>> public types & members)
>> 3. Yes export at compile time and run time, including specific grants to
>> access package-private members from targeted modules and/or packages
>>
>> Note: We could add a 1½ which is an export at run time but not compile
>> time if the EG agrees; I don't think that is particularly important
>> though as build tools can accomplish this easily enough.  I don't want
>> to derail the core point of the discussion on this topic, so I'm leaving
>> it aside for now.
>
> friend accesssibility doesn't solve the issue here, when you want 'strong encapsulation' i.e. no way to use reflection to access members of a not-exported class

You literally *never* want that.  By making a class public, users are 
saying "I do not want to encapsulate this class, I want everyone to be 
able to use it".  That is literally what it means.  This is impossible 
under the current Jigsaw solution.

If you want to prevent reflection being used, then package-private is 
the one you want.

>, you should not have to provide the list of all possible existing module.
 > I think we want 'export private' (strong encapsulation) and 'export 
dynamic' to be available without having a friend list.

Sure, and the same can be said for the Jigsaw solution, but in the 
majority of cases, your public classes would be public and for the 
others, you definitely want to control who specifically has access.

> And again, changing the meaning of package private is a no go for me, by example for a package like java.lang.invoke, i want the package to be exported, i want strong encapsulation and i want some classes inside the package by example the class that implements the whole method handle hierarchy to share secret (using the package private access).

I'm not sure I follow what you mean here.  If you have an internal 
package like java.lang.invoke, and you want to share non-public things 
to specific JDK-internal targets, you would just list them as friends.

>> This way the Java 8 behavior is retained with no "export dynamic"
>> anywhere in sight.  Rather you'd have something like "export private",
>> which you'd only grant for those special cases where a package must be
>> shared to a specific friend.  Things not intended to be shared can be
>> safely migrated to the new friend package feature.
>
> I agree that we should have a way to have Java 8 behavior without having to declare every packages with export dynamic,
> see my discussion with mark about specifying a default export behavior.

Yeah, the problem with that is that if you do this, you have defeated 
the security mechanism completely.  With the friend package solution, 
you don't have to defeat the mechanism to have Java 8 behavior, allowing 
all existing reflection code to continue to work, which is the entire 
point.  It still lets you solve the same use cases, just in a different way.

>> No new access limits
>> are introduced - instead old ones are enhanced in what I think is a
>> simple and intuitive way.  All the compatibility concerns go away, but
>> we can still use the mechanism to seal up the problems in the JDK with
>> shared code and things which should not be public.
>>
>> We should retain setAccessible() for now for compatibility.  We could
>> also introduce a "--safe-mode" flag which disables setAccessible()
>> *across the board* (that is, not just for modular code) without
>> controversy, because all those public members will remain accessible
>> without "help", plus a way is provided to allow access to be volunteered
>> from one package to another (along much the same philosophical lines as
>> MethodHandles.Lookup IMO).
>
> Deprecating setAccessible is another story for me.

I was under the impression that doing so is the whole point of this 
encapsulation feature.

-- 
- DML