Unique ptr for body stream

sdk/core/azure-core/CMakeLists.txt

@@ -19,7 +19,6 @@ add_library (
   src/context.cpp
   src/credentials/credentials.cpp
   src/credentials/policy/policies.cpp
-  src/http/http.cpp
   src/http/policy.cpp
   src/http/request.cpp
   src/http/response.cpp

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

@@ -14,7 +14,7 @@
 
 namespace Azure { namespace Core { namespace Http {
 
-  constexpr auto UploadSstreamPageSize = 1024 * 64;
+  constexpr auto UploadStreamPageSize = 1024 * 64;
   constexpr auto LibcurlReaderSize = 100;
 
   /**
@@ -43,6 +43,18 @@ namespace Azure { namespace Core { namespace Http {
       EndOfHeaders,
     };
 
+    /**
+     * @brief Defines the strategy to read the body from an HTTP Response
+     *
+     */
+    enum class ResponseBodyLengthType
+    {
+      ContentLength,
+      Chunked,
+      ReadToCloseConnection,
+      NoBody,
+    };
+
     /**
      * @brief stateful component used to read and parse a buffer to construct a valid HTTP Response.
      *
@@ -212,6 +224,20 @@ namespace Azure { namespace Core { namespace Http {
      */
     size_t m_bodyStartInBuffer;
 
+    /**
+     * @brief Control field to handle the number of bytes containing relevant data within the
+     * internal buffer. This is because internal buffer can be set to be size N but after writing
+     * from wire into it, it can be holding less then N bytes.
+     *
+     */
+    size_t m_innerBufferSize;
+
+    /**
+     * @brief Defines the strategy to read a body from an HTTP Response
+     *
+     */
+    ResponseBodyLengthType m_bodyLengthType;
+
     /**
      * @brief This is a copy of the value of an HTTP response header `content-length`. The value is
      * received as string and parsed to size_t. This field avoid parsing the string header everytime
@@ -329,6 +355,7 @@ namespace Azure { namespace Core { namespace Http {
      * requested.
      */
     uint64_t ReadSocketToBuffer(uint8_t* buffer, size_t bufferSize);
+    uint64_t ReadChunkedBody(uint8_t* buffer, uint64_t bufferSize, uint64_t offset);
 
   public:
     /**
@@ -340,6 +367,7 @@ namespace Azure { namespace Core { namespace Http {
     {
       this->m_pCurl = curl_easy_init();
       this->m_bodyStartInBuffer = 0;
+      this->m_innerBufferSize = LibcurlReaderSize;
     }
 
     /**
@@ -412,6 +440,8 @@ namespace Azure { namespace Core { namespace Http {
      */
     uint64_t m_offset;
 
+    bool m_unknownSize;
+
   public:
     /**
      * @brief Construct a new Curl Body Stream object.
@@ -425,12 +455,9 @@ namespace Azure { namespace Core { namespace Http {
     {
     }
 
-    ~CurlBodyStream()
+    CurlBodyStream(CurlSession* curlSession)
+        : m_length(0), m_curlSession(curlSession), m_offset(0), m_unknownSize(true)
     {
-      if (this->m_curlSession != nullptr)
-      {
-        delete this->m_curlSession; // Session Destructor will cleanup libcurl handle
-      }
     }
 
     /**
@@ -450,13 +477,14 @@ namespace Azure { namespace Core { namespace Http {
      */
     uint64_t Read(uint8_t* buffer, uint64_t count) override
     {
-      if (this->m_length == this->m_offset)
+      if (this->m_length == this->m_offset && !this->m_unknownSize)
       {
         return 0;
       }
       // Read bytes from curl into buffer. As max as the length of Stream is allowed
       auto readCount = this->m_curlSession->ReadWithOffset(buffer, count, this->m_offset);
       this->m_offset += readCount;
+
       return readCount;
     }
 

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

@@ -215,7 +215,7 @@ namespace Azure { namespace Core { namespace Http {
     std::map<std::string, std::string> m_retryHeaders;
     std::map<std::string, std::string> m_retryQueryParameters;
     // Work only with streams
-    BodyStream* m_bodyStream;
+    std::unique_ptr<BodyStream> m_bodyStream;
 
     // flag to know where to insert header
     bool m_retryModeEnabled;
@@ -233,25 +233,14 @@ namespace Azure { namespace Core { namespace Http {
     std::string GetQueryString() const;
 
   public:
-    Request(HttpMethod httpMethod, std::string const& url, BodyStream* bodyStream)
-        : m_method(std::move(httpMethod)), m_url(url), m_bodyStream(bodyStream),
+    Request(HttpMethod httpMethod, std::string const& url, std::unique_ptr<BodyStream> bodyStream)
+        : m_method(std::move(httpMethod)), m_url(url), m_bodyStream(std::move(bodyStream)),
           m_retryModeEnabled(false)
     {
     }
 
     // Typically used for GET with no request body.
-    Request(HttpMethod httpMethod, std::string const& url)
-        : Request(httpMethod, url, BodyStream::null)
-    {
-    }
-
-    ~Request()
-    {
-      if (this->m_bodyStream != BodyStream::null)
-      {
-        delete this->m_bodyStream;
-      }
-    }
+    Request(HttpMethod httpMethod, std::string const& url) : Request(httpMethod, url, nullptr) {}
 
     // Methods used to build HTTP request
     void AppendPath(std::string const& path);
@@ -296,16 +285,16 @@ namespace Azure { namespace Core { namespace Http {
     std::string m_reasonPhrase;
     std::map<std::string, std::string> m_headers;
 
-    BodyStream* m_bodyStream;
+    std::unique_ptr<BodyStream> m_bodyStream;
 
     Response(
         int32_t majorVersion,
         int32_t minorVersion,
         HttpStatusCode statusCode,
         std::string const& reasonPhrase,
-        BodyStream* const BodyStream)
+        std::unique_ptr<BodyStream> BodyStream)
         : m_majorVersion(majorVersion), m_minorVersion(minorVersion), m_statusCode(statusCode),
-          m_reasonPhrase(reasonPhrase), m_bodyStream(BodyStream)
+          m_reasonPhrase(reasonPhrase), m_bodyStream(std::move(BodyStream))
     {
     }
 
@@ -315,23 +304,15 @@ namespace Azure { namespace Core { namespace Http {
         int32_t minorVersion,
         HttpStatusCode statusCode,
         std::string const& reasonPhrase)
-        : Response(majorVersion, minorVersion, statusCode, reasonPhrase, BodyStream::null)
-    {
-    }
-
-    ~Response()
+        : Response(majorVersion, minorVersion, statusCode, reasonPhrase, nullptr)
     {
-      if (this->m_bodyStream != BodyStream::null)
-      {
-        delete this->m_bodyStream;
-      }
     }
 
     // Methods used to build HTTP response
     void AddHeader(std::string const& name, std::string const& value);
     // rfc form header-name: OWS header-value OWS
     void AddHeader(std::string const& header);
-    void SetBodyStream(BodyStream* stream);
+    void SetBodyStream(std::unique_ptr<BodyStream> stream);
 
     // adding getters for version and stream body. Clang will complain on Mac if we have unused
     // fields in a class
@@ -340,7 +321,15 @@ namespace Azure { namespace Core { namespace Http {
     HttpStatusCode GetStatusCode() const;
     std::string const& GetReasonPhrase();
     std::map<std::string, std::string> const& GetHeaders();
-    BodyStream* GetBodyStream() { return this->m_bodyStream; }
+    std::unique_ptr<BodyStream> GetBodyStream()
+    {
+      if (this->m_bodyStream == nullptr)
+      {
+        // Moved before or not yet created
+        return nullptr;
+      }
+      return std::move(this->m_bodyStream);
+    }
 
     // Allocates a buffer in heap and reads and copy stream content into it.
     // util for any API that needs to get the content from stream as a buffer

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

@@ -14,7 +14,7 @@ namespace Azure { namespace Core { namespace Http {
   // BodyStream is used to read data to/from a service
   class BodyStream {
   public:
-    static BodyStream* null;
+    virtual ~BodyStream() = 0;
 
     // Returns the length of the data; used with the HTTP Content-Length header
     virtual uint64_t Length() const = 0;
@@ -33,9 +33,6 @@ namespace Azure { namespace Core { namespace Http {
 
     // Closes the stream; typically called after all data read or if an error occurs.
     virtual void Close() = 0;
-
-    // Desstructor. Enables derived classes to call its destructor
-    virtual ~BodyStream() = 0;
   };
 
   class MemoryBodyStream : public BodyStream {

sdk/core/azure-core/src/credentials/credentials.cpp

@@ -73,7 +73,7 @@ AccessToken ClientSecretCredential::GetToken(
 
     auto bodyStream = std::make_unique<Http::MemoryBodyStream>(bodyVec);
 
-    Http::Request request(Http::HttpMethod::Post, url.str(), bodyStream.get());
+    Http::Request request(Http::HttpMethod::Post, url.str(), std::move(bodyStream));
     bodyStream.release();
 
     request.AddHeader("Content-Type", "application/x-www-form-urlencoded");
@@ -103,8 +103,8 @@ AccessToken ClientSecretCredential::GetToken(
     {
       std::ostringstream errorMsg;
       errorMsg << errorMsgPrefix << "error response: "
-               << static_cast<std::underlying_type<Http::HttpStatusCode>::type>(statusCode)
-               << " " << response->GetReasonPhrase();
+               << static_cast<std::underlying_type<Http::HttpStatusCode>::type>(statusCode) << " "
+               << response->GetReasonPhrase();
 
       throw AuthenticationException(errorMsg.str());
     }