Fixes handling of short reads/writes for Mac SSL

runtime/bin/secure_socket_macos.cc

Index: runtime/bin/secure_socket_macos.cc
diff --git a/runtime/bin/secure_socket_macos.cc b/runtime/bin/secure_socket_macos.cc
index a18b32fe26dce0022fb5ee8f60e1efa754693ffe..93399c235316b3b19e0402e6cc015d2623d2f2b3 100644
--- a/runtime/bin/secure_socket_macos.cc
+++ b/runtime/bin/secure_socket_macos.cc
@@ -56,6 +56,16 @@ SecIdentityRef SecIdentityCreate(CFAllocatorRef allocator,
 namespace dart {
 namespace bin {
 
+static const int kSSLFilterNativeFieldIndex = 0;
+static const int kSecurityContextNativeFieldIndex = 0;
+static const int kX509NativeFieldIndex = 0;
+
+static const bool SSL_LOG_STATUS = false;
+static const bool SSL_LOG_DATA = false;
+static const bool SSL_LOG_CERTS = false;
+static const int SSL_ERROR_MESSAGE_BUFFER_SIZE = 1000;
+static const intptr_t PEM_BUFSIZE = 1024;
+
 // SSLCertContext wraps the certificates needed for a SecureTransport
 // connection. Fields are protected by the mutex_ field, and may only be set
 // once. This is to allow access by both the Dart thread and the IOService
@@ -183,15 +193,6 @@ class SSLCertContext {
   DISALLOW_COPY_AND_ASSIGN(SSLCertContext);
 };
 
-static const int kSSLFilterNativeFieldIndex = 0;
-static const int kSecurityContextNativeFieldIndex = 0;
-static const int kX509NativeFieldIndex = 0;
-
-static const bool SSL_LOG_STATUS = false;
-static const bool SSL_LOG_DATA = false;
-static const bool SSL_LOG_CERTS = false;
-static const int SSL_ERROR_MESSAGE_BUFFER_SIZE = 1000;
-static const intptr_t PEM_BUFSIZE = 1024;
 
 static char* CFStringRefToCString(CFStringRef cfstring) {
   CFIndex len = CFStringGetLength(cfstring);
@@ -1842,30 +1843,6 @@ void SSLFilter::Destroy() {
 }
 
 
-static intptr_t AvailableToRead(intptr_t start, intptr_t end, intptr_t size) {
-  intptr_t data_available = 0;
-  if (end < start) {
-    // Data may be split into two segments.  In this case,
-    // the first is [start, size).
-    intptr_t buffer_end = (start == 0) ? size - 1 : size;
-    intptr_t available = buffer_end - start;
-    start += available;
-    data_available += available;
-    ASSERT(start <= size);
-    if (start == size) {
-      start = 0;
-    }
-  }
-  if (start < end) {
-    intptr_t available = end - start;
-    start += available;
-    data_available += available;
-    ASSERT(start <= end);
-  }
-  return data_available;
-}
-
-
 OSStatus SSLFilter::SSLReadCallback(SSLConnectionRef connection,
                                     void* data, size_t* data_requested) {
   // Copy at most `data_requested` bytes from `buffers_[kReadEncrypted]` into
@@ -1884,11 +1861,6 @@ OSStatus SSLFilter::SSLReadCallback(SSLConnectionRef connection,
   intptr_t requested = static_cast<intptr_t>(*data_requested);
   intptr_t data_read = 0;
 
-  if (AvailableToRead(start, end, size) < requested) {
-    *data_requested = 0;
-    return errSSLWouldBlock;
-  }
-
   if (end < start) {
     // Data may be split into two segments.  In this case,
     // the first is [start, size).
@@ -1922,7 +1894,11 @@ OSStatus SSLFilter::SSLReadCallback(SSLConnectionRef connection,
   }
 
   filter->SetBufferStart(kReadEncrypted, start);
+  bool short_read = data_read < static_cast<intptr_t>(*data_requested);
   *data_requested = data_read;
+  if (short_read) {
+    return errSSLWouldBlock;
+  }
   return noErr;

[Ivan Posva, 2016/03/29 17:14:16]

return short_read ? errSSLWouldBlock : noErr;

[zra, 2016/03/29 17:29:19]

Done.

 }
 
@@ -1941,6 +1917,9 @@ OSStatus SSLFilter::ProcessReadPlaintextBuffer(intptr_t start,
         reinterpret_cast<void*>((buffers_[kReadPlaintext] + start)),
         length,
         &bytes);
+    if (SSL_LOG_STATUS) {
+      Log::Print("SSLRead: status = %ld\n", static_cast<intptr_t>(status));
+    }
     if ((status != noErr) && (status != errSSLWouldBlock)) {
       *bytes_processed = 0;
       return status;
@@ -1955,35 +1934,6 @@ OSStatus SSLFilter::ProcessReadPlaintextBuffer(intptr_t start,
 }
 
 
-intptr_t SpaceToWrite(intptr_t start, intptr_t end, intptr_t size) {
-  intptr_t writable_space = 0;
-
-  // is full, neither if statement is executed and nothing happens.
-  if (start <= end) {
-    // If the free space may be split into two segments,
-    // then the first is [end, size), unless start == 0.
-    // Then, since the last free byte is at position start - 2,
-    // the interval is [end, size - 1).
-    intptr_t buffer_end = (start == 0) ? size - 1 : size;
-    intptr_t available = buffer_end - end;
-    end += available;
-    writable_space += available;
-    ASSERT(end <= size);
-    if (end == size) {
-      end = 0;
-    }
-  }
-  if (start > end + 1) {
-    intptr_t available = (start - 1) - end;
-    end += available;
-    writable_space += available;
-    ASSERT(end < start);
-  }
-
-  return writable_space;
-}
-
-
 OSStatus SSLFilter::SSLWriteCallback(SSLConnectionRef connection,
                                      const void* data, size_t* data_provided) {
   // Copy at most `data_provided` bytes from data into
@@ -2002,11 +1952,6 @@ OSStatus SSLFilter::SSLWriteCallback(SSLConnectionRef connection,
   intptr_t provided = static_cast<intptr_t>(*data_provided);
   intptr_t data_written = 0;
 
-  if (SpaceToWrite(start, end, size) < provided) {
-    *data_provided = 0;
-    return errSSLWouldBlock;
-  }
-
   // is full, neither if statement is executed and nothing happens.
   if (start <= end) {
     // If the free space may be split into two segments,
@@ -2044,6 +1989,9 @@ OSStatus SSLFilter::SSLWriteCallback(SSLConnectionRef connection,
 
   filter->SetBufferEnd(kWriteEncrypted, end);
   *data_provided = data_written;
+  if (data_written == 0) {
+    return errSSLWouldBlock;
+  }
   return noErr;

[Ivan Posva, 2016/03/29 17:14:16]

ditto.

[zra, 2016/03/29 17:29:19]

Done.

 }
 
@@ -2061,11 +2009,18 @@ OSStatus SSLFilter::ProcessWritePlaintextBuffer(intptr_t start,
         reinterpret_cast<void*>(buffers_[kWritePlaintext] + start),
         length,
         &bytes);
+    if (SSL_LOG_STATUS) {
+      Log::Print("SSLWrite: status = %ld\n", static_cast<intptr_t>(status));
+    }
     if ((status != noErr) && (status != errSSLWouldBlock)) {
       *bytes_processed = 0;
       return status;
     }
   }
+  if (SSL_LOG_DATA) {
+    Log::Print("ProcessWritePlaintextBuffer: requested: %ld, written: %ld\n",
+        length, bytes);
+  }
   *bytes_processed = static_cast<intptr_t>(bytes);
   return status;
 }