You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by jo...@apache.org on 2021/08/04 19:50:21 UTC

[mina] branch bugfix/DIRMINA1132 updated: Fixes Issue with SSLEngine emit FINISHED twice in conjunction with needing to loop the receive buffer to consume more data. Prevents accidently freeing of the ZERO buffer. Enables receive() recursion from within a receive -> write -> finish -> receive loop.

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 8d076dc  Fixes Issue with SSLEngine emit FINISHED twice in conjunction with needing to loop the receive buffer to consume more data. Prevents accidently freeing of the ZERO buffer. Enables receive() recursion from within a receive -> write -> finish -> receive loop.
8d076dc is described below

commit 8d076dcd12e97fe1ed984a1e34474aa6469c6036
Author: Jonathan Valliere <jo...@apache.org>
AuthorDate: Wed Aug 4 15:50:18 2021 -0400

    Fixes Issue with SSLEngine emit FINISHED twice in conjunction with
    needing to loop the receive buffer to consume more data.
    Prevents accidently freeing of the ZERO buffer.
    Enables receive() recursion from within a receive -> write -> finish ->
    receive loop.
---
 .../org/apache/mina/filter/ssl2/SSL2Handler.java   | 14 ++++++----
 .../org/apache/mina/filter/ssl2/SSL2HandlerG0.java | 30 +++++++++++++++++-----
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Handler.java b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Handler.java
index 4e04c58..4206ba6 100644
--- a/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Handler.java
+++ b/mina-core/src/main/java/org/apache/mina/filter/ssl2/SSL2Handler.java
@@ -186,12 +186,14 @@ public abstract class SSL2Handler {
 	 */
 	protected IoBuffer resume_decode_buffer(IoBuffer source) {
 		if (mDecodeBuffer == null)
-			if (source == null)
+			if (source == null) {
 				return ZERO;
-			else
+			} else {
+				mDecodeBuffer = source;
 				return source;
+			}
 		else {
-			if (source != null) {
+			if (source != null && source != ZERO) {
 				mDecodeBuffer.expand(source.remaining());
 				mDecodeBuffer.put(source);
 				source.free();
@@ -207,7 +209,7 @@ public abstract class SSL2Handler {
 	 * @param source the buffer previously returned by
 	 *               {@link #resume_decode_buffer(IoBuffer)}
 	 */
-	protected void save_decode_buffer(IoBuffer source) {
+	protected void suspend_decode_buffer(IoBuffer source) {
 		if (source.hasRemaining()) {
 			if (source.isDerived()) {
 				this.mDecodeBuffer = IoBuffer.allocate(source.remaining());
@@ -217,7 +219,9 @@ public abstract class SSL2Handler {
 				this.mDecodeBuffer = source;
 			}
 		} else {
-			source.free();
+			if (source != ZERO) {
+				source.free();
+			}
 			this.mDecodeBuffer = null;
 		}
 	}
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 8b203c8..1b308aa 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
@@ -106,9 +106,14 @@ public class SSL2HandlerG0 extends SSL2Handler {
 			try {
 				this.qreceive(next, source);
 			} finally {
-				save_decode_buffer(source);
+				suspend_decode_buffer(source);
 				this.mDecodeThread = null;
 			}
+		} else {
+			if (LOGGER.isDebugEnabled()) {
+				LOGGER.debug("{} receive() - recursion", toString());
+			}
+			this.qreceive(next, this.mDecodeBuffer);
 		}
 	}
 
@@ -125,7 +130,7 @@ public class SSL2HandlerG0 extends SSL2Handler {
 			LOGGER.debug("{} qreceive() - source {}", toString(), message);
 		}
 
-		final IoBuffer source = message == null ? ZERO : message;
+		final IoBuffer source = message;
 		final IoBuffer dest = allocate_app_buffer(source.remaining());
 
 		final SSLEngineResult result = mEngine.unwrap(source.buf(), dest.buf());
@@ -146,6 +151,15 @@ public class SSL2HandlerG0 extends SSL2Handler {
 		}
 
 		switch (result.getHandshakeStatus()) {
+			case NEED_UNWRAP:
+			case NEED_UNWRAP_AGAIN:
+				if (result.bytesConsumed() != 0 && message.hasRemaining()) {
+					if (LOGGER.isDebugEnabled()) {
+						LOGGER.debug("{} qreceive() - handshake needs unwrap, looping", toString());
+					}
+					this.qreceive(next, message);
+				}
+				break;
 			case NEED_TASK:
 				if (LOGGER.isDebugEnabled()) {
 					LOGGER.debug("{} qreceive() - handshake needs task, scheduling", toString());
@@ -163,10 +177,9 @@ public class SSL2HandlerG0 extends SSL2Handler {
 					LOGGER.debug("{} qreceive() - handshake finished, flushing queue", toString());
 				}
 				this.lfinish(next);
-			case NEED_UNWRAP:
-			case NEED_UNWRAP_AGAIN:
+				break;
 			case NOT_HANDSHAKING:
-				if (result.bytesProduced() != 0 && message.hasRemaining()) {
+				if ((result.bytesProduced() != 0 || result.bytesConsumed() != 0) && message.hasRemaining()) {
 					if (LOGGER.isDebugEnabled()) {
 						LOGGER.debug("{} qreceive() - trying to decode more messages, looping", toString());
 					}
@@ -430,9 +443,12 @@ public class SSL2HandlerG0 extends SSL2Handler {
 			this.mHandshakeComplete = true;
 			this.mSession.setAttribute(SSL2Filter.SSL_SECURED, this);
 			next.event(this.mSession, SslEvent.SECURED);
-			this.flush(next);
-			this.receive(next, ZERO);
 		}
+		/**
+		 * There exists a bug in the JDK which emits FINISHED twice instead of once.
+		 */
+		this.receive(next, ZERO);
+		this.flush(next);
 	}
 
 	/**