uschindler commented on pull request #1849: URL: https://github.com/apache/lucene-solr/pull/1849#issuecomment-690040410
> uschindler why are you so grumpy, you have so much to add to this issue, why don’t you do it in a positive way? Sorry for this. It was a bit busy yesterday and after I have seen a "workaround" that has security-critical effects (we're taking responsibility for access to declared members by JDK), and multiple people hammering: "hey there's a bug!", while at the same time this PR was almost merged (@s1monw said "LGTM") I had to stop this. Whenever you intend to use AccessController#doPrivileged, you should really only do this, if you are under control of that code and you take responsibility - and definitely never as a workaround for a bug. Side note: The bug in the JDK annoyed me too much, so I quickly went to "dark side". After a second thought, I developed the better workaround in #1850 (I had this in mind previously, too - but as @dweiss said, the new workaround is not as elegant as the subclassing). It can't handle the ByteBuffer case. This will come back to us, if we want to use Deflater at more places in Lucene; for now both bugfixes are fine. Maybe before merging #1850: *Could you also test, if making the bugfix class "public" helps?* (I don't think it will help, but it's worth a try). The problem is: Our class is in different classloader than JDK's Deflater (module class loader), so nobody from JDK would be able to see our private implementation and therefore JVM refuses to lookup that method (although the "old reflection API is not fully correct here; nowadays one should always use `MethodHandles.lookup()` in such cases as it clearly emulates and enforces what the JLS specifies). Because of this, I would have more expected a "package access" security exception. If JDK would have used a Lookup, this won't have happened, too. But as @iverase has a quick reproducer in Elasticsearch, maybe he can try to just make the class public. If that does not work, I will merge #1850. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
