Skip to content
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.

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

Converted WinHTTP to Async. #4015

Merged
merged 50 commits into from
Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
de0409b
Async WinHTTP
LarryOsterman Oct 10, 2022
8da47ee
Added cancellation test; added support for request cancellation.
LarryOsterman Oct 11, 2022
e48f66f
Added header for requests
LarryOsterman Oct 11, 2022
6bff722
Fixed deadlock when TLS error happens
LarryOsterman Oct 11, 2022
4a76919
FIxed accidentally changed timeout
LarryOsterman Oct 11, 2022
a37968f
Cleaned up some comments and headers
LarryOsterman Oct 11, 2022
5c5c63a
CSpell and fixed UWP build issue
LarryOsterman Oct 11, 2022
1539336
Update sdk/core/azure-core/CMakeLists.txt
LarryOsterman Oct 12, 2022
b85b2b8
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 12, 2022
25b2ff5
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 12, 2022
e1eaa35
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 12, 2022
8e14296
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 12, 2022
7a217e2
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 12, 2022
14e0c86
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 12, 2022
8d41533
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 12, 2022
768bfc6
Improved discoverability of tests; handle exceptions thrown during te…
LarryOsterman Oct 12, 2022
c695b02
Merge branch 'larryo/asyncwinhttp' of https://github.com/LarryOsterma…
LarryOsterman Oct 12, 2022
01d7904
clang-format
LarryOsterman Oct 13, 2022
cba73d3
clang-format
LarryOsterman Oct 13, 2022
2b67738
Merge branch 'main' into larryo/asyncwinhttp
LarryOsterman Oct 13, 2022
3b40e8b
Undid bad merge
LarryOsterman Oct 13, 2022
3f23d74
Use sigabrt handler to report any thrown exceptions
LarryOsterman Oct 13, 2022
9dc8514
clang-format
LarryOsterman Oct 13, 2022
ec7577f
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 17, 2022
015c846
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 17, 2022
684c013
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 17, 2022
9b45291
pull request feedback
LarryOsterman Oct 17, 2022
aea5284
Merge branch 'larryo/asyncwinhttp' of https://github.com/LarryOsterma…
LarryOsterman Oct 17, 2022
12dd753
Pull request feedback
LarryOsterman Oct 17, 2022
57c50fb
More pull request feedback
LarryOsterman Oct 18, 2022
807a3cf
clang-format
LarryOsterman Oct 18, 2022
ae10a3f
Added more control over what events cause requests to complete
LarryOsterman Oct 18, 2022
68530e8
Pull request feedback
LarryOsterman Oct 19, 2022
2084eab
Update sdk/core/azure-core/inc/azure/core/internal/diagnostics/global…
LarryOsterman Oct 19, 2022
1a349d9
clang-format
LarryOsterman Oct 19, 2022
7068ca0
Merge branch 'larryo/asyncwinhttp' of https://github.com/LarryOsterma…
LarryOsterman Oct 19, 2022
e3e4cae
clang-format, again
LarryOsterman Oct 19, 2022
2e270fb
Don't set the complete event until after the mutex is released
LarryOsterman Oct 19, 2022
6c8ac5b
Added a bit of comments
LarryOsterman Oct 20, 2022
d431631
Swap out twiter.com for wikipedia.org
LarryOsterman Oct 20, 2022
d673a49
Cannot use wikipedia on CI pipeline
LarryOsterman Oct 20, 2022
2dfa186
Fixed aws url
LarryOsterman Oct 20, 2022
b013c05
Comment changes from pull request feedback
LarryOsterman Oct 21, 2022
10148f8
Update sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
LarryOsterman Oct 21, 2022
a8573e0
pull request feedback
LarryOsterman Oct 21, 2022
f29c866
Merge branch 'larryo/asyncwinhttp' of https://github.com/LarryOsterma…
LarryOsterman Oct 21, 2022
53af4ee
Bumped test discovery timeout
LarryOsterman Oct 21, 2022
8d00d34
Update sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
LarryOsterman Oct 21, 2022
beac2fb
More pull request feedback
LarryOsterman Oct 21, 2022
1086aae
Merge branch 'larryo/asyncwinhttp' of https://github.com/LarryOsterma…
LarryOsterman Oct 21, 2022
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
1 change: 1 addition & 0 deletions .vscode/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"HKEY",
"HRESULT",
"IMDS",
"immutability",
"Intel",
"itfactor",
"iusg",
Expand Down
5 changes: 3 additions & 2 deletions sdk/core/azure-core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
LarryOsterman marked this conversation as resolved.
Show resolved Hide resolved
)
SET(WIN_TRANSPORT_ADAPTER_INC
SET(WIN_TRANSPORT_ADAPTER_INC
inc/azure/core/http/win_http_transport.hpp
)
)
endif()

set(
Expand Down
117 changes: 11 additions & 106 deletions sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* @file
* @brief #Azure::Core::Http::HttpTransport implementation via WinHTTP.
*/
// cspell:words HCERTIFICATECHAIN PCCERT CCERT HCERTCHAINENGINE HCERTSTORE

#pragma once

Expand All @@ -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)
Expand Down Expand Up @@ -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

/**
Expand Down Expand Up @@ -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};
const Azure::Core::_internal::UniqueHandle<HINTERNET>
m_sessionHandle; // const to ensure immutability.
LarryOsterman marked this conversation as resolved.
Show resolved Hide resolved

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
*
Expand Down Expand Up @@ -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.
LarryOsterman marked this conversation as resolved.
Show resolved Hide resolved
*
* @param context A context to control the request lifetime.
* @param request an HTTP request to be send.
Expand All @@ -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();
LarryOsterman marked this conversation as resolved.
Show resolved Hide resolved
};

} // namespace Http
Expand Down
Loading