Add Socket::ReadIfReady()

net/socket/ssl_client_socket_impl.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.
 
 #include "net/socket/ssl_client_socket_impl.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <algorithm>
@@ skipping 842 matching lines @@
 
 // static
 void SSLClientSocketImpl::DumpSSLClientSessionMemoryStats(
     base::trace_event::ProcessMemoryDump* pmd) {
   SSLContext::GetInstance()->session_cache()->DumpMemoryStats(pmd);
 }
 
 int SSLClientSocketImpl::Read(IOBuffer* buf,
                               int buf_len,
                               const CompletionCallback& callback) {
-  user_read_buf_ = buf;
-  user_read_buf_len_ = buf_len;
+  int rv = ReadIfReady(buf, buf_len, callback);
+  if (rv == ERR_IO_PENDING) {
+    user_read_buf_ = buf;
+    user_read_buf_len_ = buf_len;
+  }
+  return rv;
+}
 
-  int rv = DoPayloadRead();
+int SSLClientSocketImpl::ReadIfReady(IOBuffer* buf,
+                                     int buf_len,
+                                     const CompletionCallback& callback) {
+  int rv = DoPayloadRead(buf, buf_len);
 
   if (rv == ERR_IO_PENDING) {
     user_read_callback_ = callback;
   } else {
     if (rv > 0)
       was_ever_used_ = true;
-    user_read_buf_ = NULL;
-    user_read_buf_len_ = 0;
   }
-
   return rv;
 }
 
 int SSLClientSocketImpl::Write(IOBuffer* buf,
                                int buf_len,
                                const CompletionCallback& callback) {
   user_write_buf_ = buf;
   user_write_buf_len_ = buf_len;
 
   int rv = DoPayloadWrite();
@@ skipping 11 matching lines @@
 }
 
 int SSLClientSocketImpl::SetReceiveBufferSize(int32_t size) {
   return transport_->socket()->SetReceiveBufferSize(size);
 }
 
 int SSLClientSocketImpl::SetSendBufferSize(int32_t size) {
   return transport_->socket()->SetSendBufferSize(size);
 }
 
-void SSLClientSocketImpl::OnReadReady() {
+void SSLClientSocketImpl::OnReadReady(int rv) {
   // During a renegotiation, either Read or Write calls may be blocked on a
   // transport read.
-  RetryAllOperations();
+  RetryAllOperations(rv);
 }
 
-void SSLClientSocketImpl::OnWriteReady() {
+void SSLClientSocketImpl::OnWriteReady(int rv) {
   // During a renegotiation, either Read or Write calls may be blocked on a
   // transport read.
-  RetryAllOperations();
+  RetryAllOperations(rv);
 }
 
 int SSLClientSocketImpl::Init() {
   DCHECK(!ssl_);
 
 #if defined(USE_NSS_CERTS)
   if (ssl_config_.cert_io_enabled) {
     // TODO(davidben): Move this out of SSLClientSocket. See
     // https://crbug.com/539520.
     EnsureNSSHttpIOInit();
@@ skipping 115 matching lines @@
     SSL_enable_ocsp_stapling(ssl_.get());
 
   return OK;
 }
 
 void SSLClientSocketImpl::DoReadCallback(int rv) {
   // Since Run may result in Read being called, clear |user_read_callback_|
   // up front.
   if (rv > 0)
     was_ever_used_ = true;
-  user_read_buf_ = NULL;
+  user_read_buf_ = nullptr;
   user_read_buf_len_ = 0;
   base::ResetAndReturn(&user_read_callback_).Run(rv);
 }
 
 void SSLClientSocketImpl::DoWriteCallback(int rv) {
   // Since Run may result in Write being called, clear |user_write_callback_|
   // up front.
   if (rv > 0)
     was_ever_used_ = true;
   user_write_buf_ = NULL;
@@ skipping 334 matching lines @@
       case STATE_NONE:
       default:
         rv = ERR_UNEXPECTED;
         NOTREACHED() << "unexpected state" << state;
         break;
     }
   } while (rv != ERR_IO_PENDING && next_handshake_state_ != STATE_NONE);
   return rv;
 }
 
-int SSLClientSocketImpl::DoPayloadRead() {
+int SSLClientSocketImpl::DoPayloadRead(IOBuffer* buf, int buf_len) {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
-  DCHECK_LT(0, user_read_buf_len_);
-  DCHECK(user_read_buf_.get());
+  DCHECK_LT(0, buf_len);
+  DCHECK(buf);
 
   int rv;
   if (pending_read_error_ != kNoPendingResult) {
     rv = pending_read_error_;
     pending_read_error_ = kNoPendingResult;
     if (rv == 0) {
       net_log_.AddByteTransferEvent(NetLogEventType::SSL_SOCKET_BYTES_RECEIVED,
-                                    rv, user_read_buf_->data());
+                                    rv, buf->data());
     } else {
       net_log_.AddEvent(
           NetLogEventType::SSL_READ_ERROR,
           CreateNetLogOpenSSLErrorCallback(rv, pending_read_ssl_error_,
                                            pending_read_error_info_));
     }
     pending_read_ssl_error_ = SSL_ERROR_NONE;
     pending_read_error_info_ = OpenSSLErrorInfo();
     return rv;
   }
 
   int total_bytes_read = 0;
   int ssl_ret;
   do {
-    ssl_ret = SSL_read(ssl_.get(), user_read_buf_->data() + total_bytes_read,
-                       user_read_buf_len_ - total_bytes_read);
+    ssl_ret = SSL_read(ssl_.get(), buf->data() + total_bytes_read,
+                       buf_len - total_bytes_read);
     if (ssl_ret > 0)
       total_bytes_read += ssl_ret;
-  } while (total_bytes_read < user_read_buf_len_ && ssl_ret > 0);
+  } while (total_bytes_read < buf_len && ssl_ret > 0);
 
   // Although only the final SSL_read call may have failed, the failure needs to
   // processed immediately, while the information still available in OpenSSL's
   // error queue.
   if (ssl_ret <= 0) {
     // A zero return from SSL_read may mean any of:
     // - The underlying BIO_read returned 0.
     // - The peer sent a close_notify.
     // - Any arbitrary error. https://crbug.com/466303
     //
@@ skipping 36 matching lines @@
       pending_read_error_ = kNoPendingResult;
   } else {
     // No bytes were returned. Return the pending read error immediately.
     DCHECK_NE(kNoPendingResult, pending_read_error_);
     rv = pending_read_error_;
     pending_read_error_ = kNoPendingResult;
   }
 
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLogEventType::SSL_SOCKET_BYTES_RECEIVED,
-                                  rv, user_read_buf_->data());
+                                  rv, buf->data());
   } else if (rv != ERR_IO_PENDING) {
     net_log_.AddEvent(
         NetLogEventType::SSL_READ_ERROR,
         CreateNetLogOpenSSLErrorCallback(rv, pending_read_ssl_error_,
                                          pending_read_error_info_));
     pending_read_ssl_error_ = SSL_ERROR_NONE;
     pending_read_error_info_ = OpenSSLErrorInfo();
   }
   return rv;
 }
