Fix readable in Async*Socket

Summary: I'm not sure this API is actually used anywhere, but the implementations don't account for cases where the underlying socket is unreadable but setting a read callback would immediately provide data.

Reviewed By: simpkins

Differential Revision: D25324200

fbshipit-source-id: 4176432feea09a4d1387b9025f485e537a5b8da5

folly/io/async/AsyncSSLSocket.cpp

@@ -408,6 +408,13 @@ void AsyncSSLSocket::shutdownWriteNow() {
   closeNow();
 }
 
+bool AsyncSSLSocket::readable() const {
+  if (ssl_ != nullptr && SSL_pending(ssl_.get()) > 0) {
+    return true;
+  }
+  return AsyncSocket::readable();
+}
+
 bool AsyncSSLSocket::good() const {
   return (
       AsyncSocket::good() &&

folly/io/async/AsyncSSLSocket.h

@@ -379,6 +379,7 @@ class AsyncSSLSocket : public AsyncSocket {
   void closeNow() override;
   void shutdownWrite() override;
   void shutdownWriteNow() override;
+  bool readable() const override;
   bool good() const override;
   bool connecting() const override;
   std::string getApplicationProtocol() const noexcept override;

folly/io/async/AsyncSocket.cpp

@@ -1905,6 +1905,10 @@ bool AsyncSocket::readable() const {
   if (fd_ == NetworkSocket()) {
     return false;
   }
+
+  if (preReceivedData_ && !preReceivedData_->empty()) {
+    return true;
+  }
   netops::PollDescriptor fds[1];
   fds[0].fd = fd_;
   fds[0].events = POLLIN;

folly/io/async/test/AsyncSSLSocketTest.cpp

@@ -204,6 +204,54 @@ TEST(AsyncSSLSocketTest, ConnectWriteReadClose) {
   EXPECT_EQ(socket->getSSLSocket()->getTotalConnectTimeout().count(), 10000);
 }
 
+TEST(AsyncSSLSocketTest, ConnectWriteReadCloseReadable) {
+  // Same as above, but test AsyncSSLSocket::readable along the way
+
+  // Start listening on a local port
+  WriteCallbackBase writeCallback;
+  ReadCallback readCallback(&writeCallback);
+  HandshakeCallback handshakeCallback(&readCallback);
+  SSLServerAcceptCallback acceptCallback(&handshakeCallback);
+  TestSSLServer server(&acceptCallback);
+
+  // Set up SSL context.
+  std::shared_ptr<SSLContext> sslContext(new SSLContext());
+  sslContext->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
+  // sslContext->loadTrustedCertificates("./trusted-ca-certificate.pem");
+  // sslContext->authenticate(true, false);
+
+  // connect
+  auto socket =
+      std::make_shared<BlockingSocket>(server.getAddress(), sslContext);
+  socket->open(std::chrono::milliseconds(10000));
+
+  // write()
+  uint8_t buf[128];
+  memset(buf, 'a', sizeof(buf));
+  socket->write(buf, sizeof(buf));
+
+  // read()
+  uint8_t readbuf[128];
+  // The TLS record includes the full 128 bytes.  Even though we only read 1
+  // byte out of the socket, the rest of the full record decrypted and buffered
+  // in the underlying SSL session.
+  uint32_t bytesRead = socket->read(readbuf, 1);
+  EXPECT_EQ(bytesRead, 1);
+  // The socket has no data pending in the kernel
+  EXPECT_FALSE(socket->getSocket()->AsyncSocket::readable());
+  // But the socket is readable
+  EXPECT_TRUE(socket->getSocket()->readable());
+  bytesRead += socket->readAll(readbuf + 1, sizeof(readbuf) - 1);
+  EXPECT_EQ(bytesRead, 128);
+  EXPECT_EQ(memcmp(buf, readbuf, bytesRead), 0);
+
+  // close()
+  socket->close();
+
+  cerr << "ConnectWriteReadClose test completed" << endl;
+  EXPECT_EQ(socket->getSSLSocket()->getTotalConnectTimeout().count(), 10000);
+}
+
 /**
  * Check that zero copy options are a noop under AsyncSSLSocket since they
  * aren't supported.

folly/io/async/test/AsyncSocketTest2.cpp

@@ -8057,6 +8057,7 @@ TEST(AsyncSocket, PreReceivedDataOnly) {
   peekCallback.dataAvailableCallback = [&]() {
     peekCallback.verifyData("hello", 5);
     acceptedSocket->setPreReceivedData(IOBuf::copyBuffer("hello"));
+    EXPECT_TRUE(acceptedSocket->readable());
     acceptedSocket->setReadCB(&readCallback);
   };
   readCallback.dataAvailableCallback = [&]() {