From 66aa9bf52d3ea56d7c5dea8c4c6a2e27f0c8db21 Mon Sep 17 00:00:00 2001
From: Nick Zitzmann <nickzman@gmail.com>
Date: Tue, 12 Feb 2013 13:02:36 -0700
Subject: darwinssl: Fix send glitchiness with data > 32 or so KB

An ambiguity in the SSLWrite() documentation lead to a bad inference in the
code where we assumed SSLWrite() returned the amount of bytes written to
the socket, when that is not actually true; it returns the amount of data
that is buffered for writing to the socket if it returns errSSLWouldBlock.
Now darwinssl_send() returns CURLE_AGAIN if data is buffered but not written.

Reference URL: http://curl.haxx.se/mail/lib-2013-02/0145.html
---
 lib/curl_darwinssl.c | 65 +++++++++++++++++++++++++++++++++++++++++-----------
 lib/urldata.h        |  1 +
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/lib/curl_darwinssl.c b/lib/curl_darwinssl.c
index d660deb2d..523370e6d 100644
--- a/lib/curl_darwinssl.c
+++ b/lib/curl_darwinssl.c
@@ -739,6 +739,7 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn,
     return CURLE_OUT_OF_MEMORY;
   }
 #endif /* defined(__MAC_10_8) || defined(__IPHONE_5_0) */
+  connssl->ssl_write_buffered_length = 0UL; /* reset buffered write length */
 
   /* check to see if we've been told to use an explicit SSL/TLS version */
 #if defined(__MAC_10_8) || defined(__IPHONE_5_0)
@@ -1424,22 +1425,58 @@ static ssize_t darwinssl_send(struct connectdata *conn,
   /*struct SessionHandle *data = conn->data;*/
   struct ssl_connect_data *connssl = &conn->ssl[sockindex];
   size_t processed = 0UL;
-  OSStatus err = SSLWrite(connssl->ssl_ctx, mem, len, &processed);
+  OSStatus err;
 
-  if(err != noErr) {
+  /* The SSLWrite() function works a little differently than expected. The
+     fourth argument (processed) is currently documented in Apple's
+     documentation as: "On return, the length, in bytes, of the data actually
+     written."
+
+     Now, one could interpret that as "written to the socket," but actually,
+     it returns the amount of data that was written to a buffer internal to
+     the SSLContextRef instead. So it's possible for SSLWrite() to return
+     errSSLWouldBlock and a number of bytes "written" because those bytes were
+     encrypted and written to a buffer, not to the socket.
+
+     So if this happens, then we need to keep calling SSLWrite() over and
+     over again with no new data until it quits returning errSSLWouldBlock. */
+
+  /* Do we have buffered data to write from the last time we were called? */
+  if(connssl->ssl_write_buffered_length) {
+    /* Write the buffered data: */
+    err = SSLWrite(connssl->ssl_ctx, NULL, 0UL, &processed);
     switch (err) {
-      case errSSLWouldBlock:  /* return how much we sent (if anything) */
-        if(processed)
-          return (ssize_t)processed;
-        *curlcode = CURLE_AGAIN;
-        return -1;
+      case noErr:
+        /* processed is always going to be 0 because we didn't write to
+           the buffer, so return how much was written to the socket */
+        processed = connssl->ssl_write_buffered_length;
+        connssl->ssl_write_buffered_length = 0UL;
         break;
-
+      case errSSLWouldBlock: /* argh, try again */
+        *curlcode = CURLE_AGAIN;
+        return -1L;
       default:
-        failf(conn->data, "SSLWrite() return error %d", err);
+        failf(conn->data, "SSLWrite() returned error %d", err);
         *curlcode = CURLE_SEND_ERROR;
-        return -1;
-        break;
+        return -1L;
+    }
+  }
+  else {
+    /* We've got new data to write: */
+    err = SSLWrite(connssl->ssl_ctx, mem, len, &processed);
+    if(err != noErr) {
+      switch (err) {
+        case errSSLWouldBlock:
+          /* Data was buffered but not sent, we have to tell the caller
+             to try sending again, and remember how much was buffered */
+          connssl->ssl_write_buffered_length = len;
+          *curlcode = CURLE_AGAIN;
+          return -1L;
+        default:
+          failf(conn->data, "SSLWrite() returned error %d", err);
+          *curlcode = CURLE_SEND_ERROR;
+          return -1L;
+      }
     }
   }
   return (ssize_t)processed;
@@ -1462,18 +1499,18 @@ static ssize_t darwinssl_recv(struct connectdata *conn,
         if(processed)
           return (ssize_t)processed;
         *curlcode = CURLE_AGAIN;
-        return -1;
+        return -1L;
         break;
 
       case errSSLClosedGraceful: /* they're done; fail gracefully */
         *curlcode = CURLE_OK;
-        return -1;
+        return -1L;
         break;
 
       default:
         failf(conn->data, "SSLRead() return error %d", err);
         *curlcode = CURLE_RECV_ERROR;
-        return -1;
+        return -1L;
         break;
     }
   }
diff --git a/lib/urldata.h b/lib/urldata.h
index 7a275da5e..4849dae16 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -326,6 +326,7 @@ struct ssl_connect_data {
   curl_socket_t ssl_sockfd;
   ssl_connect_state connecting_state;
   bool ssl_direction; /* true if writing, false if reading */
+  size_t ssl_write_buffered_length;
 #endif /* USE_DARWINSSL */
 };
 
-- 
cgit v1.2.3