From 9f9e30b51e3912c0b63258badf5501d3cb2550be Mon Sep 17 00:00:00 2001
From: Martin Haimberger <martin.haimberger@thincast.com>
Date: Fri, 6 Oct 2017 09:57:27 +0200
Subject: THRIFT-4331: C++ TSSLSocket fixes for huge message handling Client:
 C++

fixed issue with large messages, where waitForEvent was called
mutliple times waiting for SSL_read() to get bytes and running
in the retry timeout.

fixed issue where poll was not using the right flags.

This fixes #1363
---
 lib/cpp/src/thrift/transport/TSSLSocket.cpp | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
index acf23aa9c..ddefb34c9 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
@@ -295,8 +295,9 @@ bool TSSLSocket::peek() {
           }
         case SSL_ERROR_WANT_READ:
         case SSL_ERROR_WANT_WRITE:
-          waitForEvent(error == SSL_ERROR_WANT_READ);
-              continue;
+          // in the case of SSL_ERROR_SYSCALL we want to wait for an read event again
+          waitForEvent(error != SSL_ERROR_WANT_WRITE);
+          continue;
         default:;// do nothing
       }
       string errors;
@@ -338,6 +339,7 @@ void TSSLSocket::close() {
               }
             case SSL_ERROR_WANT_READ:
             case SSL_ERROR_WANT_WRITE:
+              // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
               waitForEvent(error == SSL_ERROR_WANT_READ);
               rc = 2;
             default:;// do nothing
@@ -387,6 +389,7 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
       break;
     }
     int32_t errno_copy = THRIFT_GET_SOCKET_ERROR;
+    unsigned int waitEventReturn;
     switch (error) {
       case SSL_ERROR_SYSCALL:
         if ((errno_copy != THRIFT_EINTR)
@@ -408,7 +411,8 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
           }
           throw TTransportException(TTransportException::INTERNAL_ERROR, "too much recv retries");
         }
-        else if (waitForEvent(error == SSL_ERROR_WANT_READ) == TSSL_EINTR ) {
+        // in the case of SSL_ERROR_SYSCALL we want to wait for an read event again
+        else if ((waitEventReturn = waitForEvent(error != SSL_ERROR_WANT_WRITE)) == TSSL_EINTR ) {
           // repeat operation
           if (readRetryCount_ < maxRecvRetries_) {
             // THRIFT_EINTR needs to be handled manually and we can tolerate
@@ -417,7 +421,15 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
           }
           throw TTransportException(TTransportException::INTERNAL_ERROR, "too much recv retries");
         }
-        continue;
+        else if (waitEventReturn == TSSL_DATA) {
+            // in case of SSL and huge thrift packets, there may be a number of 
+            // socket operations, before any data becomes available by SSL_read(). 
+            // Therefore the number of retries should not be increased and 
+            // the operation should be repeated.
+            readRetryCount_--;
+            continue;
+        }
+        throw TTransportException(TTransportException::INTERNAL_ERROR, "unkown waitForEvent return value");
       default:;// do nothing
     }
     string errors;
@@ -451,6 +463,7 @@ void TSSLSocket::write(const uint8_t* buf, uint32_t len) {
             return;
           }
           else {
+            // in the case of SSL_ERROR_SYSCALL we want to wait for an write event again
             waitForEvent(error == SSL_ERROR_WANT_READ);
             continue;
           }
@@ -494,6 +507,7 @@ uint32_t TSSLSocket::write_partial(const uint8_t* buf, uint32_t len) {
             return 0;
           }
           else {
+            // in the case of SSL_ERROR_SYSCALL we want to wait for an write event again
             waitForEvent(error == SSL_ERROR_WANT_READ);
             continue;
           }
@@ -579,6 +593,7 @@ void TSSLSocket::initializeHandshake() {
             }
             else {
               // repeat operation
+              // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
               waitForEvent(error == SSL_ERROR_WANT_READ);
               rc = 2;
             }
@@ -610,6 +625,7 @@ void TSSLSocket::initializeHandshake() {
             }
             else {
               // repeat operation
+              // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
               waitForEvent(error == SSL_ERROR_WANT_READ);
               rc = 2;
             }
@@ -759,7 +775,9 @@ unsigned int TSSLSocket::waitForEvent(bool wantRead) {
   struct THRIFT_POLLFD fds[2];
   memset(fds, 0, sizeof(fds));
   fds[0].fd = fdSocket;
-  fds[0].events = wantRead ? THRIFT_POLLIN : THRIFT_POLLOUT;
+  // use POLLIN also on write operations too, this is needed for operations
+  // which requires read and write on the socket.
+  fds[0].events = wantRead ? THRIFT_POLLIN : THRIFT_POLLIN | THRIFT_POLLOUT;
 
   if (interruptListener_) {
     fds[1].fd = *(interruptListener_.get());
-- 
cgit v1.2.1