Feedback on proposal for #ReflectiveAccessToNonExportedTypes

[Jason Greene]

> On Jul 21, 2016, at 3:45 PM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> Just a few comments on the examples posed in the last mail.

Thanks!

> 
> 
> On 21/07/2016 18:01, Jason Greene wrote:
>> :
>> That would help, but there is also class visibility issues that would need to be addressed as well.
>> 
>> Example 1 (Ambiguous class names):
>> 
>> Both A and B export “bar”, and both define “bar.MyClass” which have differing definitions. Victim could load the supposed to be hidden A’s MyClass instead of the intended B’s MyClass.
>> 
>> There is also a variant of this where the conflict is between Victim and A if A also exports another hidden package that is present in Victim itself.
> If the scenario were:
> 
> A exports bar
> B exports bar
> Victim requires A; requires B;
> 
> then it would be a split package issue and would be rejected at resolution time (or more specifically, one of the post resolution checks). The simple rule is that two or more modules can't export the same package to a module that reads both. This includes the case where a module M containing package p reads another module that exports p to M.
> 
> If the scenario is:
> 
> A exports dynamic bar
> B exports bar
> Victim requires A; requires B;

Sorry, I should have been more clear that I was reusing the earlier example that was immediately before the reply. This latter scenario you describe is what I intended. 

> 
> then, with the updated semantics, there is no split package issue at resolution time. Victim reads module A and module B but the `exports dynamic` edges are ignored by the post-resolution checks. If it "as if" Victim invokes addReads(A), nothing more
> 
> Moving on to the class loader delegation graph then it needs to support the readability graph and exports. With the updated semantics then any reference to bar.MyClass in Victim will delegate to the loader of module B. Victim's loader doesn't know anything about package bar in module A.
> 
> There are of course dozens of ways in which a Class object for some type bar.* type in module A could leak to Victim, maybe it uses Class.forName specifying module A's loader. Assuming code in Victim gets a reference to a public type in module A's bar package then it will be accessible to code in Victim (as intended in the scenario I think).

Ok great, that’s the behavior I was advocating. My original intention was more that a separate unrelated module (e.g. DI framework used by A) would dynamically access A, and that victim was just the unintended consequence of A enabling the unrelated module. However, the behavior of Victim being able to access it as well makes sense and is completely consistent with the definitions above and past behavior.

> 
> 
>> Example 2 (Unintentional discovery):
>> 
>> Victim uses ClassLoader.getResources (plural), looking for a standard configuration file or class name, and receives entries for both A and B. A’s was not intended to be discovered by victim, and leads to a failure state. As an example perhaps the configuration file in B specifies a class name in B’s dependency, which is not visible to Victim. Or, perhaps A’s config leads to duplicate runtime actions being configured (since the file was really only indented for A, which also processes it)
> In this scenario then I can't tell if the "should-not-be-discovered" A type is an exported package or not. If it's a public type in a package exported to Victim then it will be accessible to code in Victim. If it's not in an exported package then it won't be accessible.
> 
> Whether it's visible is another question, that depends on how the class loaders and delegation are arranged. In the simple case then A, B, and Victim are all defined to the same class loader. That would allow Victim to load any type in A but only the public types in A's exported packages would be  accessible.
> 
> For the scenario where the file lists some class in a random module B reads, say module C, then it may or may not be visible to Victim. It might be able to load it, it might not. Assuming it is visible then it may or may not be accessible. There isn't enough in the example scenario to know if this type is in a packaged exported by C or not.

This was intended to mirror the same structure as the original example as well: A exports dynamic bar, and B exports bar, and also all modules are following a class loader per module paradigm.  I had a follow-up that corrected this, it should have read:

"As an example perhaps the configuration file in [A] specifies a class name in [A]’s dependency, which is not visible to Victim. Or, perhaps A’s config leads to duplicate runtime actions being configured (since the file was really only indented for A, which also processes it)”

It sounds from your description above that Victim’s classloader would not delegate to A’s class loader, so resources in A’s /bar would not be discoverable from Victim. Extending this scenario as you describe with B requires C.  I imagine that C’s contents would not be visible to Victim (unless it was defined B requires public C)?

--
Jason T. Greene
WildFly Lead / JBoss EAP Platform Architect
JBoss, a division of Red Hat