Issue2662

classification
Title: IllegalAccessException accessing public abstract method via PyReflectedFunction
Type: crash Severity: normal
Components: Core Versions:
Milestone: Jython 2.7.2
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: jeff.allen Nosy List: jeff.allen, zyasoft
Priority: high Keywords: Java Roadmap

Created on 2018-04-01.08:04:53 by jeff.allen, last changed 2018-04-08.08:31:24 by jeff.allen.

Files
File name Uploaded Description Edit Remove
LegalAccess.java jeff.allen, 2018-04-02.18:57:50 Toy version of issue 2662
AccessAPI.java jeff.allen, 2018-04-05.20:07:59 Two ideas that involve following the MRO to an accessible method.
Messages
msg11864 (view) Author: Jeff Allen (jeff.allen) Date: 2018-04-01.08:04:52
I'm spinning this off from #2656 to cover the subset of those Java 9 problems with a common cause in PyReflectedFunction.

The codecs module provides an easy way to reproduce this (remote is just a batch file that runs jython withthe command line option for remote debugging):

PS jython-jvm9> .\remote -J--illegal-access=deny
Listening for transport dt_socket at address: 8000
Jython 2.7.2a1+ (default:d74f8c2cd56f, Mar 31 2018, 23:07:07)
[Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java9.0.1
Type "help", "copyright", "credits" or "license" for more information.
>>> import codecs
>>> enc = codecs.getencoder('gbk')
>>> enc(u"hello")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\encodings\_java.py", line 78, in encode
    encoder = Charset.forName(self.encoding).newEncoder()
        at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:361)
        at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:589)
        at java.base/java.lang.reflect.Method.invoke(Method.java:556)
java.lang.IllegalAccessException: java.lang.IllegalAccessException: class org.python.core.PyReflectedFunction cannot access class sun.nio.cs.GBK (in module java.base) because module java.base does not export sun.nio.cs to unnamed module @7ceb3185

For me the worrying thing is that newEncoder() is a *public* abstract method of CharSet, and yet we get this message on the grounds that the *implementation* sun.nio.cs.GBK is not exported. Obviously a factory that hands out instances of a private class that implements a public interface is a common pattern, and had better work in Jython.

My first idea is that we need to handle objects not according to their actual class, but according to some super class or interface that we are allowed to access.
msg11868 (view) Author: Jeff Allen (jeff.allen) Date: 2018-04-02.18:57:49
I tried an isolated experiment with reflection on the same method. It appears that if the method we were trying to call was:
  public abstract java.nio.charset.CharsetEncoder java.nio.charset.Charset.newEncoder()
and not:
  public java.nio.charset.CharsetEncoder sun.nio.cs.GBK.newEncoder()
then we'd be ok. This even though the second is the implementation of the first. Demonstration attached.

In compiled Java, the object that is the target of the call is declared as a Charset, and so the compiler emits an invokevirtual of Charset.newEncoder whereas in the reflective call we find the method in the actual class of the object.

In asking how to deal with this in Jython, where there are only actual types, not declared types, I believe we are really considering the semantics of attribute access. In this case, we would like to call the abstract method. This suggests that when constructing the PyJavaType, we should insert a Method object found by chasing the Java or Python MRO, not the one we find in the class itself. But is this always right?

As a counter-example, I think it would be possible to arrange that the base class be inaccessible while a derived class is not. And checking accessibility from PyJavaType does not necessarily reflect accessibility to the user application.
msg11878 (view) Author: Jeff Allen (jeff.allen) Date: 2018-04-05.20:07:58
I have a couple of ideas that involve going up the inheritance chain (MRO) from the actual class of the target object to find a method with the same signature that is amenable to a reflective invocation at run-time. THese botyh depend on the behaviour that Method.invoke promises: that where the target method is overidden in the actual class of the target object, the overriding definition is used.

In one idea we go up the MRO until we find the "most senior" definition of the required method, which is assumed to be the public API.

In the second, we go up until we find one where trySetAccessible succeeds.

See attached toy. I prefer the first of these because it works in all versions of Java. I think the place to do it is where the PyReflectedFunction is being created, not during the call itself. However, we would do this for all Java classes, and it so change the contents of the type object significantly. I'm unclear about what malign consequences may follow. A good consequence would be fewer PyReflectedFunction/Method objects. I haven't tried this yet, but the effect can be simulated  like this:

PS jython-jvm9> dist\bin\jython -J--illegal-access=deny
...
>>> import java.nio.charset.Charset as Charset
>>> import java.nio.CharBuffer as CharBuffer
>>> gbk = Charset.forName('gbk')
>>> type(gbk)
<type 'sun.nio.cs.GBK'>
>>> enc = gbk.newEncoder()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
        at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:361)
        at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:589)
        at java.base/java.lang.reflect.Method.invoke(Method.java:556)
java.lang.IllegalAccessException: java.lang.IllegalAccessException: class org.python.core.PyReflectedFunction cannot access class sun.nio.cs.GBK (in module java.base) because module java.base does not export sun.nio.cs to unnamed module @2a7ed1f

But if we get the (abstract) Charset.newEncoder method, that works:

>>> ne = Charset.__dict__['newEncoder']
>>> ne
<java function newEncoder 0x3>
>>> enc = ne(gbk)
>>> enc.encode(CharBuffer.wrap(u"hello"))
java.nio.HeapByteBuffer[pos=0 lim=5 cap=10]
msg11886 (view) Author: Jeff Allen (jeff.allen) Date: 2018-04-08.08:31:23
In fact we do something similar when we encounter a non-public class here:
https://hg.python.org/jython/file/d74f8c2cd56f/src/org/python/core/PyJavaType.java#l383

However, the role that "public" plays here needs to be replaced by "accessible", in the sense of modular accessibility from the unnamed module. I wonder if that can be done without invoking Java 9 specifics?

There's a bug in that if-statement too. The name field for core classes, has already been changed from the Java class name at:
https://hg.python.org/jython/file/d74f8c2cd56f/src/org/python/core/PyJavaType.java#l324
and so the prefix tested is no longer there, which means we perfrom this unnecessarily for core classes. That should be fixed, but for now it gives me lots of opportunity to see it in action :/

Also, the logic of this seems odd:
https://hg.python.org/jython/file/d74f8c2cd56f/src/org/python/core/PyJavaType.java#l957
We check to see if the class is non-public, after the merge?
History
Date User Action Args
2018-04-08 08:31:24jeff.allensetmessages: + msg11886
2018-04-05 20:08:00jeff.allensetfiles: + AccessAPI.java
messages: + msg11878
2018-04-02 18:57:51jeff.allensetfiles: + LegalAccess.java
nosy: + zyasoft
messages: + msg11868
2018-04-01 08:07:19jeff.allenlinkissue2656 dependencies
2018-04-01 08:04:54jeff.allencreate