You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2020/10/29 10:55:30 UTC

[httpcomponents-core] branch tls_upgrade_redesign created (now 030040a)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch tls_upgrade_redesign
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git.


      at 030040a  Revised TLS session initialization in I/O reactor code

This branch includes the following new commits:

     new 030040a  Revised TLS session initialization in I/O reactor code

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[httpcomponents-core] 01/01: Revised TLS session initialization in I/O reactor code

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch tls_upgrade_redesign
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit 030040a6cd332f288f44178eb39c53dece62e03d
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Thu Oct 29 10:26:57 2020 +0100

    Revised TLS session initialization in I/O reactor code
---
 .../http/impl/nio/AbstractHttp1StreamDuplexer.java |  6 ++-
 .../hc/core5/reactor/InternalDataChannel.java      | 63 ++++++++++++----------
 .../apache/hc/core5/reactor/ssl/SSLIOSession.java  | 53 ++++++++++--------
 3 files changed, 69 insertions(+), 53 deletions(-)

diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java
index d8b27d6..7d63e08 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java
@@ -230,8 +230,10 @@ abstract class AbstractHttp1StreamDuplexer<IncomingMessage extends HttpMessage,
     }
 
     public final void onConnect() throws HttpException, IOException {
-        connState = ConnectionState.ACTIVE;
-        processCommands();
+        if (connState == ConnectionState.READY) {
+            connState = ConnectionState.ACTIVE;
+            processCommands();
+        }
     }
 
     IncomingMessage parseMessageHead(final boolean endOfStream) throws IOException, HttpException {
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
index 583220d..ebdad0f 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
@@ -58,7 +58,6 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
     private final IOSessionListener sessionListener;
     private final AtomicReference<SSLIOSession> tlsSessionRef;
     private final Queue<InternalDataChannel> closedSessions;
-    private final AtomicBoolean connected;
     private final AtomicBoolean closed;
 
     InternalDataChannel(
@@ -71,7 +70,6 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         this.closedSessions = closedSessions;
         this.sessionListener = sessionListener;
         this.tlsSessionRef = new AtomicReference<>(null);
-        this.connected = new AtomicBoolean(false);
         this.closed = new AtomicBoolean(false);
     }
 
@@ -95,6 +93,11 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         ioSession.upgrade(handler);
     }
 
