Test proxy & storage tests improvements

sdk/attestation/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "cpp",
   "TagPrefix": "cpp/attestation",
-  "Tag": "cpp/attestation_10abfdc3e0"
+  "Tag": "cpp/attestation_6398169251"
 }

sdk/attestation/azure-security-attestation/test/ut/policycertmgmt_test.cpp

@@ -171,8 +171,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test {
 
   TEST_F(CertificateTests, AddPolicyManagementCertificate_LIVEONLY_)
   {
-    CHECK_SKIP_TEST()
-
     auto adminClient(CreateClient(ServiceInstanceType::Isolated));
 
     auto isolatedCertificateBase64(GetEnv("ISOLATED_SIGNING_CERTIFICATE"));
@@ -233,8 +231,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test {
 
   TEST_F(CertificateTests, RemovePolicyManagementCertificate_LIVEONLY_)
   {
-    CHECK_SKIP_TEST()
-
     auto adminClient(CreateClient(ServiceInstanceType::Isolated));
 
     auto isolatedCertificateBase64(GetEnv("ISOLATED_SIGNING_CERTIFICATE"));

sdk/core/azure-core-test/inc/azure/core/test/network_models.hpp

@@ -8,14 +8,6 @@
 
 #pragma once
 
-#include <azure/core/http/policies/policy.hpp>
-#include <azure/core/io/body_stream.hpp>
-
-#include <list>
-#include <map>
-#include <string>
-#include <vector>
-
 namespace Azure { namespace Core { namespace Test {
 
   /**
@@ -29,109 +21,4 @@ namespace Azure { namespace Core { namespace Test {
     LIVE,
   };
 
-  /**
-   * @brief Keeps track of network call records from each unit test session.
-   *
-   */
-  struct NetworkCallRecord
-  {
-    std::string Method;
-    std::string Url;
-    std::map<std::string, std::string> Headers;
-    std::map<std::string, std::string> Response;
-  };
-
-  /**
-   * @brief Keeps track of the network calls and variable names that were made in a test
-   * session.
-   *
-   */
-  class RecordedData {
-  public:
-    std::list<NetworkCallRecord> NetworkCallRecords;
-    std::list<std::string> Variables;
-  };
-
-  /**
-   * @brief A body stream which holds the memory inside.
-   *
-   * @remark The playback http uses this body stream to be returned as part of the raw response so
-   * the transport policy can read from it.
-   *
-   */
-  class WithMemoryBodyStream : public Azure::Core::IO::BodyStream {
-  private:
-    std::vector<uint8_t> m_memory;
-    Azure::Core::IO::MemoryBodyStream m_streamer;
-
-    size_t OnRead(uint8_t* buffer, size_t count, Azure::Core::Context const& context) override
-    {
-      return m_streamer.Read(buffer, count, context);
-    }
-
-  public:
-    // Forbid constructor for rval so we don't end up storing dangling ptr
-    WithMemoryBodyStream(std::vector<uint8_t> const&&) = delete;
-
-    /**
-     * @brief Construct using vector of bytes.
-     *
-     * @param buffer Vector of bytes with the contents to provide the data from to the readers.
-     */
-    WithMemoryBodyStream(std::vector<uint8_t> const& buffer)
-        : m_memory(buffer), m_streamer(m_memory)
-    {
-    }
-
-    int64_t Length() const override { return m_streamer.Length(); }
-
-    void Rewind() override { m_streamer.Rewind(); }
-  };
-
-  /**
-   * @brief Wraps a stream and keep reading bytes from it by rewinding it until some length.
-   *
-   * @note Enables to create a stream with huge size by re-using a small buffer (1Mb).
-   *
-   */
-  class CircularBodyStream : public Azure::Core::IO::BodyStream {
-  private:
-    std::unique_ptr<std::vector<uint8_t>> m_buffer;
-    size_t m_length;
-    size_t m_totalRead = 0;
-    Azure::Core::IO::MemoryBodyStream m_memoryStream;
-
-    size_t OnRead(uint8_t* buffer, size_t count, Azure::Core::Context const& context) override
-    {
-      auto available = m_length - m_totalRead;
-      if (available == 0)
-      {
-        return 0;
-      }
-
-      auto toRead = std::min(count, available);
-      auto read = m_memoryStream.Read(buffer, toRead, context);
-
-      // Circurlar implementation. Rewind the stream every time we reach the end
-      if (read == 0) // No more bytes to read from.
-      {
-        m_memoryStream.Rewind();
-        read = m_memoryStream.Read(buffer, toRead, context);
-      }
-
-      m_totalRead += read;
-      return read;
-    }
-
-  public:
-    CircularBodyStream(size_t size, uint8_t fillWith)
-        : m_buffer(std::make_unique<std::vector<uint8_t>>(1024 * 1024, fillWith)), m_length(size),
-          m_memoryStream(*m_buffer)
-    {
-    }
-
-    int64_t Length() const override { return m_length; }
-    void Rewind() override { m_totalRead = 0; }
-  };
-
 }}} // namespace Azure::Core::Test

sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp

@@ -22,13 +22,6 @@
 #include <regex>
 #include <thread>
 
-#define CHECK_SKIP_TEST() \
-  std::string const readTestNameAndUpdateTestContext = GetTestName(); \
-  if (shouldSkipTest()) \
-  { \
-    GTEST_SKIP(); \
-  }
-
 using namespace std::chrono_literals;
 
 namespace Azure { namespace Core { namespace Test {
@@ -143,28 +136,6 @@ namespace Azure { namespace Core { namespace Test {
       }
     }
 
-    inline void ValidateSkippingTest()
-    {
-      if (m_wasSkipped)
-      {
-        GTEST_SKIP();
-      }
-    }
-
-    bool IsValidTime(const Azure::DateTime& datetime)
-    {
-      // Playback won't check dates
-      if (m_testContext.IsPlaybackMode())
-      {
-        return true;
-      }
-
-      // We assume datetime within a week is valid.
-      const auto minTime = std::chrono::system_clock::now() - std::chrono::hours(24 * 7);
-      const auto maxTime = std::chrono::system_clock::now() + std::chrono::hours(24 * 7);
-      return datetime > minTime && datetime < maxTime;
-    }
-
     // Reads the current test instance name.
     // Name gets also sanitized (special chars are removed) to avoid issues when recording or
     // creating
@@ -236,11 +207,13 @@ namespace Azure { namespace Core { namespace Test {
       return std::make_unique<T>(url, credential, options);
     }
 
+    template <class T> void InitClientOptions(T& options) { PrepareOptions(options); }
+
     template <class T> T InitClientOptions()
     {
       // Run instrumentation before creating the client
       T options;
-      PrepareOptions(options);
+      InitClientOptions(options);
       return options;
     }
 
@@ -395,6 +368,12 @@ namespace Azure { namespace Core { namespace Test {
       {
         m_testProxy = std::make_unique<Azure::Core::Test::TestProxyManager>(m_testContext);
       }
+
+      std::string const readTestNameAndUpdateTestContext = GetTestName();
+      if (shouldSkipTest())
+      {
+        GTEST_SKIP();
+      }
     }
 
     /**

sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp

@@ -139,18 +139,6 @@ namespace Azure { namespace Core { namespace Test {
       return std::make_shared<TestNonExpiringCredential>();
     }
 
-    /**
-     * Sets proxy to stop RECORD test and save the recording file.
-     *
-     */
-    void SetStopRecordMode();
-
-    /**
-     * Sets proxy to stop PLAYBACK test.
-     *
-     */
-    void SetStopPlaybackMode();
-
     /**
      * Gets the test recording ID
      *

sdk/core/azure-core-test/src/test_base.cpp

@@ -3,7 +3,6 @@
 
 #include <azure/core/internal/json/json.hpp>
 
-#include "azure/core/test/network_models.hpp"
 #include "azure/core/test/test_base.hpp"
 
 #include <fstream>

sdk/core/azure-core-test/src/test_proxy_manager.cpp

@@ -94,8 +94,6 @@ void TestProxyManager::StartPlaybackRecord(TestMode testMode)
 {
   if (IsPlaybackMode() || IsRecordMode())
   {
-    std::string mode = (IsPlaybackMode() ? "playback" : "record");
-    std::cout << "TestProxy already in " + mode + " mode.";
     return;
   }
 
@@ -225,12 +223,50 @@ void TestProxyManager::SetProxySanitizer()
   Azure::Core::Url matcherRequest(m_proxy);
   matcherRequest.AppendPath("Admin");
   matcherRequest.AppendPath("SetMatcher");
-  const std::string matcherBody
-      = "{\"compareBodies\": false ,\"ignoreQueryOrdering\": true , \"ignoredHeaders\": "
-        "\"x-ms-copy-source,x-ms-proposed-lease-id,x-ms-lease-id,x-ms-file-change-"
-        "time,x-ms-file-creation-time,x-ms-file-last-write-time,x-ms-destination-"
-        "lease-id,x-ms-source-lease-id,x-ms-source-content-crc64,x-ms-content-crc64,x-ms-source-"
-        "content-md5,x-ms-content-md5,Content-MD5\"}";
+  std::string matcherBody;
+  {
+    auto jsonRoot = Json::_internal::json::object();
+    jsonRoot["compareBodies"] = false;
+    jsonRoot["ignoreQueryOrdering"] = true;
+    const std::vector<std::string> excludedHeaders = {
+        "Expect",
+        "Connection",
+    };
+    jsonRoot["excludedHeaders"] = std::accumulate(
+        excludedHeaders.begin(),
+        excludedHeaders.end(),
+        std::string(),
+        [](const std::string& lhs, const std::string& rhs) {
+          return lhs + (lhs.empty() ? "" : ",") + rhs;
+        });
+    const std::vector<std::string> ignoredHeaders = {
+        "x-ms-copy-source",
+        "x-ms-file-change-time",
+        "x-ms-file-creation-time",
+        "x-ms-file-last-write-time",
+        "x-ms-rename-source",
+    };
+    const std::vector<std::string> ignoreQueryParameters = {
+        "st",
+        "se",
+        "sig",
+    };
+    jsonRoot["ignoredHeaders"] = std::accumulate(
+        ignoredHeaders.begin(),
+        ignoredHeaders.end(),
+        std::string(),
+        [](const std::string& lhs, const std::string& rhs) {
+          return lhs + (lhs.empty() ? "" : ",") + rhs;
+        });
+    jsonRoot["ignoredQueryParameters"] = std::accumulate(
+        ignoreQueryParameters.begin(),
+        ignoreQueryParameters.end(),
+        std::string(),
+        [](const std::string& lhs, const std::string& rhs) {
+          return lhs + (lhs.empty() ? "" : ",") + rhs;
+        });
+    matcherBody = jsonRoot.dump();
+  }
 
   {
     Azure::Core::IO::MemoryBodyStream payloadStream(

sdk/core/azure-core-test/src/test_proxy_policy.cpp

@@ -42,15 +42,21 @@ std::unique_ptr<RawResponse> TestProxyPolicy::Send(
   {
     // This is a download with keep connection open. Let's switch the request
     redirectRequest = Azure::Core::Http::Request(
-        request.GetMethod(), Azure::Core::Url(m_testProxy->GetTestProxy()), false);
+        request.GetMethod(),
+        Azure::Core::Url(m_testProxy->GetTestProxy()),
+        request.GetBodyStream(),
+        false);
   }
 
   redirectRequest.GetUrl().SetPath(request.GetUrl().GetPath());
 
   // Copy all headers
   for (auto& header : request.GetHeaders())
   {
-    redirectRequest.SetHeader(header.first, header.second);
+    if (header.first != "host")
+    {
+      redirectRequest.SetHeader(header.first, header.second);
+    }
   }
   // QP
   for (auto const& qp : request.GetUrl().GetQueryParameters())

sdk/core/azure-core/CHANGELOG.md

@@ -8,9 +8,12 @@
 
 ### Bugs Fixed
 
+- Fixed a bug where `Host` request header is not set for non-default port (80, 443).
+
 ### Other Changes
 
 - [[#4352]](https://github.com/Azure/azure-sdk-for-cpp/pull/4352) Fixed compilation error on Visual Studio 2017.
+- Libcurl transport doesn't add `Content-Length` request header for GET/HEAD/DELETE requests anymore.
 
 ### Acknowledgments
 
@@ -584,4 +587,4 @@ Thank you to our developer community members who helped to make Azure Core bette
 
 ## 1.0.0-beta.1 (2020-09-09)
 
-- Initial release
+- Initial release

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

@@ -413,10 +413,18 @@ CURLcode CurlSession::Perform(Context const& context)
     if (hostHeader == headers.end())
     {
       Log::Write(Logger::Level::Verbose, LogMsgPrefix + "No Host in request headers. Adding it");
-      this->m_request.SetHeader("Host", this->m_request.GetUrl().GetHost());
+      std::string hostName = this->m_request.GetUrl().GetHost();
+      auto port = this->m_request.GetUrl().GetPort();
+      if (port != 0)
+      {
+        hostName += ":" + std::to_string(port);
+      }
+      this->m_request.SetHeader("Host", hostName);
     }
-    auto isContentLengthHeaderInRequest = headers.find("content-length");
-    if (isContentLengthHeaderInRequest == headers.end())
+    if (this->m_request.GetMethod() != HttpMethod::Get
+        && this->m_request.GetMethod() != HttpMethod::Head
+        && this->m_request.GetMethod() != HttpMethod::Delete
+        && headers.find("content-length") == headers.end())
     {
       Log::Write(Logger::Level::Verbose, LogMsgPrefix + "No content-length in headers. Adding it");
       this->m_request.SetHeader(

sdk/storage/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "cpp",
   "TagPrefix": "cpp/storage",
-  "Tag": "cpp/storage_90ac51538b"
+  "Tag": "cpp/storage_fc079eacac"
 }

sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_options.hpp

@@ -924,6 +924,8 @@ namespace Azure { namespace Storage { namespace Blobs {
 
     /**
      * @brief Optional conditions that source must meet to perform this operation.
+     * @remarks Azure storage service doesn't support tags access condition for this operation.
+     * Don't use it.
      */
     struct : public Azure::ModifiedConditions,
              public Azure::MatchConditions,