Feedback on proposal for #ReflectiveAccessToNonExportedTypes

[Peter Levart]

Hi,

Just one thougt...

It seems that visibility of resources should be governed by some module
policy. It is becoming apparent that global visibility of resources where
only class loader delegation matters can sometimes be a problem. OTOH,
totally encapsulating all resources is also not acceptable. It would be
nice if module descriptor could specify which resources are "exported" and
which visible only to the owner module.

Regards, Peter

On Jul 23, 2016 2:33 AM, "Jason Greene" <jason.greene at redhat.com> wrote:

>
> > 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
>
>