Remove deprecated SecurityManager, AccessController, SecurityException #184 #185
Conversation
jbescos
force-pushed
the
issue184
branch
7 times, most recently
from
9d85cef to
a4a7e86
Compare
…eclipse-ee4j#184 Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
There are 2 options:
|
|
@jbescos For logging-mailhandler, in the last release I tried to make the security manager work operable on old JDKs and forward compatible with JDK25 to deal with missing, hostile, methods and classes. What you have coded up would have been the patch I would code for when Angus-Mail requires JDK25. That said, the simplicity of not dealing with SecurityManager is nice. Also SecurityException is not deprecated yet which is why I have certain catches. Especially for overridable methods like in java.lang.Thread which could unconditionally throw SecurityException without a SecurityManager. |
|
OK, then we are good in Angus Mail for the time being and we don't need this PR so far. My understanding is, the main requirement is to not depend on SecurityManager, and we don't depend on it currently. CC @lukasj . I think he wanted to get rid of AccessController and SecurityExceptions too. |
|
There still might be work to do in Angus Mail to align with JakartaEE. For logging-mailhandler, I try to support a wide range of JDKs down to min JDK. I'll try to do a proper review today. |
| * | ||
| * @param format the format pattern or null for default pattern. | ||
| * @throws SecurityException if a security manager exists and the caller | ||
| * does not have <code>LoggingPermission("control")</code>. |
There was a problem hiding this comment.
If SecurityException is removed then so should LoggingPermission as it was deprecated for removal. This will be repeated throughout logging package.
| * named with '$' so WebappClassLoader.clearReferencesStaticFinal() method will | ||
| * ignore this field. However there is no standard documentation about this. | ||
| */ | ||
| private static final PrivilegedAction<Object> MAILHANDLER_LOADER |
There was a problem hiding this comment.
The deleted comments explain why left hand side was an interface. I would rather get rid of the inner class and static reference. All this code does is a getAndSet of the context classloader with guards to make it a no-op in some cases. It was all to workaround the SecurityManager. A new method private static ClassLoader setContextClassLoader(ClassLoader l) with no short circuit would be clearer since SM is gone.
| if (this.sealed) { | ||
| LogManagerProperties.checkLogManagerAccess(); | ||
| } else { | ||
| throw new SecurityException("this-escape"); |
There was a problem hiding this comment.
Change the type from SecurityException to java.lang.IllegalStateException. if that else condition is triggered the caller has access to a broken MailHander.
| */ | ||
| private Object getAndSetContextClassLoader(final Object ccl) { | ||
| if (ccl != GetAndSetContext.NOT_MODIFIED) { | ||
| try { |
There was a problem hiding this comment.
Change return and param to ClassLoader. Change body to simple implementation
2 participants
#184
Do we remove this?:
angus-mail/mailhandler/src/main/java/org/eclipse/angus/mail/util/logging/LogManagerProperties.java
Line 246 in eca8e08