jetty-9 sbordet inspired ssl cleanups

jetty-io/src/main/java/org/eclipse/jetty/io/AbstractAsyncConnection.java

@@ -109,6 +109,6 @@ public abstract class AbstractAsyncConnection implements AsyncConnection
     @Override
     public String toString()
     {
-        return String.format("%s@%x", getClass().getSimpleName(), hashCode());
+        return String.format("%s@%x{%s}", getClass().getSimpleName(), hashCode(),_readInterested.get()?"R":"");
     }
 }

jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java

@@ -63,7 +63,6 @@ abstract public class WriteFlusher
         }
         catch (IOException e)
         {
-            e.printStackTrace();
             if (!_writing.compareAndSet(true,false))
                 throw new ConcurrentModificationException(e);
             callback.failed(context,e);

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/ReadState.java

@@ -1,19 +0,0 @@
-// ========================================================================
-// Copyright (c) 2012 Mort Bay Consulting Pty. Ltd.
-// ------------------------------------------------------------------------
-// All rights reserved. This program and the accompanying materials
-// are made available under the terms of the Eclipse Public License v1.0
-// and Apache License v2.0 which accompanies this distribution.
-// The Eclipse Public License is available at
-// http://www.eclipse.org/legal/epl-v10.html
-// The Apache License v2.0 is available at
-// http://www.opensource.org/licenses/apache2.0.php
-// You may elect to redistribute this code under either of these licenses.
-// ========================================================================
-
-package org.eclipse.jetty.io.ssl;
-
-enum ReadState
-{
-    HANDSHAKING, HANDSHAKEN, IDLE, UNDERFLOW, DECRYPTED, CLOSED
-}

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SSLMachine.java

@@ -1,265 +0,0 @@
-package org.eclipse.jetty.io.ssl;
-
-import java.nio.ByteBuffer;
-import javax.net.ssl.SSLEngine;
-import javax.net.ssl.SSLEngineResult;
-import javax.net.ssl.SSLException;
-
-import org.eclipse.jetty.util.BufferUtil;
-import org.eclipse.jetty.util.log.Log;
-import org.eclipse.jetty.util.log.Logger;
-
-public abstract class SSLMachine
-{
-    private static final Logger logger = Log.getLogger(SslConnection.class);
-    private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocate(0);
-    private final Object wrapLock = new Object();
-    private final SSLEngine engine;
-    private boolean handshaken;
-    private boolean remoteClosed;
-
-    public SSLMachine(SSLEngine engine)
-    {
-        this.engine = engine;
-    }
-
-    public ReadState decrypt(ByteBuffer netInput, ByteBuffer appInput) throws SSLException
-    {
-        while (true)
-        {
-            SSLEngineResult.HandshakeStatus handshakeStatus = engine.getHandshakeStatus();
-            logger.debug("Reading from {}, handshake status: {}", netInput, handshakeStatus);
-            switch (handshakeStatus)
-            {
-                case NEED_UNWRAP:
-                {
-                    ReadState result = unwrap(netInput, appInput);
-                    if (result == null)
-                        break;
-                    return result;
-                }
-                case NEED_TASK:
-                {
-                    executeTasks();
-                    break;
-                }
-                case NEED_WRAP:
-                {
-                    writeForDecrypt(EMPTY_BUFFER);
-                    break;
-                }
-                case NOT_HANDSHAKING:
-                {
-                    ReadState result = unwrap(netInput, appInput);
-                    if (result == null)
-                        break;
-                    return result;
-                }
-                default:
-                    throw new IllegalStateException();
-            }
-        }
-    }
-
-    public WriteState encrypt(ByteBuffer appOutput, ByteBuffer netOutput) throws SSLException
-    {
-        return wrap(appOutput, netOutput);
-    }
-
-    protected abstract void writeForDecrypt(ByteBuffer appOutput);
-
-    private ReadState unwrap(ByteBuffer netInput, ByteBuffer appInput) throws SSLException
-    {
-        boolean decrypted = false;
-        while (netInput.hasRemaining())
-        {
-            logger.debug("Decrypting from {} to {}", netInput, appInput);
-            SSLEngineResult result = unwrap(engine, netInput, appInput);
-            logger.debug("Decrypted from {} to {}, result {}", netInput, appInput, result);
-            switch (result.getStatus())
-            {
-                case OK:
-                {
-                    SSLEngineResult.HandshakeStatus handshakeStatus = result.getHandshakeStatus();
-                    if (handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED)
-                    {
-                        if (engine.getUseClientMode())
-                        {
-                            logger.debug("Handshake finished (client), new SSL session");
-                            handshaken = true;
-                            return ReadState.HANDSHAKEN;
-                        }
-                        else
-                        {
-                            if (handshaken)
-                            {
-                                logger.debug("Rehandshake finished (server)");
-                            }
-                            else
-                            {
-                                logger.debug("Handshake finished (server), cached SSL session");
-                                handshaken = true;
-                                return ReadState.HANDSHAKEN;
-                            }
-                        }
-                    }
-
-                    if (result.bytesProduced() > 0)
-                    {
-                        decrypted = true;
-                        continue;
-                    }
-
-                    if (handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_UNWRAP)
-                        continue;
-
-                    if (handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED ||
-                            handshakeStatus == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING)
-                        break;
-
-                    return null;
-                }
-                case BUFFER_UNDERFLOW:
-                {
-                    return decrypted ? ReadState.DECRYPTED : ReadState.UNDERFLOW;
-                }
-                case CLOSED:
-                {
-                    // We have read the SSL close message and updated the SSLEngine
-                    logger.debug("Close alert received from remote peer");
-                    remoteClosed = true;
-                    return decrypted ? ReadState.DECRYPTED : ReadState.CLOSED;
-                }
-                default:
-                    throw new IllegalStateException();
-            }
-        }
-
-        SSLEngineResult.HandshakeStatus handshakeStatus = engine.getHandshakeStatus();
-        if (handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_UNWRAP ||
-                engine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING)
-            return ReadState.UNDERFLOW;
-
-        return null;
-    }
-
-    private SSLEngineResult unwrap(SSLEngine engine, ByteBuffer netInput, ByteBuffer appInput) throws SSLException
-    {
-        int position = BufferUtil.flipToFill(appInput);
-        try
-        {
-            return engine.unwrap(netInput, appInput);
-        }
-        finally
-        {
-            BufferUtil.flipToFlush(appInput, position);
-        }
-    }
-
-    private void executeTasks()
-    {
-        Runnable task;
-        while ((task = engine.getDelegatedTask()) != null)
-        {
-            task.run();
-            logger.debug("Executed task: {}", task);
-        }
-    }
-
-    private WriteState wrap(ByteBuffer appOutput, ByteBuffer netOutput) throws SSLException
-    {
-        while (true)
-        {
-            // Locking is important because application code may call handshake() (to rehandshake)
-            // followed by encrypt(). In this case, the handshake() call may need to wrap, then
-            // unwrap and then wrap again to perform the handshake, and the second wrap may be
-            // concurrent with the wrap triggered by encrypt(), which would corrupt SSLEngine.
-            // It is also important that wrap and write are atomic, because SSL packets needs to
-            // be ordered (and therefore the packet created first must be written before the packet
-            // created second).
-            SSLEngineResult result;
-            synchronized (wrapLock)
-            {
-                logger.debug("Encrypting from {} to {}", appOutput, netOutput);
-                result = wrap(engine, appOutput, netOutput);
-                logger.debug("Encrypted from {} to {}, result: {}", appOutput, netOutput, result);
-
-/*
-                if (result.bytesProduced() > 0)
-                {
-                    try
-                    {
-                        netOutput.flip();
-                        write(netOutput);
-                        netOutput.clear();
-                    }
-                    catch (RuntimeIOException x)
-                    {
-                        // If we try to write the SSL close message but we cannot
-                        // because the other peer has already closed the connection,
-                        // then ignore the exception and continue
-                        if (result.getStatus() != SSLEngineResult.Status.CLOSED)
-                            throw x;
-                    }
-                }
-*/
-            }
-
-            if (result.getStatus() == SSLEngineResult.Status.CLOSED)
-                return WriteState.CLOSED;
-
-            SSLEngineResult.HandshakeStatus handshakeStatus = result.getHandshakeStatus();
-            if (handshakeStatus == SSLEngineResult.HandshakeStatus.NEED_WRAP)
-                continue;
-
-            if (handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED)
-            {
-                if (engine.getUseClientMode())
-                {
-                    if (handshaken)
-                    {
-                        logger.debug("Rehandshake finished (client)");
-                    }
-                    else
-                    {
-                        logger.debug("Handshake finished (client), cached SSL session");
-                        handshaken = true;
-                        return WriteState.HANDSHAKEN;
-                    }
-                }
-                else
-                {
-                    logger.debug("Handshake finished (server), new SSL session");
-                    assert !appOutput.hasRemaining();
-                    handshaken = true;
-                    return WriteState.HANDSHAKEN;
-                }
-            }
-
-            if (!appOutput.hasRemaining())
-                return null;
-        }
-    }
-
-    private SSLEngineResult wrap(SSLEngine engine, ByteBuffer appOutput, ByteBuffer netOutput) throws SSLException
-    {
-        int position = BufferUtil.flipToFill(netOutput);
-        try
-        {
-            return engine.wrap(appOutput, netOutput);
-        }
-        finally
-        {
-            BufferUtil.flipToFlush(netOutput, position);
-        }
-    }
-
-    public boolean isRemoteClosed()
-    {
-        return remoteClosed;
-    }
-
-    public void close()
-    {
-    }
-}

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java

@@ -131,16 +131,29 @@ public class SslConnection extends AbstractAsyncConnection
         if (_appEndPoint._readInterest.isInterested())
             _appEndPoint._readInterest.readable();
 
-        else if (_appEndPoint._writeFlusher.isWriting() && _sslEngine.getHandshakeStatus()!=HandshakeStatus.NOT_HANDSHAKING)
         // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read
+        if (_appEndPoint._writeFlusher.isWriting() && _appEndPoint._flushUnwrap)
+        {
+            _appEndPoint._flushUnwrap=false;
             _appEndPoint._writeFlusher.completeWrite();
         }
+    }
 
     /* ------------------------------------------------------------ */
     @Override
     public void onReadFail(Throwable cause)
     {
         super.onReadFail(cause);
+        
+        if (_appEndPoint._readInterest.isInterested())
+            _appEndPoint._readInterest.failed(cause);
+        
+        if (_appEndPoint._writeFlusher.isWriting() && _appEndPoint._flushUnwrap)
+        {
+            _appEndPoint._flushUnwrap=false;
+            _appEndPoint._writeFlusher.failed(cause);
+        }
+        
     }
 
     /* ------------------------------------------------------------ */
