Hi Robert,
I found the problematics code part. In general, setAccessible should be added to POI's forbidden-apis, because libraries embedded in other projects should never ever violate Java Language constraints.
I will check for other AccessibleObject#setAccesible.

I have no idea why we need reflection at all. We should just remove this. The target class is a POI-internal class, why not just make the problematic method public?

I reviewed the POI code: There are tons of this stuff inside.
Some code parts also try to access stuff inside JVM's classes. This will break with Java 9, as setAccessible on classes from Java runtime will fail.

I have no idea how to fix this mess. Giving up for now. The code does not even use AccessController.doPrivileged(), so one could limit the additional security permissions to POI's JAR files.

FYI, I added some initial rules for reflection usage in r1713218, they are currently commented out to not break the build.

Thanks Dominik. I was about to do the same.

At least we should wrap all reflection usage with AccessController.doPrivileged(), as this allows to enable this type of reflection only for the POI JAR file and not your whole project.

I started to fix some of the setAccessible stuff. The first one was the mmap unmapper, which I copied from Lucene. See r1713350.

Created attachment 33267 [details]
patch

First patch removing many useless setAccessible in POIs internal classes:

It adds getters marked with @Internal annotation. For the issue reported here, it is a bit more stupid, because you cannot make the method public, because it is protected by design. I added a public Accessor class with a static method that delegates. Maybe we shoud fix this in another way.

What do others think?

There are more stuff like this, I just started. There are some of those in tests, which is easy to fix in a similar way (using Accessor class inside tests, just placed in right package).

I will commit the patch tomorrow and proceed with more fixes like this. This should also fix the issue of Robert.

Created attachment 33270 [details]
improved patch

New patch that does not add a new class for the hack. I just added a static method in POIXMLDocumentPart to allow access to onDocumentRead, but clearly marked as @Internal.

On longer term the class structure and method acceibility should be reviewed. From looking at the code onDocument*() should all be public.

I'll commit this now.

I committed this now. There are more setAccessible calls outside of test that are not yet wrapped. I will work on remving them in a similar way.

The root cause of this issue is now fixed, but the other problems must be solved too. Robert just did not see them lack of suitable test documents to trigger setAccessible calls.

I removed all useless reflection in the remaining code:
- For test cases I added helper methods to do the actual reflection inside a doPrivileged. This also checks that you only reflect on POI classes (package check).
- Made some properties public with @Internal (where needed)
- Wrapped remaining stuff with doPrivileged()

I will open new issues for the following problems:
- Under Java 9, the ooxml-lite generator code will definitely fail. My idea is to use a separate classloader that tracks all loaded classes instead of reflecting into the ClassLoader
- The ZIP bomb detection will also fail in Java 9. The code should be refactored to wrap the zip file before opening a ZIPInputStream. This is not so easy, maybe somebody has an idea (I don't understand what's going on!!!)

I'll close this issue.