github/azure-sdk-for-cpp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Add HTTP request version logging, and scope the variables (#5643)

Browse Source 
Co-authored-by: Anton Kolesnyk <antkmsft@users.noreply.github.com>
This commit is contained in:
Anton Kolesnyk 2024-05-20 14:06:55 -07:00 committed by GitHub
parent 597d12fd6a
commit fdb8657f41
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 25 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
sdk/core/azure-core/src/http/curl/curl.cpp
View File

@ -933,20 +933,6 @@ CURLcode CurlSession::ReadStatusLineAndHeadersFromRawResponse(
}
else
{
bool hasConnectionKeepAlive = false;
bool hasConnectionClose = false;
{
const Core::CaseInsensitiveMap& responseHeaders = m_response->GetHeaders();
const auto connectionHeader = responseHeaders.find("Connection");
if (connectionHeader != responseHeaders.cend())
{
const std::string headerValueLowercase
= Core::_internal::StringExtensions::ToLower(connectionHeader->second);
hasConnectionKeepAlive = headerValueLowercase.find("keep-alive") != std::string::npos;
hasConnectionClose = headerValueLowercase.find("close") != std::string::npos;
}
}
// HTTP <=1.0 is "close" by default. HTTP 1.1 is "keep-alive" by default.
// The value can also be "keep-alive, close" (i.e. "both are fine"), in which case we are
// preferring to treat it as keep-alive.
@ -962,9 +948,25 @@ CURLcode CurlSession::ReadStatusLineAndHeadersFromRawResponse(
// would only mean there's a perf hit, but the communication flow is expected to be correct.
if (m_response->GetMajorVersion() == 1)
{
std::string connectionHeaderValue;
{
const Core::CaseInsensitiveMap& responseHeaders = m_response->GetHeaders();
const auto connectionHeader = responseHeaders.find("Connection");
if (connectionHeader != responseHeaders.cend())
{
connectionHeaderValue
= Core::_internal::StringExtensions::ToLower(connectionHeader->second);
}
}
const bool hasConnectionKeepAlive
= connectionHeaderValue.find("keep-alive") != std::string::npos;
if (m_response->GetMinorVersion() >= 1)
{
// HTTP/1.1+
const bool hasConnectionClose = connectionHeaderValue.find("close") != std::string::npos;
m_httpKeepAlive = (!hasConnectionClose || hasConnectionKeepAlive);
}
else
@ -975,13 +977,7 @@ CURLcode CurlSession::ReadStatusLineAndHeadersFromRawResponse(
}
else
{
Log::Write(
Logger::Level::Verbose,
LogMsgPrefix
+ "Unsupported HTTP version in the response. Only HTTP/1.x is supported as a "
"response, for an HTTP/1.1 request.");
// We don't expect HTTP/2.0 or 3.0 as a response, when sending an HTTP/1.1 request.
// We don't expect HTTP/0.9, 2.0 or 3.0 in responses.
// Barring rejecting as malformed, the safest thing to do here is to assume the connection is
// not reusable.
m_httpKeepAlive = false;

2
sdk/core/azure-core/src/http/log_policy.cpp
View File

@ -56,7 +56,7 @@ inline std::string GetResponseLogMessage(
{
std::ostringstream log;
log << "HTTP Response ("
log << "HTTP/" << response.GetMajorVersion() << '.' << response.GetMinorVersion() << " Response ("
<< std::chrono::duration_cast<std::chrono::milliseconds>(duration).count()
<< "ms) : " << static_cast<int>(response.GetStatusCode()) << " "
<< response.GetReasonPhrase();

14
sdk/core/azure-core/test/ut/log_policy_test.cpp
View File

@ -185,7 +185,7 @@ TEST(LogPolicy, Default)
"\nheader2 : REDACTED"
"\nx-ms-request-id : 6c536700-4c36-4e22-9161-76e7b3bf8269");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}
@ -214,7 +214,7 @@ TEST(LogPolicy, PortAndPath)
"\nheader2 : REDACTED"
"\nx-ms-request-id : 6c536700-4c36-4e22-9161-76e7b3bf8269");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}
@ -248,7 +248,7 @@ TEST(LogPolicy, Headers)
"\nheader2 : REDACTED"
"\nx-ms-request-id : 6c536700-4c36-4e22-9161-76e7b3bf8269");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}
@ -305,7 +305,7 @@ TEST(LogPolicy, DefaultHeaders)
"\nx-ms-request-id : x-ms-request-id"
"\nx-ms-return-client-request-id : x-ms-return-client-request-id");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
// Ensure that the entire list of allowed headers is in the list of headers.
@ -345,7 +345,7 @@ TEST(LogPolicy, QueryParams)
"\nheader2 : REDACTED"
"\nx-ms-request-id : REDACTED");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}
@ -374,7 +374,7 @@ TEST(LogPolicy, QueryParamsUnencoded)
"\nheader2 : REDACTED"
"\nx-ms-request-id : REDACTED");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}
@ -403,6 +403,6 @@ TEST(LogPolicy, QueryParamsEncoded)
"\nheader2 : REDACTED"
"\nx-ms-request-id : REDACTED");
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP Response ("));
EXPECT_TRUE(StartsWith(entry2.Message, "HTTP/1.1 Response ("));
EXPECT_TRUE(EndsWith(entry2.Message, "ms) : 200 OKAY"));
}