@@ -158,8 +171,9 @@ public class SslConnection extends AbstractAsyncConnection
     public class SslEndPoint extends AbstractEndPoint implements AsyncEndPoint
     {
         private AsyncConnection _connection;
-        private boolean _fillWrite;
+        private boolean _fillWrap;
-        private boolean _writing;
+        private boolean _flushUnwrap;
+        private boolean _netWriting;
         private boolean _underflown;
         private boolean _ishut=false;
         
@@ -170,18 +184,14 @@ public class SslConnection extends AbstractAsyncConnection
             {      
                 synchronized (SslEndPoint.this)
                 {
-                    LOG.debug("{} write.complete {}",SslConnection.this,_writing?(_fillWrite?"FW":"F"):(_fillWrite?"W":""));
+                    LOG.debug("{} write.complete {}",SslConnection.this,_netWriting?(_fillWrap?"FW":"F"):(_fillWrap?"W":""));
 
-                    if (_netOut==null && !_netOut.hasRemaining())
+                    releaseNetOut();
-                    {
-                        _bufferPool.release(_netOut);
-                        _netOut=null;
-                    }
 
-                    _writing=false;
+                    _netWriting=false;
-                    if (_fillWrite)
+                    if (_fillWrap)
                     {
-                        _fillWrite=false;
+                        _fillWrap=false;
                         _readInterest.readable();
                     }
                     
@@ -196,18 +206,22 @@ public class SslConnection extends AbstractAsyncConnection
                 synchronized (SslEndPoint.this)
                 {
                     LOG.debug("{} write.failed",SslConnection.this,x);
-                    _writing=false;
+                    if (_netOut!=null)
-                    if (_fillWrite)
+                        BufferUtil.clear(_netOut);
+                    releaseNetOut();
+                    _netWriting=false;
+                    if (_fillWrap)
                     {
-                        _fillWrite=false;
+                        _fillWrap=false;
                         _readInterest.failed(x);
                     }
                     
                     if (_writeFlusher.isWriting())
                         _writeFlusher.failed(x);
-                }           
-            }
                     
+                    // TODO release all buffers??? or may in onClose
+                }           
+            }
         };
         
         private final ReadInterest _readInterest = new ReadInterest()
