This is the 'fix' for the IceStorm related bug having to do with oneways.

    It requires that Marc make it legal for read() to return 0 bytes.

1 files changed, 152 insertions, 160 deletions

diff --git a/cpp/src/Ice/SslConnectionOpenSSLClient.cpp b/cpp/src/Ice/SslConnectionOpenSSLClient.cpp
index acd736de782..e13759f3fbd 100644
--- a/cpp/src/Ice/SslConnectionOpenSSLClient.cpp
+++ b/cpp/src/Ice/SslConnectionOpenSSLClient.cpp
@@ -36,8 +36,6 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::~ClientConnection()
 {
     ICE_METHOD_INV("OpenSSL::ClientConnection::~ClientConnection()");
 
-    shutdown();
-
     ICE_METHOD_RET("OpenSSL::ClientConnection::~ClientConnection()");
 }
 
@@ -46,27 +44,6 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::shutdown()
 {
     ICE_METHOD_INV("OpenSSL::ClientConnection::shutdown()");
 
-    if (_sslConnection != 0)
-    {
-        int shutdown = 0;
-        int retries = 100;
-
-        do
-        {
-            shutdown = SSL_shutdown(_sslConnection);
-            retries--;
-        }
-        while ((shutdown == 0) && (retries > 0));
-
-        if (shutdown <= 0)
-        {
-            ostringstream s;
-            s << "SSL shutdown failure encountered: code[" << shutdown << "] retries[";
-            s << retries << "]\n" << fdToString(SSL_get_fd(_sslConnection));
-            ICE_PROTOCOL_DEBUG(s.str());
-        }
-    }
-
     Connection::shutdown();
 
     ICE_METHOD_RET("OpenSSL::ClientConnection::shutdown()");
@@ -75,25 +52,23 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::shutdown()
 int
 IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout)
 {
-    JTCSyncT<JTCMutex> sync(_initMutex);
-
-    ICE_METHOD_INV("OpenSSL::ClientConnection::init()");

-

-    if (_timeoutEncountered)

-    {

-        throw TimeoutException(__FILE__, __LINE__);

-    }

-

+    ICE_METHOD_INV("OpenSSL::ClientConnection::init()");
+
+    if (_timeoutEncountered)
+    {
+        throw TimeoutException(__FILE__, __LINE__);
+    }
+
     int retCode = SSL_is_init_finished(_sslConnection);
 
     while (!retCode)
     {
-        int i = 0;

-

-        _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout;

-

-        try

-        {

+        int i = 0;
+
+        _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout;
+
+        try
+        {
             if (_initWantRead)
             {
                 i = readSelect(_readTimeout);
@@ -101,12 +76,12 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout)
             else if (_initWantWrite)
             {
                 i = writeSelect(timeout);
-            }

-        }

-        catch (const TimeoutException&)

-        {

-            _timeoutEncountered = true;

-            throw;

+            }
+        }
+        catch (const TimeoutException&)
+        {
+            _timeoutEncountered = true;
+            throw;
         }
 
         if (_initWantRead && i == 0)
@@ -174,12 +149,14 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout)
 
                     if (connectionLost())
                     {
+                        ICE_DEV_DEBUG("ClientConnection::init(): Throwing ConnectionLostException... SslConnectionOpenSSLClient.cpp, 177");
                         ConnectionLostException ex(__FILE__, __LINE__);
                         ex.error = getSocketErrno();
                         throw ex;
                     }
                     else
                     {
+                        ICE_DEV_DEBUG("ClientConnection::init(): Throwing SocketException... SslConnectionOpenSSLClient.cpp, 184");
                         SocketException ex(__FILE__, __LINE__);
                         ex.error = getSocketErrno();
                         throw ex;
@@ -266,8 +243,8 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::write(Buffer& buf, int timeout)
     int totalBytesWritten = 0;
     int bytesWritten = 0;
 
-    int packetSize = buf.b.end() - buf.i;

-

+    int packetSize = buf.b.end() - buf.i;
+
 #ifdef WIN32
     //
     // Limit packet size to avoid performance problems on WIN32.
@@ -283,162 +260,177 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::write(Buffer& buf, int timeout)
     while (buf.i != buf.b.end())
     {
         // Ensure we're initialized.
-        if (init(timeout))
+        int initReturn = initialize(timeout);
+
+        if (initReturn == -1)
         {
-            // Perform a select on the socket.
-            if (!writeSelect(timeout))
-            {
-                // We're done here.
-                break;
-            }
+            // Handshake underway, we should just return with what we've got (even if that's nothing).
+            break;
+        }
+
+        if (initReturn == 0)
+        {
+            // Retry the initialize call
+            continue;
+        }
 
-            bytesWritten = sslWrite((char *)buf.i, packetSize);
+        // initReturn must be > 0, so we're okay to try a write
 
-            switch (getLastError())
+        // Perform a select on the socket.
+        if (!writeSelect(timeout))
+        {
+            // We're done here.
+            break;
+        }
+
+        bytesWritten = sslWrite((char *)buf.i, packetSize);
+
+        switch (getLastError())
+        {
+            case SSL_ERROR_NONE:
             {
-                case SSL_ERROR_NONE:
+                if (bytesWritten > 0)
                 {
-                    if (bytesWritten > 0)
+                    if (_traceLevels->network >= 3)
                     {
-	                if (_traceLevels->network >= 3)

-	                {

-                            ostringstream s;

-                            s << "sent " << bytesWritten << " of " << packetSize;

-                            s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection));

-	                    _logger->trace(_traceLevels->networkCat, s.str());

-                        }

-

-                        totalBytesWritten += bytesWritten;
-
-                        buf.i += bytesWritten;
-
-                        if (packetSize > buf.b.end() - buf.i)
-                        {
-                            packetSize = buf.b.end() - buf.i;
-                        }
+                        ostringstream s;
+                        s << "sent " << bytesWritten << " of " << packetSize;
+                        s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection));
+                       _logger->trace(_traceLevels->networkCat, s.str());
                     }
-                    else
-                    {
-                        // TODO: The client application performs a cleanup at this point,
-                        //       not even shutting down SSL - it just frees the SSL
-                        //       structure. I'm ignoring this, at the moment, as I'm sure
-                        //       the demo is handling it in an artificial manner.
 
-                        ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol.");
+                    totalBytesWritten += bytesWritten;
+
+                    buf.i += bytesWritten;
+
+                    if (packetSize > buf.b.end() - buf.i)
+                    {
+                        packetSize = buf.b.end() - buf.i;
                     }
-                    continue;
                 }
-
-                case SSL_ERROR_WANT_WRITE:
+                else
                 {
-                    // Repeat with the same arguments! (as in the OpenSSL documentation)
-                    // Whatever happened, the last write didn't actually write anything
-                    // for us.  This is effectively a retry.
+                    // TODO: The client application performs a cleanup at this point,
+                    //       not even shutting down SSL - it just frees the SSL
+                    //       structure. I'm ignoring this, at the moment, as I'm sure
+                    //       the demo is handling it in an artificial manner.
 
-                    ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol.");
-
-                    continue;
+                    ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol.");
                 }
+                continue;
+            }
 
-                case SSL_ERROR_WANT_READ:
-                {
-                    // If we get this error here, it HAS to be because
-                    // the protocol wants to do something handshake related.
-                    // In the case that we might actually get some application data,
-                    // we will use the base SSL read method, using the _inBuffer.
+            case SSL_ERROR_WANT_WRITE:
+            {
+                // Repeat with the same arguments! (as in the OpenSSL documentation)
+                // Whatever happened, the last write didn't actually write anything
+                // for us.  This is effectively a retry.
 
-                    ICE_PROTOCOL("Error SSL_ERROR_WANT_READ.");
+                ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol.");
 
-                    readSSL(_inBuffer, timeout);
+                continue;
+            }
 
-                    continue;
-                }
+            case SSL_ERROR_WANT_READ:
+            {
+                // If we get this error here, it HAS to be because
+                // the protocol wants to do something handshake related.
+                // In the case that we might actually get some application data,
+                // we will use the base SSL read method, using the _inBuffer.
 
-                case SSL_ERROR_WANT_X509_LOOKUP:
-                {
-                    // Perform another read.  The read should take care of this.
+                ICE_PROTOCOL("Error SSL_ERROR_WANT_READ.");
 
-                    ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol.");
+                readSSL(_inBuffer, timeout);
 
-                    continue;
-                }
+                continue;
+            }
 
-                case SSL_ERROR_SYSCALL:
-                {
-                    // NOTE:  The demo client only throws an exception if there were actually bytes
-                    //        written.  This is considered to be an error status requiring shutdown.
-                    //        If nothing was written, the demo client stops writing - we continue.
-                    //        This is potentially something wierd to watch out for.
-                    if (bytesWritten == -1)
-                    {
-                        // IO Error in underlying BIO
-
-	                if (interrupted())
-	                {
-		            break;
-	                }
-
-                        if (wouldBlock())
-	                {
-                            break;
-                        }
-
-                        if (connectionLost())
-	                {
-		            ConnectionLostException ex(__FILE__, __LINE__);
-		            ex.error = getSocketErrno();
-		            throw ex;
-	                }
-	                else
-	                {
-		            SocketException ex(__FILE__, __LINE__);
-		            ex.error = getSocketErrno();
-		            throw ex;
-	                }
-                    }
-                    else if (bytesWritten > 0)
-                    {
-                        ProtocolException protocolEx(__FILE__, __LINE__);
+            case SSL_ERROR_WANT_X509_LOOKUP:
+            {
+                // Perform another read.  The read should take care of this.
 
-                        // Protocol Error: Unexpected EOF
-                        protocolEx._message = "Encountered an EOF that violates the SSL Protocol.\n";
+                ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol.");
 
-                        ICE_SSLERRORS(protocolEx._message);
-                        ICE_EXCEPTION(protocolEx._message);
+                continue;
+            }
 
-                        throw protocolEx;
-                    }
-                    else // bytesWritten == 0
-                    {
-                        // Didn't write anything, continue, should be fine.
+            case SSL_ERROR_SYSCALL:
+            {
+                // NOTE:  The demo client only throws an exception if there were actually bytes
+                //        written.  This is considered to be an error status requiring shutdown.
+                //        If nothing was written, the demo client stops writing - we continue.
+                //        This is potentially something wierd to watch out for.
+                if (bytesWritten == -1)
+                {
+                    // IO Error in underlying BIO
 
-                        ICE_PROTOCOL("Error SSL_ERROR_SYSCALL: Repeating as per protocol.");
+                    if (interrupted())
+                    {
+                        break;
+                    }
 
+                    if (wouldBlock())
+	            {
                         break;
                     }
-                }
 
-                case SSL_ERROR_SSL:
+                    if (connectionLost())
+	            {
+                        ICE_DEV_DEBUG("ClientConnection::write(): Throwing ConnectionLostException... SslConnectionOpenSSLClient.cpp, 390");
+		        ConnectionLostException ex(__FILE__, __LINE__);
+		        ex.error = getSocketErrno();
+		        throw ex;
+	            }
+	            else
+	            {
+                        ICE_DEV_DEBUG("ClientConnection::write(): Throwing SocketException... SslConnectionOpenSSLClient.cpp, 397");
+		        SocketException ex(__FILE__, __LINE__);
+		        ex.error = getSocketErrno();
+		        throw ex;
+	            }
+                }
+                else if (bytesWritten > 0)
                 {
                     ProtocolException protocolEx(__FILE__, __LINE__);
 
-                    protocolEx._message = "Encountered a violation of the SSL Protocol.\n";
+                    // Protocol Error: Unexpected EOF
+                    protocolEx._message = "Encountered an EOF that violates the SSL Protocol.\n";
 
                     ICE_SSLERRORS(protocolEx._message);
                     ICE_EXCEPTION(protocolEx._message);
 
                     throw protocolEx;
                 }
-
-                case SSL_ERROR_ZERO_RETURN:
+                else // bytesWritten == 0
                 {
-                    ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN");
+                    // Didn't write anything, continue, should be fine.
+
+                    ICE_PROTOCOL("Error SSL_ERROR_SYSCALL: Repeating as per protocol.");
 
-		    ConnectionLostException ex(__FILE__, __LINE__);
-		    ex.error = getSocketErrno();
-		    throw ex;
+                    break;
                 }
             }
+
+            case SSL_ERROR_SSL:
+            {
+                ProtocolException protocolEx(__FILE__, __LINE__);
+
+                protocolEx._message = "Encountered a violation of the SSL Protocol.\n";
+
+                ICE_SSLERRORS(protocolEx._message);
+                ICE_EXCEPTION(protocolEx._message);
+
+                throw protocolEx;
+            }
+
+            case SSL_ERROR_ZERO_RETURN:
+            {
+                ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN");
+
+		ConnectionLostException ex(__FILE__, __LINE__);
+		ex.error = getSocketErrno();
+		throw ex;
+            }
         }
     }