Port two SSLClientSocketNSS fixes to SSLClientSocketOpenSSL.

net/socket/ssl_client_socket_openssl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <openssl/ssl.h>
@@ skipping 398 matching lines @@
   context->session_cache()->Flush();
 }
 
 SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
     ClientSocketHandle* transport_socket,
     const HostPortPair& host_and_port,
     const SSLConfig& ssl_config,
     const SSLClientSocketContext& context)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
+      transport_recv_eof_(false),
       completed_handshake_(false),
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_(transport_socket),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       ssl_session_cache_shard_(context.ssl_session_cache_shard),
       trying_cached_session_(false),
@@ skipping 282 matching lines @@
   }
 
   // Shut down anything that may call us back.
   verifier_.reset();
   transport_->socket()->Disconnect();
 
   // Null all callbacks, delete all buffers.
   transport_send_busy_ = false;
   send_buffer_ = NULL;
   transport_recv_busy_ = false;
+  transport_recv_eof_ = false;
   recv_buffer_ = NULL;
 
   user_connect_callback_.Reset();
   user_read_callback_.Reset();
   user_write_callback_.Reset();
   user_read_buf_         = NULL;
   user_read_buf_len_     = 0;
   user_write_buf_        = NULL;
   user_write_buf_len_    = 0;
 
@@ skipping 217 matching lines @@
       intermediates.push_back(sk_X509_value(chain, i));
   }
   server_cert_ = X509Certificate::CreateFromHandle(cert.get(), intermediates);
   DCHECK(server_cert_);
 
   return server_cert_;
 }
 
 bool SSLClientSocketOpenSSL::DoTransportIO() {
   bool network_moved = false;
-  int nsent = BufferSend();
-  int nreceived = BufferRecv();
-  network_moved = (nsent > 0 || nreceived >= 0);
+  int rv;
+  // Read and write as much data as we can. The loop is neccessary
+  // because Write() may return synchronously.
+  do {
+    rv = BufferSend();
+    if (rv > 0)
+      network_moved = true;
+  } while (rv > 0);

[Johnny(Jianning) Ding, 2012/05/18 15:45:58]

I don't think it's necessary to use the loop to keep writing the buffer because
SSLClientSocketOpenSSL::BufferSend already has same logic, see line 989-999,

[joth, 2012/05/18 16:02:08]

wtc will have a better understanding, but I believe the issue is BufferSend will
only call "new DrainableIOBuffer" once. If that buffer gets fully sent (maybe
with multiple iterations of that loop) then we need to go ahead and loop right
back round and try and get another buffer and send?

[wtc, 2012/05/18 18:38:00]

jnd: you are right!  I did a naive port of the SSLClientSocketNSS
change and didn't check the code in SSLClientSocketOpenSSL::BufferSend
carefully.

SSLClientSocketNSS does not have a loop in its BufferSend
function, which is why it needs to call BufferSend in a
loop in DoTransportIO.  I agree with you that SSLClientSocketOpenSSL
does not need this change.

+  if (!transport_recv_eof_ && BufferRecv() >= 0)
+    network_moved = true;
   return network_moved;
 }
 
 int SSLClientSocketOpenSSL::BufferSend(void) {
   if (transport_send_busy_)
     return ERR_IO_PENDING;
 
   if (!send_buffer_) {
     // Get a fresh send buffer out of the send BIO.
     size_t max_read = BIO_ctrl_pending(transport_bio_);
@@ skipping 72 matching lines @@
   OnRecvComplete(result);
 }
 
 void SSLClientSocketOpenSSL::TransportReadComplete(int result) {
   DCHECK(ERR_IO_PENDING != result);
   if (result <= 0) {
     DVLOG(1) << "TransportReadComplete result " << result;
     // Received 0 (end of file) or an error. Either way, bubble it up to the
     // SSL layer via the BIO. TODO(joth): consider stashing the error code, to
     // relay up to the SSL socket client (i.e. via DoReadCallback).
+    if (result == 0)
+      transport_recv_eof_ = true;
     BIO_set_mem_eof_return(transport_bio_, 0);
     (void)BIO_shutdown_wr(transport_bio_);
   } else {
     DCHECK(recv_buffer_);
     int ret = BIO_write(transport_bio_, recv_buffer_->data(), result);
     // A write into a memory BIO should always succeed.
     CHECK_EQ(result, ret);
   }
   recv_buffer_ = NULL;
   transport_recv_busy_ = false;
@@ skipping 28 matching lines @@
   int rv_write = ERR_IO_PENDING;
   bool network_moved;
   do {
       if (user_read_buf_)
           rv_read = DoPayloadRead();
       if (user_write_buf_)
           rv_write = DoPayloadWrite();
       network_moved = DoTransportIO();
   } while (rv_read == ERR_IO_PENDING &&
            rv_write == ERR_IO_PENDING &&
+           (user_read_buf_ || user_write_buf_) &&
            network_moved);
 
   if (user_read_buf_ && rv_read != ERR_IO_PENDING)
       DoReadCallback(rv_read);
   if (user_write_buf_ && rv_write != ERR_IO_PENDING)
       DoWriteCallback(rv_write);
 }
 
 void SSLClientSocketOpenSSL::OnRecvComplete(int result) {
   if (next_handshake_state_ == STATE_HANDSHAKE) {
@@ skipping 182 matching lines @@
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_SENT, rv,
                                   user_write_buf_->data());
     return rv;
   }
 
   int err = SSL_get_error(ssl_, rv);
   return MapOpenSSLError(err, err_tracer);
 }
 
 }  // namespace net