@@ -225,22 +239,23 @@ public class SslConnection extends AbstractAsyncConnection
                     if (!_underflown && BufferUtil.hasContent(_netIn))
                         return true;
                     
-                    
                     // So we are not read ready
                     
                     // Are we actually write blocked?
-                    if (_sslEngine.getHandshakeStatus()==HandshakeStatus.NEED_WRAP )
+                    if (_fillWrap )
                     {
                         // we must be blocked trying to write before we can read
                         
                         // Do we don't have some netdata to write
                         if (BufferUtil.isEmpty(_netOut))
-                            // pretend we are readable so the wrap is done by next fill call
+                        {
+                            // pretend we are readable so the wrap is done by next readable callback
+                            _fillWrap=false;
                             return true;
+                        }
                         
                         // otherwise write the net data
-                        _fillWrite=true;
+                        _netWriting=true;
-                        _writing=true;
                         getEndPoint().write(null,_writeCallback,_netOut);
                     }
                     else
@@ -263,7 +278,7 @@ public class SslConnection extends AbstractAsyncConnection
                     if (BufferUtil.hasContent(_netOut))
                     {    
                         // write it
-                        _writing=true;
+                        _netWriting=true;
                         getEndPoint().write(null,_writeCallback,_netOut);
                     }
                     else if (_sslEngine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP )
