Bug: `Host` header is not correctly set for non-default port

[Jinming-Hu]

It's specified in RFC2616 that

A "host" without any trailing port information implies the default
port for the service requested (e.g., "80" for an HTTP URL).

so for HTTP or HTTPS that are not running on 80 or 443, we need to include port in Host header.

This is a bug in our code:

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

Line 414 in f872841

this->m_request.SetHeader("Host", this->m_request.GetUrl().GetHost());

[Jinming-Hu]

this issue can be fixed with below path

diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp
index f8467e0c..30dd4b00 100644
--- a/sdk/core/azure-core/src/http/curl/curl.cpp
+++ b/sdk/core/azure-core/src/http/curl/curl.cpp
@@ -411,7 +411,13 @@ 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 headerValue = this->m_request.GetUrl().GetHost();
+      auto port = this->m_request.GetUrl().GetPort();
+      if (port != 0)
+      {
+        headerValue += ":" + std::to_string(port);
+      }
+      this->m_request.SetHeader("Host", headerValue);
     }
     auto isContentLengthHeaderInRequest = headers.find("content-length");
     if (isContentLengthHeaderInRequest == headers.end())

[ahsonkhan]

so for HTTP or HTTPS that are not running on 80 or 443, we need to include port in Host header.

In what scenario would that happen? How did you run into this issue?

cc @LarryOsterman, @vhvb1989

[Jinming-Hu]

In what scenario would that happen? How did you run into this issue?

When we send requests to non 80 or 443 ports.

I ran into this issue when running test-proxy (localhost:5001) through Fiddler.