Skip to content

Remove build-time detection of supported WinINet/WinHTTP protocol versions. #3201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Remove build-time detection of supported WinINet/WinHTTP protocol versions. #3201

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a666ad
Remove build-time detection of supported WinINet/WinHTTP protocol ver…
teo-tsirpanis Nov 19, 2024
31b713f
Address PR feedback.
teo-tsirpanis Nov 20, 2024
bec8918
Remove unused conditional compilation directives.
teo-tsirpanis Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 0 additions & 63 deletions src/aws-cpp-sdk-core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,69 +189,6 @@ elseif(ENABLE_WINDOWS_CLIENT)
else()
file(GLOB HTTP_WINDOWS_CLIENT_HEADERS "include/aws/core/http/windows/Win*.h")
file(GLOB HTTP_WINDOWS_CLIENT_SOURCE "${CMAKE_CURRENT_SOURCE_DIR}/source/http/windows/Win*.cpp")

# Starting with Windows 10, version 1507, WinINet supports HTTP2.
# https://docs.microsoft.com/en-us/windows/desktop/WinInet/option-flags#INTERNET_OPTION_ENABLE_HTTP_PROTOCOL
# Starting with Windows 10, version 1607, WinHttp supports HTTP2.
# https://docs.microsoft.com/en-us/windows/desktop/WinHttp/option-flags#WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL
set(CMAKE_REQUIRED_LIBRARIES "WinHttp.lib")
check_cxx_source_runs("
#include <Windows.h>
#include <winhttp.h>
int main() {

auto handle = WinHttpOpen(L\"aws-cpp-sdk\"/*user-agent*/,
WINHTTP_ACCESS_TYPE_NO_PROXY,
nullptr/*pszProxyW*/,
nullptr/*pszProxyBypassW*/,
0/*dwFlags*/);

DWORD http2 = WINHTTP_PROTOCOL_FLAG_HTTP2;
if (WinHttpSetOption(handle, WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL, &http2, sizeof(http2)))
{
return 0;
}
return 1;
}" WINHTTP_HAS_H2)

check_cxx_source_runs("
#include <Windows.h>
#include <winhttp.h>
int main() {

auto handle = WinHttpOpen(L\"aws-cpp-sdk\"/*user-agent*/,
WINHTTP_ACCESS_TYPE_NO_PROXY,
nullptr/*pszProxyW*/,
nullptr/*pszProxyBypassW*/,
0/*dwFlags*/);

DWORD http3 = WINHTTP_PROTOCOL_FLAG_HTTP3;
if (WinHttpSetOption(handle, WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL, &http3, sizeof(http3)))
{
return 0;
}
return 1;
}" WINHTTP_HAS_H3)

set(CMAKE_REQUIRED_LIBRARIES "Wininet.lib")
check_cxx_source_runs("
#include <Windows.h>
#include <WinInet.h>
int main() {
auto handle = InternetOpenA(\"aws-cpp-sdk\"/*lpszAgent*/,
INTERNET_OPEN_TYPE_DIRECT /*dwAccessType*/,
nullptr /*lpszProxy*/,
nullptr /*lpszProxyBypass*/,
0 /*dwFlags*/);
DWORD http2 = HTTP_PROTOCOL_FLAG_HTTP2;
if (InternetSetOptionA(handle, INTERNET_OPTION_ENABLE_HTTP_PROTOCOL, &http2, sizeof(http2)))
{
return 0;
}
return 1;
}" WININET_HAS_H2)
unset(CMAKE_REQUIRED_LIBRARIES)

endif()
elseif(USE_CRT_HTTP_CLIENT)
file(GLOB CRT_HTTP_HEADERS "include/aws/core/http/crt/*.h")
Expand Down
58 changes: 26 additions & 32 deletions src/aws-cpp-sdk-core/source/http/windows/WinHttpSyncHttpClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,61 +40,54 @@ using namespace Aws::Utils::Logging;
#ifndef WINHTTP_OPTION_HTTP_PROTOCOL_USED
static const DWORD WINHTTP_OPTION_HTTP_PROTOCOL_USED = 134;
#endif
#ifndef WINHTTP_PROTOCOL_FLAG_HTTP2
static const DWORD WINHTTP_PROTOCOL_FLAG_HTTP2 = 0x1;
#endif
#ifndef WINHTTP_PROTOCOL_FLAG_HTTP3
static const DWORD WINHTTP_PROTOCOL_FLAG_HTTP3 = 0x2;
#endif

DWORD ConvertHttpVersionToWinHttpVersion(const Aws::Http::Version version)
std::initializer_list<DWORD> GetWinHttpVersionsToTry(const Aws::Http::Version version)
Copy link
Contributor

@SergeyRyabinin SergeyRyabinin Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that initializer_list works in such context (return by value from a method), it is probably an undefined behavior here.
I'd suggest returning a const reference to a local static const object, such as

const std::initializer_list<DWORD>& GetWinHttpVersionsToTry(const Aws::Http::Version version) {
...
    else if (version == Version::HTTP_VERSION_3)
    {
      static const std::initializer_list<DWORD> HTTP123{ WINHTTP_PROTOCOL_FLAG_HTTP3 | WINHTTP_PROTOCOL_FLAG_HTTP2, WINHTTP_PROTOCOL_FLAG_HTTP2};
      return HTTP123;
    }

or use some other approach.

Please check the cpp reference:

Copying a std::initializer_list does not copy the backing array of the corresponding initializer list.

https://en.cppreference.com/w/cpp/utility/initializer_list

see also: https://stackoverflow.com/questions/15286450/lifetime-of-a-stdinitializer-list-return-value

Copy link
Contributor Author

@teo-tsirpanis teo-tsirpanis Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

{
if (version == Version::HTTP_VERSION_NONE)
{
/* WinHTTP http version None maps to HTTP1.1, however, libCurl None maps to "let http client decide", sticking to libCurl behavior here (use highest) */
#if defined(WINHTTP_HAS_H3)
return WINHTTP_PROTOCOL_FLAG_HTTP3;
#elif defined(WINHTTP_HAS_H2)
return WINHTTP_PROTOCOL_FLAG_HTTP2;
#else
return 0x0;
#endif
return { WINHTTP_PROTOCOL_FLAG_HTTP3 | WINHTTP_PROTOCOL_FLAG_HTTP2, WINHTTP_PROTOCOL_FLAG_HTTP2 };
}
else if (version == Version::HTTP_VERSION_1_0)
{
return 0x0; // HTTP 1.1 can be still used, WinHTTP does not allow disabling 1,1
return {}; // HTTP 1.1 can be still used, WinHTTP does not allow disabling 1,1
}
else if (version == Version::HTTP_VERSION_1_1)
{
return 0x0;
return {};
}
#ifdef WINHTTP_HAS_H2
else if (version == Version::HTTP_VERSION_2_0 ||
version == Version::HTTP_VERSION_2TLS)
{
return WINHTTP_PROTOCOL_FLAG_HTTP2;
return { WINHTTP_PROTOCOL_FLAG_HTTP2 };
}
else if (version == Version::HTTP_VERSION_2_PRIOR_KNOWLEDGE)
{
AWS_LOGSTREAM_WARN("WinHttpHttp2", "Unable to set HTTP/2 with Prior Knowledge on WinHTTP, enabling regular HTTP2");
return WINHTTP_PROTOCOL_FLAG_HTTP2;
return { WINHTTP_PROTOCOL_FLAG_HTTP2 };
}
#endif
#ifdef WINHTTP_HAS_H3
else if (version == Version::HTTP_VERSION_3)
else if (version == Version::HTTP_VERSION_3)
{
return WINHTTP_PROTOCOL_FLAG_HTTP3;
return { WINHTTP_PROTOCOL_FLAG_HTTP3 | WINHTTP_PROTOCOL_FLAG_HTTP2, WINHTTP_PROTOCOL_FLAG_HTTP2 };
}
else if (version == Version::HTTP_VERSION_3ONLY)
{
AWS_LOGSTREAM_WARN("WinHttpHttp2", "Unable to set HTTP3 only on WinHTTP");
return WINHTTP_PROTOCOL_FLAG_HTTP3;
AWS_LOGSTREAM_WARN("WinHttpHttp2", "Unable to set HTTP3 only on WinHTTP");
return { WINHTTP_PROTOCOL_FLAG_HTTP3 };
}
#endif

#ifdef WINHTTP_HAS_H2
AWS_LOGSTREAM_WARN("WinHttpHttp2", "Unable to map requested HTTP Version: (raw enum value) "
<< static_cast<std::underlying_type<Aws::Http::Version>::type>(version) << " defaulting to WINHTTP_PROTOCOL_FLAG_HTTP2");
return WINHTTP_PROTOCOL_FLAG_HTTP2;
#else
AWS_LOGSTREAM_WARN("WinHttpHttp2", "Unable to map requested HTTP Version: (raw enum value) "
<< static_cast<std::underlying_type<Aws::Http::Version>::type>(version) << " defaulting to None (aka 1.1 and below)");
return 0x0;
#endif
<< static_cast<std::underlying_type<Aws::Http::Version>::type>(version) << " defaulting to WINHTTP_PROTOCOL_FLAG_HTTP2");
return {WINHTTP_PROTOCOL_FLAG_HTTP2};
}

void AzWinHttpLogLastError(const char* FuncName)
Expand Down Expand Up @@ -197,14 +190,15 @@ bool AzCallWinHttp(const char* FuncName, WinHttpFunc func, Args &&... args)

static void WinHttpSetHttpVersion(void* handle, const Aws::Http::Version version)
{
DWORD winHttpVersion = ConvertHttpVersionToWinHttpVersion(version);
if (winHttpVersion == 0x0)
{
return;
}
if (!AzCallWinHttp("WinHttpSetOption", WinHttpSetOption, handle, WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL, &winHttpVersion, (DWORD) sizeof(winHttpVersion)))
for (DWORD winHttpVersion : GetWinHttpVersionsToTry(version))
{
AWS_LOGSTREAM_ERROR("WinHttpHttp2", "Failed to set WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL to " << winHttpVersion << " on WinHttp handle: " << handle);
if (AzCallWinHttp("WinHttpSetOption", WinHttpSetOption, handle, WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL, &winHttpVersion, (DWORD)sizeof(winHttpVersion)))
{
break;
}
{
AWS_LOGSTREAM_ERROR("WinHttpHttp2", "Failed to set WINHTTP_OPTION_ENABLE_HTTP_PROTOCOL to " << winHttpVersion << " on WinHttp handle: " << handle);
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/aws-cpp-sdk-core/source/http/windows/WinINetSyncHttpClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ using namespace Aws::Http::Standard;
using namespace Aws::Utils;
using namespace Aws::Utils::Logging;

#ifndef HTTP_PROTOCOL_FLAG_HTTP2
static const DWORD HTTP_PROTOCOL_FLAG_HTTP2 = 0x2;
#endif

static void WinINetEnableHttp2(void* handle)
{
#ifdef WININET_HAS_H2
DWORD http2 = HTTP_PROTOCOL_FLAG_HTTP2;
if (!InternetSetOptionA(handle, INTERNET_OPTION_ENABLE_HTTP_PROTOCOL, &http2, sizeof(http2)))
{
Expand All @@ -39,9 +42,6 @@ static void WinINetEnableHttp2(void* handle)
{
AWS_LOGSTREAM_DEBUG("WinINetHttp2", "HTTP/2 enabled on WinInet handle: " << handle << ".");
}
#else
AWS_UNREFERENCED_PARAM(handle);
#endif
}

WinINetSyncHttpClient::WinINetSyncHttpClient(const ClientConfiguration& config) :
Expand Down
Loading