Fix LdapGroupMapping on JDK 16+#32
Conversation
| Hashtable<String, String> env = new Hashtable<String, String>(); | ||
| env.put(Context.INITIAL_CONTEXT_FACTORY, | ||
| com.sun.jndi.ldap.LdapCtxFactory.class.getName()); | ||
| env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); |
There was a problem hiding this comment.
Since JDK 16 this class is inteded to be used by javax.naming only
Shouldn't we use javax.something here then?
There was a problem hiding this comment.
No. Javax.naming has an access to com.sun so it's fine
There was a problem hiding this comment.
Remove compile-time reference to LdapCtxFactory
is it about compile-time only (and could perhaps be alleviated with some javac toggles), or runtime as well? i guess both?
There was a problem hiding this comment.
It should compile on 17 but will fail in runtime due to the access to the module
There was a problem hiding this comment.
Correct, this will fail on JDK 17 due to JEP 403:
Strong encapsulation applies at both compile time and run time, including when compiled code attempts to access elements via reflection at run time. The non-public elements of exported packages, and all elements of unexported packages, are said to be strongly encapsulated.
com.sun.jndi.ldap.LdapCtxFactory is in the java.naming module which doesn't export anything outside of the javax.naming package, so simply referencing the class will fail at runtime and probably at compile time, even though it is public and accessible via the pre-module rules.
| <exclude>org/apache/hadoop/util/LineReader.class</exclude> | ||
| <exclude>org/apache/hadoop/crypto/key/kms/KMSClientProvider.class</exclude> | ||
| <exclude>org/apache/hadoop/crypto/key/kms/KMSClientProvider$*.class</exclude> | ||
| <exclude>org/apache/hadoop/security/LdapGroupsMapping.class</exclude> |
There was a problem hiding this comment.
Does LdapGroupsMapping have any anonymous inner classes? Won't it ever have?
would be safer to exclude them proactively, like KMSClientProvider$*.class above
There was a problem hiding this comment.
It doesn't have but I can add it proactively
wendigo
force-pushed
the
serafin/ldap-jdk16
branch
from
bf1b36a to
3087294
Compare
|
PTAL @findepi |
Since JDK 16 this class is inteded to be used by javax.naming only.
wendigo
force-pushed
the
serafin/ldap-jdk16
branch
from
3087294 to
2f9bced
Compare
4 participants
No description provided.