jakartaee
Fix #403 - Adding doPrivileged block to work properly in environments with installed security manager
I'm leaning towards rejecting this PR, as Jakarta wide there is a movement to remove all SecurityManager usages in light of the deprecating / neutering / and future removal of the SecurityManager in the JVM.
See:
- https://openjdk.org/jeps/411
- https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/SecurityManager.html
I'm leaning towards rejecting this PR, as Jakarta wide there is a movement to remove all SecurityManager usages in light of the deprecating / neutering / and future removal of the SecurityManager in the JVM.
Yes, the next API would be for EE 11, which is supposed to be on JDK 17, minimum, no security manager. This PR is towards the 2.1.0 branch which expects another EE 10 release, though.
BTW, This means Websocket 2.1.x wont run on new JVM's that have the System.getSecurityManager() method entirely removed.
This implementation is broken for newer JVMs in a different way.
The correct implementation is ...
public static WebSocketContainer getWebSocketContainer() {
if (System.getSecurityManager() == null) {
return getWebSocketContainerImpl();
}
// In newer (pre-removal) JVMs doPrivileged() return null always
WebSocketContainer container = AccessController.doPrivileged(
new PrivilegedAction<WebSocketContainer>() {
@Override
public WebSocketContainer run() {
return getWebSocketContainerImpl();
}
}
);
// if a value is present, return it
if (container != null){
return container;
}
// otherwise default to non-privileged behavior
return getWebSocketContainer();
}
It is still present in Java 21 so that isn't an issue yet. And folks that want a 2.1 API without any SecurityManager calls can always use 2.1.0 once it does get removed.
@markt-asf it's far far more subtle than mere existence right now.
The entire layer is considered deprecated, and different JVMs behave differently now with those deprecated classes and methods. Many always return null, others return without the security manager behaviors, and others have fallbacks to security manager if specified.
It should be considered unreliable, and any code that use those JVM features needs to be aware of that and handle things accordingly.
I'm not seeing anything in the Javadoc (looking at Java 21 the most recent available) that would justify any Java implementation providing anything other than the pre-deprecation behaviour. Can you provide an example of a JRE that isn't providing the pre-deprecation behaviour?
Possibly (but unlikely), the next JDK (22) can drop the AccessController completely. Then the working solution would be a multi-release jar, for JDK pre-17, with AccessController on, and for later, where no SecurityManager is available. That's for the case where a new 2.1.x API won't ever be released.
I have no objection to handling the removal of the SecurityManager functionality once we know what that looks like but at this point all we can do is guess what might happen. Return null? Complete removal? Something else? Given the trivial work-around (use 2.1.0) my preference is to code against what is currently known i.e. Java 21. Point releases are easy to do (or will be once I figure out the current issues with the CI system) so we can easily do a 2.1.2 at some point in the future if needed.
So, options:
- do nothing
- release 2.1.1 as-is (a first RC is on staging now for review)
- refactor the current implementation to handle what might go wrong in the future (MethodNotFoundException, null returns etc)
- keep the current changes but use a multi-release JAR that restores the 2.1.0 behaviour when running on Java > X
My own preference is for 2) now and 4) once we know which Java version the SecurityManager will be disabled in.
My impression is that SecurityManager is a dummy since JDK 17 and can be left from the code since then.
once we know which Java version the SecurityManager will be disabled in.
My personal preference is to not wait until the very very last moment until it will actually be fully removed / disabled, but already stop supporting it.
People should really stop using the security manager already, and also not keep using it on the last versions that supported it. IMHO that is; doing to gradually phases the usage out, instead of having a big bang moment where everyone gets stuck on one particular version again.
My impression is that SecurityManager is a dummy since JDK 17 and can be left from the code since then.
Reviewing the SecurityManager and associated implementations in the latest JDK source code, the implementation remains complete. So far, it has been deprecated and nothing else.
My personal preference is to not wait until the very very last moment until it will actually be fully removed / disabled, but already stop supporting it.
Keep in mind this discussion is only for 2.1.x (Jakarta EE 10 with minimum Java version of Java 11). WebSocket 2.2 onwards (Jakarta EE 11) won't have any security manager support.
Reviewing the SecurityManager and associated implementations in the latest JDK source code, the implementation remains complete. So far, it has been deprecated and nothing else.
You are right, the only change I can see is that java.security.manager system property must be set to "allow".
Keep in mind this discussion is only for 2.1.x (Jakarta EE 10 with minimum Java version of Java 11). WebSocket 2.2 onwards (Jakarta EE 11) won't have any security manager support.
My feeling is that the Platform wants to be more microservices oriented than fat application-server oriented, so I'd expect the platform to drop the support, too. But hearing people from the application server team being unhappy with the SecurityManager deprecation, I'd be happy to hear a statement from Jakarta about EE11 Platform SecurityManager support first.
I'd be happy to hear a statement from Jakarta about EE11 Platform SecurityManager support first.
That statement was made a year ago or so. Essentially everybody (all vendor representatives) wanted it gone.
So, given the above discussions, are there any objections to proceeding with a 2.1.1 release (Jakarta EE 10) that includes this fix so the WebSocket 2.1.1 API works when running under a security manager.