Http/fix reading more than content (#311)

* Fix reading more data than content length

* fix for head request

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

@@ -368,6 +368,7 @@ void CurlSession::ReadStatusLineAndHeadersFromRawResponse()
   if (this->m_request.GetMethod() == HttpMethod::Head)
   {
     this->m_contentLength = 0;
+    this->m_bodyStartInBuffer = -1;
     this->m_rawResponseEOF = true;
     return;
   }
@@ -455,9 +456,18 @@ int64_t CurlSession::Read(Azure::Core::Context& context, uint8_t* buffer, int64_
   }
 
   auto totalRead = int64_t();
-  auto ReadRequestLength
+  auto readRequestLength
       = this->m_isChunkedResponseType ? std::min(this->m_chunkSize, count) : count;
 
+  // For responses with content-length, avoid trying to read beyond Content-length or
+  // libcurl could return a second response as BadRequest.
+  // https://github.com/Azure/azure-sdk-for-cpp/issues/306
+  if (this->m_contentLength > 0)
+  {
+    auto remainingBodyContent = this->m_contentLength - this->m_sessionTotalRead;
+    readRequestLength = std::min(readRequestLength, remainingBodyContent);
+  }
+
   // Take data from inner buffer if any
   if (this->m_bodyStartInBuffer >= 0)
   {
@@ -466,7 +476,7 @@ int64_t CurlSession::Read(Azure::Core::Context& context, uint8_t* buffer, int64_
         this->m_readBuffer + this->m_bodyStartInBuffer,
         this->m_innerBufferSize - this->m_bodyStartInBuffer);
 
-    totalRead = innerBufferMemoryStream.Read(context, buffer, ReadRequestLength);
+    totalRead = innerBufferMemoryStream.Read(context, buffer, readRequestLength);
     this->m_bodyStartInBuffer += totalRead;
     this->m_sessionTotalRead += totalRead;
     if (this->m_isChunkedResponseType)
@@ -491,7 +501,7 @@ int64_t CurlSession::Read(Azure::Core::Context& context, uint8_t* buffer, int64_
 
   // Read from socket when no more data on internal buffer
   // For chunk request, read a chunk based on chunk size
-  totalRead = ReadSocketToBuffer(buffer, static_cast<size_t>(ReadRequestLength));
+  totalRead = ReadSocketToBuffer(buffer, static_cast<size_t>(readRequestLength));
   this->m_sessionTotalRead += totalRead;
   if (this->m_isChunkedResponseType)
   {

sdk/core/azure-core/test/ut/CMakeLists.txt

@@ -11,10 +11,13 @@ set(CMAKE_CXX_STANDARD_REQUIRED True)
 
 add_executable (
      ${TARGET_NAME}
+     http.cpp
      main.cpp
      nullable.cpp
-     http.cpp
-     string.cpp)
+     string.cpp
+     transport_adapter.cpp
+     transport_adapter.hpp
+     )
 
 target_link_libraries(${TARGET_NAME} PRIVATE azure-core)
 add_gtest(${TARGET_NAME})

sdk/core/azure-core/test/ut/transport_adapter.cpp

@@ -0,0 +1,166 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+#include "transport_adapter.hpp"
+#include <string>
+
+namespace Azure { namespace Core { namespace Test {
+
+  static std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> CreatePolicies()
+  {
+    std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> p;
+    std::shared_ptr<Azure::Core::Http::HttpTransport> transport
+        = std::make_shared<Azure::Core::Http::CurlTransport>();
+
+    p.push_back(std::make_unique<Azure::Core::Http::TransportPolicy>(std::move(transport)));
+    return p;
+  }
+
+  std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> TransportAdapter::policies
+      = CreatePolicies();
+
+  Azure::Core::Http::HttpPipeline TransportAdapter::pipeline(policies);
+  Azure::Core::Context TransportAdapter::context = Azure::Core::Context();
+
+  void TransportAdapter::CheckBodyStreamLength(
+      Azure::Core::Http::BodyStream& body,
+      int64_t size,
+      std::string expectedBody)
+  {
+    EXPECT_EQ(body.Length(), size);
+    auto bodyVector = Azure::Core::Http::BodyStream::ReadToEnd(context, body);
+    if (size > 0)
+    { // only for known body size
+      EXPECT_EQ(bodyVector.size(), size);
+    }
+
+    if (expectedBody.size() > 0)
+    {
+      auto bodyString = std::string(bodyVector.begin(), bodyVector.end());
+      EXPECT_STREQ(expectedBody.data(), bodyString.data());
+    }
+  }
+
+  TEST_F(TransportAdapter, get)
+  {
+    std::string host("http://httpbin.org/get");
+
+    auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+    auto body = response->GetBodyStream();
+    auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length"));
+    CheckBodyStreamLength(*body, expectedResponseBodySize);
+
+    // Add a header and send again. Response should return that header in the body
+    request.AddHeader("123", "456");
+    response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+    body = response->GetBodyStream();
+    // header length is 6 (data) + 13 (formating) -> `    "123": "456"\r\n,`
+    CheckBodyStreamLength(*body, expectedResponseBodySize + 6 + 13);
+  }
+
+  TEST_F(TransportAdapter, getLoop)
+  {
+    std::string host("http://httpbin.org/get");
+
+    auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host);
+
+    // loop sending request
+    for (auto i = 0; i < 20; i++)
+    {
+      auto response = pipeline.Send(context, request);
+      auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length"));
+      EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+      auto body = response->GetBodyStream();
+      CheckBodyStreamLength(*body, expectedResponseBodySize);
+    }
+  }
+
+  TEST_F(TransportAdapter, head)
+  {
+    std::string host("http://httpbin.org/get");
+    auto expectedResponseBodySize = 0;
+
+    auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Head, host);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+    auto body = response->GetBodyStream();
+    CheckBodyStreamLength(*body, expectedResponseBodySize);
+
+    // Check content-length header to be greater than 0
+    int64_t contentLengthHeader = std::stoull(response->GetHeaders().at("content-length"));
+    EXPECT_TRUE(contentLengthHeader > 0);
+  }
+
+  TEST_F(TransportAdapter, put)
+  {
+    std::string host("http://httpbin.org/put");
+
+    // PUT 1MB
+    auto requestBodyVector = std::vector<uint8_t>(1024 * 1024, 'x');
+    auto bodyRequest = Azure::Core::Http::MemoryBodyStream(requestBodyVector);
+    auto request
+        = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+    auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length"));
+
+    auto body = response->GetBodyStream();
+    CheckBodyStreamLength(*body, expectedResponseBodySize);
+  }
+
+  TEST_F(TransportAdapter, deleteRequest)
+  {
+    std::string host("http://httpbin.org/delete");
+
+    // Delete with 1MB payload
+    auto requestBodyVector = std::vector<uint8_t>(1024 * 1024, 'x');
+    auto bodyRequest = Azure::Core::Http::MemoryBodyStream(requestBodyVector);
+    auto request
+        = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Delete, host, &bodyRequest);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+
+    auto body = response->GetBodyStream();
+    auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length"));
+    CheckBodyStreamLength(*body, expectedResponseBodySize);
+  }
+
+  TEST_F(TransportAdapter, patch)
+  {
+    std::string host("http://httpbin.org/patch");
+
+    // Patch with 1kb payload
+    auto requestBodyVector = std::vector<uint8_t>(1024, 'x');
+    auto bodyRequest = Azure::Core::Http::MemoryBodyStream(requestBodyVector);
+    auto request
+        = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Patch, host, &bodyRequest);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+
+    auto body = response->GetBodyStream();
+    auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length"));
+    CheckBodyStreamLength(*body, expectedResponseBodySize);
+  }
+
+  TEST_F(TransportAdapter, getChunk)
+  {
+    std::string host("http://anglesharp.azurewebsites.net/Chunked");
+    auto expectedResponseBodySize = -1; // chunked will return unknown body length
+    auto expectedChunkResponse = std::string(
+        "<!DOCTYPE html>\r\n<html lang=en>\r\n<head>\r\n<meta charset='utf-8'>\r\n<title>Chunked "
+        "transfer encoding test</title>\r\n</head>\r\n<body><h1>Chunked transfer encoding "
+        "test</h1><h5>This is a chunked response after 100 ms.</h5><h5>This is a chunked "
+        "response after 1 second. The server should not close the stream before all chunks are "
+        "sent to a client.</h5></body></html>");
+
+    auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host);
+    auto response = pipeline.Send(context, request);
+    EXPECT_TRUE(response->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok);
+    auto body = response->GetBodyStream();
+    CheckBodyStreamLength(*body, expectedResponseBodySize, expectedChunkResponse);
+  }
+
+}}} // namespace Azure::Core::Test

sdk/core/azure-core/test/ut/transport_adapter.hpp

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+#include "gtest/gtest.h"
+#include <http/body_stream.hpp>
+#include <http/curl/curl.hpp>
+#include <http/http.hpp>
+#include <http/pipeline.hpp>
+
+#include <memory>
+#include <vector>
+
+namespace Azure { namespace Core { namespace Test {
+
+  class TransportAdapter : public ::testing::Test {
+  protected:
+    static Azure::Core::Http::HttpPipeline pipeline;
+    static std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> policies;
+    static Azure::Core::Context context;
+
+    static void CheckBodyStreamLength(
+        Azure::Core::Http::BodyStream& body,
+        int64_t size,
+        std::string expectedBody = std::string(""));
+  };
+
+}}} // namespace Azure::Core::Test