@@ skipping 15 matching lines @@
   int net_error = MapLastOpenSSLError(ssl_error, err_tracer, &error_info);
 
   if (net_error != ERR_IO_PENDING) {
     net_log_.AddEvent(
         NetLogEventType::SSL_WRITE_ERROR,
         CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
   }
   return net_error;
 }
 
-void SSLClientSocketImpl::RetryAllOperations() {
+void SSLClientSocketImpl::RetryAllOperations(int rv) {
+  DCHECK_NE(ERR_IO_PENDING, rv);
+  DCHECK_GE(OK, rv);
+
   // SSL_do_handshake, SSL_read, and SSL_write may all be retried when blocked,
   // so retry all operations for simplicity. (Otherwise, SSL_get_error for each
   // operation may be remembered to retry only the blocked ones.)
 
   if (next_handshake_state_ == STATE_HANDSHAKE) {
     // In handshake phase. The parameter to OnHandshakeIOComplete is unused.
     OnHandshakeIOComplete(OK);
     return;
   }
 
   int rv_read = ERR_IO_PENDING;
   int rv_write = ERR_IO_PENDING;
-  if (user_read_buf_)
-    rv_read = DoPayloadRead();
+  if (user_read_buf_) {
+    rv_read = DoPayloadRead(user_read_buf_.get(), user_read_buf_len_);
+  } else if (!user_read_callback_.is_null()) {
+    // When ReadIfReady() is used, skip DoPayloadRead().
+    rv_read = rv;

[davidben, 2017/02/10 23:33:48]

Hrm. I'm not sure this is quite right. Suppose we get OnWriteReady(ERR_FAILED).
Read() may or may not have been blocked on the underlying write channel. You
only want to forward the error if it was blocked. All operations are safe to
pump whether they're ready or not, so rather than keep track of what everything
is blocked on, this code just retries everything for simplicity.

Instead, I believe if you remove the return value and just do:

  rv_read = OK;

It will work? Even if we woke up because the transport Read was stuck on
ERR_CONNECTION_CLOSED, the caller will ReadIfReady() again and then we'll
synchronously get back the ERR_CONNECTION_CLOSED back out. (SocketBIOAdapter
stashes the error and replays it, to satisfy the "it is always safe to retry a
blocked op" constraint.)

[xunjieli, 2017/02/13 20:28:18]

Done.

+  }
+
   if (user_write_buf_)
     rv_write = DoPayloadWrite();
 
   // Performing the Read callback may cause |this| to be deleted. If this
   // happens, the Write callback should not be invoked. Guard against this by
   // holding a WeakPtr to |this| and ensuring it's still valid.
   base::WeakPtr<SSLClientSocketImpl> guard(weak_factory_.GetWeakPtr());
   if (rv_read != ERR_IO_PENDING)
     DoReadCallback(rv_read);
 
@@ skipping 325 matching lines @@
   DCHECK(ssl_config_.client_private_key);
 
   net_log_.EndEventWithNetErrorCode(NetLogEventType::SSL_PRIVATE_KEY_OP, error);
 
   signature_result_ = error;
   if (signature_result_ == OK)
     signature_ = signature;
 
   // During a renegotiation, either Read or Write calls may be blocked on an
   // asynchronous private key operation.
-  RetryAllOperations();
+  RetryAllOperations(error);
 }
 
 int SSLClientSocketImpl::TokenBindingAdd(const uint8_t** out,
                                          size_t* out_len,
                                          int* out_alert_value) {
   if (ssl_config_.token_binding_params.empty()) {
     return 0;
   }
   bssl::ScopedCBB output;
   CBB parameters_list;
@@ skipping 137 matching lines @@
     if (ERR_GET_REASON(info->error_code) == SSL_R_TLSV1_ALERT_ACCESS_DENIED &&
         !certificate_requested_) {
       net_error = ERR_SSL_PROTOCOL_ERROR;
     }
   }
 
   return net_error;
 }
 
 }  // namespace net