Converted WinHTTP to Async.

.vscode/cspell.json

@@ -73,6 +73,7 @@
     "HKEY",
     "HRESULT",
     "IMDS",
+    "immutability",
     "Intel",
     "itfactor",
     "iusg",

cmake-modules/AddGoogleTest.cmake

@@ -78,7 +78,8 @@ macro(add_gtest TESTNAME)
         else()
             gtest_discover_tests(${TESTNAME}
                 TEST_PREFIX "${TESTNAME}."
-                PROPERTIES FOLDER "Tests")
+                PROPERTIES FOLDER "Tests"
+                DISCOVERY_TIMEOUT 600)
         endif()
     else()
         add_test(${TESTNAME} ${TESTNAME})

sdk/core/azure-core-tracing-opentelemetry/test/ut/azure_core_test.cpp

@@ -1,6 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // SPDX-License-Identifier: MIT
 
+#include "azure/core/internal/diagnostics/global_exception.hpp"
+#include "azure/core/platform.hpp"
+
+#include <csignal>
 #include <gtest/gtest.h>
 
 int main(int argc, char** argv)
@@ -10,6 +14,17 @@ int main(int argc, char** argv)
   // End users need to decide if SIGPIPE should be ignored or not.
   signal(SIGPIPE, SIG_IGN);
 #endif
+  // Declare a signal handler to report unhandled exceptions on Windows - this is not needed for
+  // other
+// OS's as they will print the exception to stderr in their terminate() function.
+#if defined(AZ_PLATFORM_WINDOWS)
+  // Ensure that all calls to abort() no longer pop up a modal dialog on Windows.
+#if defined(_DEBUG) && defined(_MSC_VER)
+  _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG);
+#endif
+
+  signal(SIGABRT, Azure::Core::Diagnostics::_internal::GlobalExceptionHandler::HandleSigAbort);
+#endif // AZ_PLATFORM_WINDOWS
 
   testing::InitGoogleTest(&argc, argv);
   auto r = RUN_ALL_TESTS();

sdk/core/azure-core/CMakeLists.txt

@@ -54,10 +54,11 @@ endif()
 if(BUILD_TRANSPORT_WINHTTP)
   SET(WIN_TRANSPORT_ADAPTER_SRC 
     src/http/winhttp/win_http_transport.cpp
+    src/http/winhttp/win_http_request.hpp
   )
