When reading from an SSL socket, attempt to fully fill the caller's buffer

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index e14527cfb8d56d69219a7ebd1520ba736e46df8c..a259cae38d68c9385c7ee9e248e9dba4c0e1ac98 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -425,6 +425,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
     : transport_send_busy_(false),
       transport_recv_busy_(false),
       transport_recv_eof_(false),
+      pending_read_error_(1),
       completed_handshake_(false),
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
@@ -1340,20 +1341,44 @@ bool SSLClientSocketOpenSSL::SetSendBufferSize(int32 size) {
 
 int SSLClientSocketOpenSSL::DoPayloadRead() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  int rv = SSL_read(ssl_, user_read_buf_->data(), user_read_buf_len_);
-  // We don't need to invalidate the non-client-authenticated SSL session
-  // because the server will renegotiate anyway.
-  if (client_auth_cert_needed_)
-    return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
+
+  int rv;
+  if (pending_read_error_ <= 0) {
+    rv = pending_read_error_;
+    pending_read_error_ = 1;

[wtc, 2013/02/13 22:06:51]

If rv == 0, we need to log a NetLog::TYPE_SSL_SOCKET_BYTES_RECEIVED
event to match the original code.

+    return rv;
+  }
+
+  int total_bytes_read = 0;
+  do {
+    rv = SSL_read(ssl_, user_read_buf_->data() + total_bytes_read,
+                  user_read_buf_len_ - total_bytes_read);
+    if (rv > 0)
+      total_bytes_read += rv;
+  } while (total_bytes_read < user_read_buf_len_ && rv > 0);
+
+  if (total_bytes_read == user_read_buf_len_) {
+    rv = total_bytes_read;
+  } else {
+    int *next_result = &rv;
+    if (total_bytes_read > 0) {
+      pending_read_error_ = rv;

[Ryan Hamilton, 2013/02/13 17:23:18]

It looks like this is the only place you assign a "success" value to
pending_read_error_ that is not "1".  I think I would be inclined to create a
const int kNoPeindingError = 1 and use it for clarity.

[Ryan Sleevi, 2013/02/13 20:44:43]

Actually, this is a "failure" value (it can only be reached if we broke on line
1358 with rv being <= 0. I will add a comment to explain that.

That said, using the constant is both reasonable and preferable, so I will make
that change.

[Ryan Hamilton, 2013/02/13 21:50:34]

As part of this CL, or in a different one?

+      rv = total_bytes_read;
+      next_result = &pending_read_error_;
+    }
+    if (client_auth_cert_needed_) {
+      *next_result = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
+    } else if (*next_result < 0) {
+      int err = SSL_get_error(ssl_, *next_result);
+      *next_result = MapOpenSSLError(err, err_tracer);
+    }
+  }
 
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_RECEIVED, rv,
                                   user_read_buf_->data());
-    return rv;
   }
-
-  int err = SSL_get_error(ssl_, rv);
-  return MapOpenSSLError(err, err_tracer);
+  return rv;
 }
 
 int SSLClientSocketOpenSSL::DoPayloadWrite() {