[WFLY-15652] load InitialContextFactory class within doPrivileged block #14895
Conversation
github-actions
bot
added
the
deps-ok
|
@darranl could you review this please? see a comment in the jira. thanks |
| final Constructor<javax.naming.spi.InitialContextFactory> factoryClassConstructor; | ||
| if (WildFlySecurityManager.isChecking()) { | ||
| factoryClassConstructor = AccessController.doPrivileged((PrivilegedExceptionAction<Constructor<javax.naming.spi.InitialContextFactory>>) () -> { | ||
| final Class<?> factoryClass = Class.forName(factoryClassName, false, classLoader); |
There was a problem hiding this comment.
It is not enough to request changes on this PR but IMO I find it better to split all of the common code out, in this case it could just be a static method.
Where code like this is duplicated there is a risk in the future that the two get out of sync so bugs only occur in one case or the other.
|
Regarding the actual changes, I have some concerns that this may not be appropriate as this now means the caller can load any classes without their own protection domain being included in the permission check - I think the existing behaviour that the callers protections domain is needed for loading classes specified in the properties is correct. |
|
@darranl Does it mean the Jira should be closed as Won'tFix then? |
|
@istudens yes I think we should close as wont fix |
https://issues.redhat.com/browse/WFLY-15652