+    private IOSession getSessionImpl() {
+        final SSLIOSession tlsSession = tlsSessionRef.get();
+        return tlsSession != null ? tlsSession : ioSession;
+    }
+
     private IOEventHandler ensureHandler(final IOSession session) {
         final IOEventHandler handler = session.getHandler();
         Asserts.notNull(handler, "IO event handler");
@@ -107,7 +110,7 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         final IOSession currentSession = tlsSession != null ? tlsSession : ioSession;
         if ((readyOps & SelectionKey.OP_CONNECT) != 0) {
             currentSession.clearEvent(SelectionKey.OP_CONNECT);
-            if (tlsSession == null && connected.compareAndSet(false, true)) {
+            if (tlsSession == null) {
                 if (sessionListener != null) {
                     sessionListener.connected(this);
                 }
@@ -144,8 +147,7 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         if (sessionListener != null) {
             sessionListener.timeout(this);
         }
-        final SSLIOSession tlsSession = tlsSessionRef.get();
-        final IOSession currentSession = tlsSession != null ? tlsSession : ioSession;
+        final IOSession currentSession = getSessionImpl();
         final IOEventHandler handler = ensureHandler(currentSession);
         handler.timeout(this, timeout);
     }
@@ -155,14 +157,25 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         if (sessionListener != null) {
             sessionListener.exception(this, cause);
         }
-        final SSLIOSession tlsSession = tlsSessionRef.get();
-        final IOSession currentSession = tlsSession != null ? tlsSession : ioSession;
+        final IOSession currentSession = getSessionImpl();
         final IOEventHandler handler = currentSession.getHandler();
         if (handler != null) {
             handler.exception(this, cause);
         }
     }
 
+    void onTLSSessionStart() {
+        if (sessionListener != null) {
+            sessionListener.connected(this);
+        }
+    }
+
+    void onTLSSessionEnd() {
+        if (closed.compareAndSet(false, true)) {
+            closedSessions.add(this);
+        }
+    }
+
     void disconnected() {
         if (sessionListener != null) {
             sessionListener.disconnected(this);
@@ -183,6 +196,17 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
             final SSLSessionInitializer initializer,
             final SSLSessionVerifier verifier,
             final Timeout handshakeTimeout) {
+        startTlsInternal(sslContext, endpoint, sslBufferMode, initializer, verifier, handshakeTimeout, null);
+    }
+
+    void startTlsInternal(
+            final SSLContext sslContext,
+            final NamedEndpoint endpoint,
+            final SSLBufferMode sslBufferMode,
+            final SSLSessionInitializer initializer,
+            final SSLSessionVerifier verifier,
+            final Timeout handshakeTimeout,
+            final Callback<SSLIOSession> callback) {
         if (tlsSessionRef.compareAndSet(null, new SSLIOSession(
                 endpoint != null ? endpoint : initialEndpoint,
                 ioSession,
@@ -195,19 +219,9 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
 
                     @Override
                     public void execute(final SSLIOSession sslSession) {
-                        if (connected.compareAndSet(false, true)) {
-                            final IOEventHandler handler = ensureHandler(ioSession);
-                            try {
-                                if (sessionListener != null) {
-                                    sessionListener.connected(InternalDataChannel.this);
-                                }
-                                handler.connected(InternalDataChannel.this);
-                            } catch (final Exception ex) {
-                                if (sessionListener != null) {
-                                    sessionListener.exception(InternalDataChannel.this, ex);
-                                }
-                                handler.exception(InternalDataChannel.this, ex);
-                            }
+                        onTLSSessionStart();
+                        if (callback != null) {
+                            callback.execute(sslSession);
                         }
                     }
 
@@ -216,9 +230,7 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
 
                     @Override
                     public void execute(final SSLIOSession sslSession) {
-                        if (closed.compareAndSet(false, true)) {
-                            closedSessions.add(InternalDataChannel.this);
-                        }
+                        onTLSSessionEnd();
                     }
 
                 },
@@ -243,11 +255,6 @@ final class InternalDataChannel extends InternalChannel implements ProtocolIOSes
         return ioSession.getLock();
     }
 
-    private IOSession getSessionImpl() {
-        final SSLIOSession tlsSession = tlsSessionRef.get();
-        return tlsSession != null ? tlsSession : ioSession;
-    }
-
     @Override
     public void close() {
         close(CloseMode.GRACEFUL);
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
index 806794c..e6a40b4 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
@@ -80,11 +80,13 @@ public class SSLIOSession implements IOSession {
     private final SSLManagedBuffer inPlain;
     private final SSLSessionInitializer initializer;
     private final SSLSessionVerifier verifier;
-    private final Callback<SSLIOSession> connectedCallback;
-    private final Callback<SSLIOSession> disconnectedCallback;
+    private final Callback<SSLIOSession> sessionStartCallback;
+    private final Callback<SSLIOSession> sessionEndCallback;
     private final Timeout connectTimeout;
     private final SSLMode sslMode;
     private final AtomicInteger outboundClosedCount;
+    private final IOEventHandler internalEventHandler;
+
     private int appEventMask;
 
     private volatile boolean endOfStream;
@@ -115,8 +117,8 @@ public class SSLIOSession implements IOSession {
             final SSLBufferMode sslBufferMode,
             final SSLSessionInitializer initializer,
             final SSLSessionVerifier verifier,
-            final Callback<SSLIOSession> connectedCallback,
-            final Callback<SSLIOSession> disconnectedCallback,
+            final Callback<SSLIOSession> sessionStartCallback,
+            final Callback<SSLIOSession> sessionEndCallback,
             final Timeout connectTimeout) {
         super();
         Args.notNull(session, "IO session");
@@ -126,8 +128,8 @@ public class SSLIOSession implements IOSession {
         this.sslMode = sslMode;
         this.initializer = initializer;
         this.verifier = verifier;
-        this.connectedCallback = connectedCallback;
-        this.disconnectedCallback = disconnectedCallback;
+        this.sessionStartCallback = sessionStartCallback;
+        this.sessionEndCallback = sessionEndCallback;
 
         this.appEventMask = session.getEventMask();
         if (this.sslMode == SSLMode.CLIENT && targetEndpoint != null) {
@@ -147,17 +149,7 @@ public class SSLIOSession implements IOSession {
         this.inPlain = SSLManagedBuffer.create(sslBufferMode, appBufferSize);
         this.outboundClosedCount = new AtomicInteger(0);
         this.connectTimeout = connectTimeout;
-    }
-
-    private IOEventHandler ensureHandler() {
-        final IOEventHandler handler = session.getHandler();
-        Asserts.notNull(handler, "IO event handler");
-        return handler;
-    }
-
-    @Override
-    public IOEventHandler getHandler() {
-        return new IOEventHandler() {
+        this.internalEventHandler = new IOEventHandler() {
 
             @Override
             public void connected(final IOSession protocolSession) throws IOException {
@@ -214,9 +206,21 @@ public class SSLIOSession implements IOSession {
             }
 
         };
+
     }
 
-    private void initialize() throws SSLException {
+    private IOEventHandler ensureHandler() {
+        final IOEventHandler handler = session.getHandler();
+        Asserts.notNull(handler, "IO event handler");
+        return handler;
+    }
+
+    @Override
+    public IOEventHandler getHandler() {
+        return internalEventHandler;
+    }
+
+    private void initialize() throws IOException {
         Asserts.check(!this.initialized, "SSL I/O session already initialized");
 
         // Save the initial socketTimeout of the underlying IOSession, to be restored after the handshake is finished
@@ -292,7 +296,7 @@ public class SSLIOSession implements IOSession {
         }
     }
 
-    private void doHandshake() throws SSLException {
+    private void doHandshake() throws IOException {
         boolean handshaking = true;
 
         SSLEngineResult result = null;
@@ -380,8 +384,11 @@ public class SSLIOSession implements IOSession {
                 final String applicationProtocol = ReflectionUtils.callGetter(this.sslEngine, "ApplicationProtocol", String.class);
                 this.tlsDetails = new TlsDetails(sslSession, applicationProtocol);
             }
-            if (this.connectedCallback != null) {
-                this.connectedCallback.execute(this);
+
+            ensureHandler().connected(this);
+
+            if (this.sessionStartCallback != null) {
+                this.sessionStartCallback.execute(this);
             }
         }
     }
@@ -409,8 +416,8 @@ public class SSLIOSession implements IOSession {
             }
             if (this.status == Status.CLOSED) {
                 this.session.close();
-                if (disconnectedCallback != null) {
-                    disconnectedCallback.execute(this);
+                if (sessionEndCallback != null) {
+                    sessionEndCallback.execute(this);
                 }
                 return;
             }