- SET(WIN_TRANSPORT_ADAPTER_INC 
+  SET(WIN_TRANSPORT_ADAPTER_INC 
     inc/azure/core/http/win_http_transport.hpp
- )
+  )
 endif()
 
 set(

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

@@ -5,7 +5,6 @@
  * @file
  * @brief #Azure::Core::Http::HttpTransport implementation via WinHTTP.
  */
-// cspell:words HCERTIFICATECHAIN PCCERT CCERT HCERTCHAINENGINE HCERTSTORE
 
 #pragma once
 
@@ -14,8 +13,7 @@
 #include "azure/core/http/policies/policy.hpp"
 #include "azure/core/http/transport.hpp"
 #include "azure/core/internal/unique_handle.hpp"
-
-#include <azure/core/platform.hpp>
+#include "azure/core/platform.hpp"
 
 #if defined(AZ_PLATFORM_WINDOWS)
 #if !defined(WIN32_LEAN_AND_MEAN)
@@ -57,53 +55,8 @@ namespace Azure { namespace Core {
       constexpr static size_t DefaultUploadChunkSize = 1024 * 64;
       constexpr static size_t MaximumUploadChunkSize = 1024 * 1024;
 
-      class WinHttpStream final : public Azure::Core::IO::BodyStream {
-      private:
-        Azure::Core::_internal::UniqueHandle<HINTERNET> m_requestHandle;
-        bool m_isEOF;
-
-        /**
-         * @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 avoids parsing the string header
-         * every time from HTTP RawResponse.
-         *
-         * @remark This value is also used to avoid trying to read more data from network than what
-         * we are expecting to.
-         *
-         * @remark A value of -1 means the transfer encoding was chunked.
-         *
-         */
-        int64_t m_contentLength;
-
-        int64_t m_streamTotalRead;
-
-        /**
-         * @brief Implement #Azure::Core::IO::BodyStream::OnRead(). Calling this function pulls data
-         * from the wire.
-         *
-         * @param context A context to control the request lifetime.
-         * @param buffer Buffer where data from wire is written to.
-         * @param count The number of bytes to read from the network.
-         * @return The actual number of bytes read from the network.
-         */
-        size_t OnRead(uint8_t* buffer, size_t count, Azure::Core::Context const& context) override;
-
-      public:
-        WinHttpStream(
-            Azure::Core::_internal::UniqueHandle<HINTERNET>& requestHandle,
-            int64_t contentLength)
-            : m_requestHandle(std::move(requestHandle)), m_contentLength(contentLength),
-              m_isEOF(false), m_streamTotalRead(0)
-        {
-        }
-
-        /**
-         * @brief Implement #Azure::Core::IO::BodyStream length.
-         *
-         * @return The size of the payload.
-         */
-        int64_t Length() const override { return this->m_contentLength; }
-      };
+      // Forward declaration for WinHttpRequest.
+      class WinHttpRequest;
     } // namespace _detail
 
     /**
@@ -173,70 +126,22 @@ namespace Azure { namespace Core {
     class WinHttpTransport : public HttpTransport {
     private:
       WinHttpTransportOptions m_options;
-
-      // This should remain immutable and not be modified after calling the ctor, to avoid threading
-      // issues.
-      Azure::Core::_internal::UniqueHandle<HINTERNET> m_sessionHandle;
-      bool m_requestHandleClosed{false};
+      // m_sessionhandle is const to ensure immutability.
+      const Azure::Core::_internal::UniqueHandle<HINTERNET> m_sessionHandle;
 
       Azure::Core::_internal::UniqueHandle<HINTERNET> CreateSessionHandle();
       Azure::Core::_internal::UniqueHandle<HINTERNET> CreateConnectionHandle(
           Azure::Core::Url const& url,
           Azure::Core::Context const& context);
-      Azure::Core::_internal::UniqueHandle<HINTERNET> CreateRequestHandle(
+      std::unique_ptr<_detail::WinHttpRequest> CreateRequestHandle(
           Azure::Core::_internal::UniqueHandle<HINTERNET> const& connectionHandle,
           Azure::Core::Url const& url,
           Azure::Core::Http::HttpMethod const& method);
-      void Upload(
-          Azure::Core::_internal::UniqueHandle<HINTERNET> const& requestHandle,
-          Azure::Core::Http::Request& request,
-          Azure::Core::Context const& context);
-      void SendRequest(
-          Azure::Core::_internal::UniqueHandle<HINTERNET> const& requestHandle,
-          Azure::Core::Http::Request& request,
-          Azure::Core::Context const& context);
-      void ReceiveResponse(
-          Azure::Core::_internal::UniqueHandle<HINTERNET> const& requestHandle,
-          Azure::Core::Context const& context);
-      int64_t GetContentLength(
-          Azure::Core::_internal::UniqueHandle<HINTERNET> const& requestHandle,
-          HttpMethod requestMethod,
-          HttpStatusCode responseStatusCode);
-      std::unique_ptr<RawResponse> SendRequestAndGetResponse(
-          Azure::Core::_internal::UniqueHandle<HINTERNET>& requestHandle,
-          HttpMethod requestMethod);
-
-      /*
-       * Callback from WinHTTP called after the TLS certificates are received when the caller sets
-       * expected TLS root certificates.
-       */
-      static void CALLBACK StatusCallback(
-          HINTERNET hInternet,
-          DWORD_PTR dwContext,
-          DWORD dwInternetStatus,
-          LPVOID lpvStatusInformation,
-          DWORD dwStatusInformationLength) noexcept;
-      /*
-       * Callback from WinHTTP called after the TLS certificates are received when the caller sets
-       * expected TLS root certificates.
-       */
-      void OnHttpStatusOperation(HINTERNET hInternet, DWORD dwInternetStatus);
-      /*
-       * Adds the specified trusted certificates to the specified certificate store.
-       */
-      bool AddCertificatesToStore(
-          std::vector<std::string> const& trustedCertificates,
-          HCERTSTORE const hCertStore);
-      /*
-       * Verifies that the certificate context is in the trustedCertificates set of certificates.
-       */
-      bool VerifyCertificatesInChain(
-          std::vector<std::string> const& trustedCertificates,
-          PCCERT_CONTEXT serverCertificate);
 
       // Callback to allow a derived transport to extract the request handle. Used for WebSocket
       // transports.
-      virtual void OnUpgradedConnection(Azure::Core::_internal::UniqueHandle<HINTERNET> const&){};
+      virtual void OnUpgradedConnection(std::unique_ptr<_detail::WinHttpRequest> const&){};
+
       /**
        * @brief Throw an exception based on the Win32 Error code
        *
@@ -266,8 +171,8 @@ namespace Azure { namespace Core {
       WinHttpTransport(Azure::Core::Http::Policies::TransportOptions const& options);
 
       /**
-       * @brief Implements the HTTP transport interface to send an HTTP Request and produce an HTTP
-       * RawResponse.
+       * @brief Implements the HTTP transport interface to send an HTTP Request and produce an
+       * HTTP RawResponse.
        *
        * @param context A context to control the request lifetime.
        * @param request an HTTP request to be send.
@@ -279,7 +184,7 @@ namespace Azure { namespace Core {
       // [Core Guidelines C.35: "A base class destructor should be either public
       // and virtual or protected and
       // non-virtual"](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual)
-      virtual ~WinHttpTransport() = default;
+      virtual ~WinHttpTransport();
     };
 
   } // namespace Http

sdk/core/azure-core/inc/azure/core/internal/diagnostics/global_exception.hpp

@@ -0,0 +1,39 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+#pragma once
+#include <exception>
+#include <iostream>
+
+namespace Azure { namespace Core { namespace Diagnostics { namespace _internal {
+  /**
+   * @brief Global Exception handler used for test collateral. This is not intended to be used by
+   * any non-test code.
+   *
+   * The GlobalExceptionHandler class is used to catch any unhandled exceptions and report them as a
+   * part of test collateral, it is intended to be called as a SIGABRT handler set by:
+   *
+   * ```
+   *  signal(SIGABRT, Azure::Core::Diagnostics::_internal::GlobalExceptionHandler);
+   * ```
+   *
+   */
+  struct GlobalExceptionHandler
+  {
+    static void HandleSigAbort(int)
+    {
+      // Rethrow any exceptions on the current stack - this will cause any pending exceptions to
+      // be thrown so we can catch them and report them to the caller. This is needed because the
+      // terminate() function on Windows calls abort() which normally pops up a modal dialog box
+      // after which it terminates the application without reporting the exception.
+      try
+      {
+        throw;
+      }
+      catch (std::exception const& ex)
+      {
+        std::cerr << "SIGABRT raised, exception: " << ex.what() << std::endl;
+      }
+    }
+  };
+}}}} // namespace Azure::Core::Diagnostics::_internal

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

@@ -132,7 +132,7 @@ int pollSocketUntilEventOrTimeout(
   auto deadline = now + std::chrono::milliseconds(timeout);
   while (now < deadline)
   {
-    // check cancelation
+    // Before doing any work, check to make sure that the context hasn't already been cancelled.
     context.ThrowIfCancelled();
     int pollTimeoutMs = static_cast<int>(
         std::min(

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

@@ -176,6 +176,7 @@ std::unique_ptr<RawResponse> RetryPolicy::Send(
     // we proceed immediately if it is 0.
     if (retryAfter.count() > 0)
     {
+      // Before sleeping, check to make sure that the context hasn't already been cancelled.
       context.ThrowIfCancelled();
       std::this_thread::sleep_for(retryAfter);
     }

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

@@ -97,6 +97,7 @@ std::unique_ptr<RawResponse> TransportPolicy::Send(
     NextHttpPolicy,
     Context const& context) const
 {
+  // Before doing any work, check to make sure that the context hasn't already been cancelled.
   context.ThrowIfCancelled();
 
   /*