From 23729124bde0ade0e89e601540beb08d1ac25d75 Mon Sep 17 00:00:00 2001
From: Ahson Khan <ahson_ahmedk@yahoo.com>
Date: Tue, 24 Sep 2024 13:14:08 -0700
Subject: [PATCH] Make the HTTP transport behavior consistent between WinHTTP
 and libcurl by disabling automatically following redirects on Windows.
 (#6018)

* Make the HTTP transport behavior consistent between WinHTTP and libCurl by disabling automatically following redirects on Windows.

* Re-enable APC test.

* Fix casing in changelog.

* Add http bin based unit test and fix typo in doc comment for curl.

* Update test name to avoid 'dont'

* Fix typo.
---
 sdk/core/azure-core/CHANGELOG.md                  |  2 ++
 .../src/http/curl/curl_connection_private.hpp     |  5 ++---
 .../src/http/winhttp/win_http_transport.cpp       | 10 ++++++++++
 .../test/ut/transport_adapter_base_test.cpp       | 15 +++++++++++++++
 .../test/ut/transport_adapter_base_test.hpp       |  8 ++++++++
 5 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md
index 446901c05..b3a84ac34 100644
--- a/sdk/core/azure-core/CHANGELOG.md
+++ b/sdk/core/azure-core/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- Make the HTTP transport behavior consistent between WinHTTP and libcurl by disabling automatically following redirects on Windows.
+
 ### Other Changes
 
 ## 1.14.0-beta.2 (2024-09-12)
diff --git a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp
index 7c360d3fb..1ddd0eae2 100644
--- a/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp
+++ b/sdk/core/azure-core/src/http/curl/curl_connection_private.hpp
@@ -33,10 +33,9 @@ typedef struct x509_store_ctx_st X509_STORE_CTX;
 namespace Azure { namespace Core {
   namespace _detail {
     /**
-     * @brief  Unique handle for WinHTTP HINTERNET handles.
+     * @brief  Unique handle for CURL handles.
      *
-     * @note HINTERNET is declared as a "void *". This means that this definition subsumes all other
-     * `void *` types when used with Azure::Core::_internal::UniqueHandle.
+     * @note CURL is declared as a "void".
      *
      */
     template <> struct UniqueHandleHelper<CURL>
diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
index 7172786d0..89e350f12 100644
--- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
+++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
@@ -1043,6 +1043,16 @@ _detail::WinHttpRequest::WinHttpRequest(
     }
   }
 
+  DWORD disableRedirects = WINHTTP_DISABLE_REDIRECTS;
+  if (!WinHttpSetOption(
+          m_requestHandle.get(),
+          WINHTTP_OPTION_DISABLE_FEATURE,
+          &disableRedirects,
+          sizeof(disableRedirects)))
+  {
+    GetErrorAndThrow("Error while disabling redirects.");
+  }
+
   // Set the callback function to be called whenever the state of the request handle changes.
   m_httpAction = std::make_unique<_detail::WinHttpAction>(this);
 
diff --git a/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp b/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp
index cf667e04b..4daf33ac8 100644
--- a/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp
+++ b/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp
@@ -389,6 +389,21 @@ namespace Azure { namespace Core { namespace Test {
     t1.join();
   }
 
+  TEST_P(TransportAdapter, redirectsNotFollowed)
+  {
+    // We don't expect the transport adapter to follow redirects automatically to this url.
+    std::string redirectToUrl = AzureSdkHttpbinServer::ResponseHeaders("foo=bar");
+
+    Azure::Core::Http::Request request(
+        Azure::Core::Http::HttpMethod::Get,
+        Azure::Core::Url(AzureSdkHttpbinServer::RedirectTo(redirectToUrl)));
+
+    auto response = m_pipeline->Send(request, Context{});
+    EXPECT_EQ(response->GetStatusCode(), Azure::Core::Http::HttpStatusCode::Found);
+    EXPECT_TRUE(response->GetHeaders().find("location") != response->GetHeaders().end());
+    EXPECT_EQ(response->GetHeaders().at("location"), redirectToUrl);
+  }
+
   TEST_P(TransportAdapter, cancelRequest)
   {
     Azure::Core::Url hostPath(AzureSdkHttpbinServer::Delay() + "/2"); // 2 seconds delay on server
diff --git a/sdk/core/azure-core/test/ut/transport_adapter_base_test.hpp b/sdk/core/azure-core/test/ut/transport_adapter_base_test.hpp
index 5429def0e..a7d8cab89 100644
--- a/sdk/core/azure-core/test/ut/transport_adapter_base_test.hpp
+++ b/sdk/core/azure-core/test/ut/transport_adapter_base_test.hpp
@@ -38,6 +38,14 @@ namespace Azure { namespace Core { namespace Test {
     {
       return Schema() + "://" + Host() + "/status/" + std::to_string(statusCode);
     }
+    inline static std::string ResponseHeaders(std::string responseHeaders)
+    {
+      return Schema() + "://" + Host() + "/response-headers?" + responseHeaders;
+    }
+    inline static std::string RedirectTo(std::string redirectToUrl)
+    {
+      return Schema() + "://" + Host() + "/redirect-to?url=" + redirectToUrl;
+    }
     inline static std::string Host() { return std::string(_detail::AzureSdkHttpbinServer); }
     inline static std::string Schema() { return std::string(_detail::AzureSdkHttpbinServerSchema); }
   };