Reuse connection when calling ReadToCount() (#518)

* change EOF for IsEOF function to avoid setting EOF after checking read everytime

* fix setting size to read

* move IsEOF to be private

* Update comment about not moving a connection back to the pool

sdk/core/azure-core/inc/http/curl/curl.hpp

@@ -238,13 +238,6 @@ namespace Azure { namespace Core { namespace Http {
      */
     int64_t m_uploadedBytes;
 
-    /**
-     * @brief Control field that gets true as soon as there is no more data to read from network. A
-     * network socket will return 0 once we got the entire reponse.
-     *
-     */
-    bool m_rawResponseEOF;
-
     /**
      * @brief Control field to handle the case when part of HTTP response body was copied to the
      * inner buffer. When a libcurl stream tries to read part of the body, this field will help to
@@ -381,6 +374,12 @@ namespace Azure { namespace Core { namespace Http {
      */
     int64_t ReadSocketToBuffer(uint8_t* buffer, int64_t bufferSize);
 
+    bool IsEOF()
+    {
+      return this->m_isChunkedResponseType ? this->m_chunkSize == 0
+                                           : this->m_contentLength == this->m_sessionTotalRead;
+    }
+
   public:
 #ifdef TESTING_BUILD
     // Makes possible to know the number of current connections in the connection pool
@@ -401,17 +400,19 @@ namespace Azure { namespace Core { namespace Http {
       this->m_connection = GetCurlConnection(this->m_request);
       this->m_bodyStartInBuffer = -1;
       this->m_innerBufferSize = Details::c_DefaultLibcurlReaderSize;
-      this->m_rawResponseEOF = false;
       this->m_isChunkedResponseType = false;
       this->m_uploadedBytes = 0;
+      this->m_sessionTotalRead = 0;
     }
 
     ~CurlSession() override
     {
       // mark connection as reusable only if entire response was read
       // If not, connection can't be reused because next Read will start from what it is currently
-      // in the wire. We leave the connection blocked until Server closes the connection
-      if (this->m_rawResponseEOF)
+      // in the wire.
+      // By not moving the connection back to the pool, it gets destroyed calling the connection
+      // destructor to clean libcurl handle and close the connection.
+      if (IsEOF())
       {
         MoveConnectionBackToPool(std::move(this->m_connection));
       }

sdk/core/azure-core/src/http/curl/curl.cpp

@@ -302,7 +302,6 @@ void CurlSession::ParseChunkSize()
 
         if (this->m_chunkSize == 0)
         { // Response with no content. end of chunk
-          this->m_rawResponseEOF = true;
           keepPolling = false;
           break;
         }
@@ -367,7 +366,6 @@ void CurlSession::ReadStatusLineAndHeadersFromRawResponse()
   {
     this->m_contentLength = 0;
     this->m_bodyStartInBuffer = -1;
-    this->m_rawResponseEOF = true;
     return;
   }
 
@@ -421,13 +419,13 @@ int64_t CurlSession::Read(Azure::Core::Context const& context, uint8_t* buffer,
 {
   context.ThrowIfCanceled();
 
-  if (count <= 0 || this->m_rawResponseEOF)
+  if (count <= 0 || this->IsEOF())
   {
     return 0;
   }
 
   // check if all chunked is all read already
-  if (this->m_isChunkedResponseType && this->m_chunkSize == 0)
+  if (this->m_isChunkedResponseType && this->m_chunkSize == this->m_sessionTotalRead)
   {
     // Need to read CRLF after all chunk was read
     for (int8_t i = 0; i < 2; i++)
@@ -443,18 +441,21 @@ int64_t CurlSession::Read(Azure::Core::Context const& context, uint8_t* buffer,
         this->m_bodyStartInBuffer = 1; // jump first char (could be \r or \n)
       }
     }
+    // Reset session read counter for next chunk
+    this->m_sessionTotalRead = 0;
     // get the size of next chunk
     ParseChunkSize();
 
-    if (this->m_rawResponseEOF)
+    if (this->IsEOF())
     { // after parsing next chunk, check if it is zero
       return 0;
     }
   }
 
   auto totalRead = int64_t();
-  auto readRequestLength
-      = this->m_isChunkedResponseType ? std::min(this->m_chunkSize, count) : count;
+  auto readRequestLength = this->m_isChunkedResponseType
+      ? std::min(this->m_chunkSize - this->m_sessionTotalRead, count)
+      : count;
 
   // For responses with content-length, avoid trying to read beyond Content-length or
   // libcurl could return a second response as BadRequest.
@@ -476,10 +477,6 @@ int64_t CurlSession::Read(Azure::Core::Context const& context, uint8_t* buffer,
     totalRead = innerBufferMemoryStream.Read(context, buffer, readRequestLength);
     this->m_bodyStartInBuffer += totalRead;
     this->m_sessionTotalRead += totalRead;
-    if (this->m_isChunkedResponseType)
-    {
-      this->m_chunkSize -= totalRead;
-    }
 
     if (this->m_bodyStartInBuffer == this->m_innerBufferSize)
     {
@@ -490,10 +487,8 @@ int64_t CurlSession::Read(Azure::Core::Context const& context, uint8_t* buffer,
 
   // Head request have contentLength = 0, so we won't read more, just return 0
   // Also if we have already read all contentLength
-  if (this->m_sessionTotalRead == this->m_contentLength || this->m_rawResponseEOF)
+  if (this->m_sessionTotalRead == this->m_contentLength || this->IsEOF())
   {
-    // make sure EOF for response is set to true
-    this->m_rawResponseEOF = true;
     return 0;
   }
 
@@ -501,10 +496,6 @@ int64_t CurlSession::Read(Azure::Core::Context const& context, uint8_t* buffer,
   // For chunk request, read a chunk based on chunk size
   totalRead = ReadSocketToBuffer(buffer, static_cast<size_t>(readRequestLength));
   this->m_sessionTotalRead += totalRead;
-  if (this->m_isChunkedResponseType)
-  {
-    this->m_chunkSize -= totalRead;
-  }
 
   return totalRead;
 }