@@ -343,7 +358,6 @@ public class SslConnection extends AbstractAsyncConnection
                             throw new IllegalStateException();
 
                         case CLOSED:
-
                             // Dang! we have to care about the handshake state
                             switch(_sslEngine.getHandshakeStatus())
                             {
@@ -357,9 +371,11 @@ public class SslConnection extends AbstractAsyncConnection
 
                                 case NEED_WRAP:
                                     // we need to send some handshake data
-                                    _fillWrite=true;
+                                    if (!_flushUnwrap)
+                                    {
+                                        _fillWrap=true;
                                         flush(BufferUtil.EMPTY_BUFFER);
-                                    getEndPoint().close();
+                                    }
                                     return -1;
 
                                 default:
@@ -397,7 +413,7 @@ public class SslConnection extends AbstractAsyncConnection
                                 case NEED_WRAP:
                                     // TODO maybe just do the wrap here ourselves?
                                     // we need to send some handshake data
-                                    _fillWrite=true;
+                                    _fillWrap=true;
                                     flush(BufferUtil.EMPTY_BUFFER);
                                     continue;
 
@@ -450,7 +466,7 @@ public class SslConnection extends AbstractAsyncConnection
             LOG.debug("{} flush",SslConnection.this);
             try
             {
-                if (_writing)
+                if (_netWriting)
                     return 0;
                     
                 // We will need a network buffer
@@ -470,7 +486,12 @@ public class SslConnection extends AbstractAsyncConnection
                     switch(_wrapResult.getStatus())
                     {
                         case CLOSED:
-                            if (appOuts.length==1 && appOuts[0]==BufferUtil.EMPTY_BUFFER)
+                            if (BufferUtil.hasContent(_netOut))
+                            {
+                                _netWriting=true;
+                                getEndPoint().write(null,_writeCallback,_netOut);
+                            }
+                            if (_fillWrap)
                                 return 0;
                             throw new EofException();
 
@@ -507,10 +528,12 @@ public class SslConnection extends AbstractAsyncConnection
                                     continue;
 
                                 case NEED_UNWRAP:
-                                    // TODO maybe just do the unwrap here ourselves?
                                     // Were we were not called from fill and not reading anyway
-                                    if ((appOuts.length!=1 || appOuts[0]!=BufferUtil.EMPTY_BUFFER) && !_readInterest.isInterested())
+                                    if (!_fillWrap && !_readInterest.isInterested())
+                                    {
+                                        _flushUnwrap=true;
                                         fill(BufferUtil.EMPTY_BUFFER);
+                                    }
                                     return 0;
 
                                 case FINISHED:
@@ -528,14 +551,35 @@ public class SslConnection extends AbstractAsyncConnection
             finally
             {
                 LOG.debug("{} !flush",SslConnection.this);
+                releaseNetOut();
             }
         }
 
+        private void releaseNetOut()
+        {
+            if (_netOut!=null && !_netOut.hasRemaining())
+            {
+                _bufferPool.release(_netOut);
+                _netOut=null;
+                if (_sslEngine.isOutboundDone())
+                    getEndPoint().shutdownOutput();
+            }
+        }
+        
+        
         @Override
         public void shutdownOutput()
         {
             _sslEngine.closeOutbound();
-            // TODO then?
+            try
+            {
+                flush(BufferUtil.EMPTY_BUFFER);
+            }
+            catch(IOException e)
+            {
+                LOG.ignore(e);
+                getEndPoint().close();
+            }
         }
 
         @Override
@@ -583,7 +627,7 @@ public class SslConnection extends AbstractAsyncConnection
         @Override 
         public String toString()
         {
-            return String.format("%s{%s%s%s}",super.toString(),_readInterest.isInterested()?"R":"",_writeFlusher.isWriting()?"W":"",_writing?"w":"");
+            return String.format("%s{%s%s%s}",super.toString(),_readInterest.isInterested()?"R":"",_writeFlusher.isWriting()?"W":"",_netWriting?"w":"");
         }
     }
 }

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/WriteState.java

