This is an automated email from the ASF dual-hosted git repository. johnnyv pushed a commit to branch bugfix/DIRMINA1132 in repository https://gitbox.apache.org/repos/asf/mina.git
The following commit(s) were added to refs/heads/bugfix/DIRMINA1132 by this push: new 3608313 simplifies #isSecured() checking; adds null checks 3608313 is described below commit 3608313fb8c3c8ccc9b14637da90d84e658315d6 Author: Jonathan Valliere <john...@apache.org> AuthorDate: Sat Jul 31 02:41:13 2021 -0400 simplifies #isSecured() checking; adds null checks --- .../org/apache/mina/filter/ssl/SslHandler.java | 4 +- .../org/apache/mina/filter/ssl2/SSL2Filter.java | 77 ++++++++++++++++------ .../org/apache/mina/filter/ssl2/SSL2HandlerG0.java | 5 +- .../transport/socket/nio/NioSocketSession.java | 8 +-- 4 files changed, 63 insertions(+), 31 deletions(-) diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java index 070ee18..29b9cae 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java @@ -23,8 +23,6 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; @@ -45,6 +43,7 @@ import org.apache.mina.core.write.DefaultWriteRequest; import org.apache.mina.core.write.WriteRequest; import org.apache.mina.core.write.WriteRequestQueue; import org.apache.mina.filter.ssl.SslFilter.EncryptedWriteRequest; +import org.apache.mina.filter.ssl2.SSL2Filter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -570,6 +569,7 @@ class SslHandler { if (firstSSLNegociation) { firstSSLNegociation = false; + this.session.setAttribute(SSL2Filter.SSL_SECURED, this); nextFilter.event(session, SslEvent.SECURED); } diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Filter.java b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Filter.java index 803fde5..9b5c5a5 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Filter.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Filter.java @@ -20,6 +20,7 @@ package org.apache.mina.filter.ssl2; import java.net.InetSocketAddress; +import java.util.Objects; import java.util.concurrent.Executor; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.ThreadPoolExecutor; @@ -39,7 +40,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * An SSL Filter which simplifies and controls the flow of encrypted information + * An simple SSL processor which performs flow control of encrypted information * on the filter-chain. * <p> * The initial handshake is automatically enabled for "client" sessions once the @@ -49,34 +50,43 @@ import org.slf4j.LoggerFactory; */ public class SSL2Filter extends IoFilterAdapter { - public static final AttributeKey SSL_HANDLER = new AttributeKey(SSL2Filter.class, "handler"); + /** + * Returns the SSL2Handler object + */ + static public final AttributeKey SSL_HANDLER = new AttributeKey(SSL2Filter.class, "handler"); + + /** + * The presence of this attribute in a session indicates that the session is + * secured. + */ + static public final AttributeKey SSL_SECURED = new AttributeKey(SSL2Filter.class, "status"); - protected static final Logger LOGGER = LoggerFactory.getLogger(SSL2Filter.class); + /** + * The logger + */ + static protected final Logger LOGGER = LoggerFactory.getLogger(SSL2Filter.class); - protected static final Executor EXECUTOR = new ThreadPoolExecutor(2, 2, 100, TimeUnit.MILLISECONDS, + /** + * Task executor for processing handshakes + */ + static protected final Executor EXECUTOR = new ThreadPoolExecutor(2, 2, 100, TimeUnit.MILLISECONDS, new LinkedBlockingDeque<Runnable>(), new BasicThreadFactory("ssl-exec", true)); protected final SSLContext mContext; - protected boolean mNeedClientAuth; - protected boolean mWantClientAuth; - protected String[] mEnabledCipherSuites; - protected String[] mEnabledProtocols; /** * Creates a new SSL filter using the specified {@link SSLContext}. * - * @param sslContext The SSLContext to use + * @param context The SSLContext to use */ - public SSL2Filter(SSLContext sslContext) { - if (sslContext == null) { - throw new IllegalArgumentException("SSLContext is null"); - } + public SSL2Filter(SSLContext context) { + Objects.requireNonNull(context, "ssl must not be null"); - this.mContext = sslContext; + this.mContext = context; } /** @@ -158,9 +168,7 @@ public class SSL2Filter extends IoFilterAdapter { public void onPreAdd(IoFilterChain parent, String name, NextFilter next) throws Exception { // Check that we don't have a SSL filter already present in the chain if (parent.contains(SSL2Filter.class)) { - String msg = "Only one SSL filter is permitted in a chain."; - LOGGER.error(msg); - throw new IllegalStateException(msg); + throw new IllegalStateException("Only one SSL filter is permitted in a chain"); } if (LOGGER.isDebugEnabled()) { @@ -168,6 +176,9 @@ public class SSL2Filter extends IoFilterAdapter { } } + /** + * {@inheritDoc} + */ @Override public void onPostAdd(IoFilterChain parent, String name, NextFilter next) throws Exception { IoSession session = parent.getSession(); @@ -177,15 +188,27 @@ public class SSL2Filter extends IoFilterAdapter { super.onPostAdd(parent, name, next); } + /** + * {@inheritDoc} + */ @Override public void onPreRemove(IoFilterChain parent, String name, NextFilter next) throws Exception { IoSession session = parent.getSession(); + session.removeAttribute(SSL_SECURED); SSL2Handler x = SSL2Handler.class.cast(session.removeAttribute(SSL_HANDLER)); if (x != null) { x.close(next); } } + /** + * Internal method for performing post-connect operations; this can be triggered + * during normal connect event or after the filter is added to the chain. + * + * @param next + * @param session + * @throws Exception + */ protected void sessionConnected(NextFilter next, IoSession session) throws Exception { SSL2Handler x = SSL2Handler.class.cast(session.getAttribute(SSL_HANDLER)); @@ -204,6 +227,9 @@ public class SSL2Filter extends IoFilterAdapter { x.open(next); } + /** + * {@inheritDoc} + */ @Override public void sessionOpened(NextFilter next, IoSession session) throws Exception { if (LOGGER.isDebugEnabled()) @@ -213,16 +239,24 @@ public class SSL2Filter extends IoFilterAdapter { super.sessionOpened(next, session); } + /** + * {@inheritDoc} + */ @Override public void messageReceived(NextFilter next, IoSession session, Object message) throws Exception { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("session {} received {}", session, message); SSL2Handler x = SSL2Handler.class.cast(session.getAttribute(SSL_HANDLER)); x.receive(next, IoBuffer.class.cast(message)); } + /** + * {@inheritDoc} + */ @Override public void messageSent(NextFilter next, IoSession session, WriteRequest request) throws Exception { if (LOGGER.isDebugEnabled()) - LOGGER.debug("session {} sent {}", session, request); + LOGGER.debug("session {} ack {}", session, request); if (request instanceof EncryptedWriteRequest) { EncryptedWriteRequest e = EncryptedWriteRequest.class.cast(request); @@ -236,10 +270,13 @@ public class SSL2Filter extends IoFilterAdapter { } } + /** + * {@inheritDoc} + */ @Override public void filterWrite(NextFilter next, IoSession session, WriteRequest request) throws Exception { - - LOGGER.debug("session {} write {}", session, request); + if (LOGGER.isDebugEnabled()) + LOGGER.debug("session {} write {}", session, request); if (request instanceof EncryptedWriteRequest) { super.filterWrite(next, session, request); diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2HandlerG0.java b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2HandlerG0.java index bf2fd6d..588ab68 100644 --- a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2HandlerG0.java +++ b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2HandlerG0.java @@ -425,6 +425,7 @@ public class SSL2HandlerG0 extends SSL2Handler { */ synchronized protected void lfinish(final NextFilter next) { this.mHandshakeComplete = true; + this.mSession.setAttribute(SSL2Filter.SSL_SECURED, this); next.event(this.mSession, SslEvent.SECURED); } @@ -459,14 +460,14 @@ public class SSL2HandlerG0 extends SSL2Handler { * {@inheritDoc} */ synchronized public void close(final NextFilter next) throws SSLException { - if (mEngine.isOutboundDone()) + if (this.mEngine.isOutboundDone()) return; if (LOGGER.isDebugEnabled()) { LOGGER.debug("{} close() - closing session", toString()); } - mEngine.closeOutbound(); + this.mEngine.closeOutbound(); this.qwrite(next); } diff --git a/mina-core/src/main/java/org/apache/mina/transport/socket/nio/NioSocketSession.java b/mina-core/src/main/java/org/apache/mina/transport/socket/nio/NioSocketSession.java index fb04fa4..34064a3 100644 --- a/mina-core/src/main/java/org/apache/mina/transport/socket/nio/NioSocketSession.java +++ b/mina-core/src/main/java/org/apache/mina/transport/socket/nio/NioSocketSession.java @@ -344,12 +344,6 @@ class NioSocketSession extends NioSession { */ @Override public final boolean isSecured() { - SslFilter s = SslFilter.class.cast(getFilterChain().get(SslFilter.class)); - if (s != null) { - return s.isSecured(this); - } else { - SSL2Handler x = SSL2Handler.class.cast(this.getAttribute(SSL2Filter.SSL_HANDLER)); - return x != null ? x.isConnected() : false; - } + return (this.getAttribute(SSL2Filter.SSL_SECURED) != null); } }