@@ -1,19 +0,0 @@
-// ========================================================================
-// Copyright (c) 2012 Mort Bay Consulting Pty. Ltd.
-// ------------------------------------------------------------------------
-// All rights reserved. This program and the accompanying materials
-// are made available under the terms of the Eclipse Public License v1.0
-// and Apache License v2.0 which accompanies this distribution.
-// The Eclipse Public License is available at
-// http://www.eclipse.org/legal/epl-v10.html
-// The Apache License v2.0 is available at
-// http://www.opensource.org/licenses/apache2.0.php
-// You may elect to redistribute this code under either of these licenses.
-// ========================================================================
-
-package org.eclipse.jetty.io.ssl;
-
-enum WriteState
-{
-    HANDSHAKING, HANDSHAKEN, IDLE, ENCRYPTED, CLOSED
-}

jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointTest.java

@@ -196,7 +196,7 @@ public class SelectChannelEndPointTest
                 }
                 catch(Exception e2)
                 {
-                    e2.printStackTrace();
+                    // e2.printStackTrace();
                 }
             }
             catch(InterruptedException|EofException e)
@@ -214,12 +214,6 @@ public class SelectChannelEndPointTest
             }
         }
 
-        @Override
-        public String toString()
-        {
-            return String.format("%s{}",
-                    super.toString());
-        }
     }
 
 
@@ -421,16 +415,12 @@ public class SelectChannelEndPointTest
         assertTrue(idle>400);
         assertTrue(idle<2000);
 
-        // But endpoint is still open.
+        // But endpoint may still be open for a little bit.
-        assertTrue(_lastEndp.isOpen());
+        if (_lastEndp.isOpen());
-
-
-        // Wait for another idle callback
             Thread.sleep(2000);
+
         // endpoint is closed.
-
         assertFalse(_lastEndp.isOpen());
-
     }