From c02e61cb3e09d857f7ad4eacb2801e916545d48f Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Tue, 11 May 2021 23:59:06 -0700 Subject: [PATCH 1/7] SDK: Use boost.beast instead of cpprestsdk --- build/bootstrap/bootstrap-ubuntu-18.04.sh | 18 ++- client-lite/test/string_ops_tests.cpp | 3 +- client-lite/test/test_helpers.h | 63 ---------- common/lib-dotestutil/do_test_helpers.h | 63 ++++++++++ sdk-cpp/CMakeLists.txt | 24 ++-- sdk-cpp/src/do_download.cpp | 2 +- sdk-cpp/src/internal/rest/download_rest.cpp | 48 ++++--- sdk-cpp/src/internal/util/do_http_client.cpp | 124 ++++++++++++++----- sdk-cpp/src/internal/util/do_http_client.h | 26 ++-- sdk-cpp/src/internal/util/do_port_finder.cpp | 2 +- sdk-cpp/src/internal/util/do_url_encode.h | 45 +++++++ sdk-cpp/tests/download_tests.cpp | 45 +++++-- sdk-cpp/tests/port_discovery_tests.cpp | 2 +- sdk-cpp/tests/test_helpers.cpp | 10 ++ sdk-cpp/tests/test_helpers.h | 2 + 15 files changed, 322 insertions(+), 155 deletions(-) create mode 100644 sdk-cpp/src/internal/util/do_url_encode.h diff --git a/build/bootstrap/bootstrap-ubuntu-18.04.sh b/build/bootstrap/bootstrap-ubuntu-18.04.sh index 0c0fcbfe..220c8f3e 100644 --- a/build/bootstrap/bootstrap-ubuntu-18.04.sh +++ b/build/bootstrap/bootstrap-ubuntu-18.04.sh @@ -30,6 +30,22 @@ cmake -G Ninja -DCMAKE_BUILD_TYPE=minsizerel -DCMAKE_POSITION_INDEPENDENT_CODE=O ninja ninja install +# Boost.Beast is available starting only in Boost 1.66 where as Ubuntu 18.04 only has 1.65. +git clone https://github.com/boostorg/beast.git /tmp/boost_beast +pushd /tmp/boost_beast +git checkout tags/boost-1.66.0 +popd + +# Boost.Beast depends on corresponding version of Boost.Asio. +git clone https://github.com/boostorg/asio.git /tmp/boost_asio +pushd /tmp/boost_asio +git checkout tags/boost-1.66.0 +popd + +# Both are header-only libs +cp -r /tmp/boost_beast/include/boost/* "/usr/include/boost/" +cp -r /tmp/boost_asio/include/boost/* "/usr/include/boost/" + # libgtest-dev is a source package and requires manual installation mkdir /tmp/build_gtest/ cd /tmp/build_gtest @@ -53,7 +69,7 @@ else apt-get -y install qemu binfmt-support qemu-user-static # Register qemu with docker to more easily run cross-arch containers - docker run --rm --privileged multiarch/qemu-user-static --reset -p yes + docker run --rm --privileged multiarch/qemu-user-static --reset -p yes fi echo "Finished bootstrapping" diff --git a/client-lite/test/string_ops_tests.cpp b/client-lite/test/string_ops_tests.cpp index 45add727..662cd856 100644 --- a/client-lite/test/string_ops_tests.cpp +++ b/client-lite/test/string_ops_tests.cpp @@ -1,5 +1,6 @@ #include "test_common.h" +#include "do_test_helpers.h" #include "string_ops.h" #include "test_helpers.h" @@ -37,7 +38,7 @@ static void StringPartitionTester(const std::string& left, const std::string& ri size_t numExpectedParts = 0; if (expectedStr1) { ++numExpectedParts; } if (expectedStr2) { ++numExpectedParts; } - auto fnLogStr = test_scope_exit([&]() + auto fnLogStr = dotest::util::scope_exit([&]() { std::cout << "Test string: \"" << testString << "\"\n"; if (parts.size() > 0) std::cout << "Partition1: \"" << parts[0] << "\"\n"; diff --git a/client-lite/test/test_helpers.h b/client-lite/test/test_helpers.h index 1fd64c75..4428da12 100644 --- a/client-lite/test/test_helpers.h +++ b/client-lite/test/test_helpers.h @@ -10,66 +10,3 @@ class TestHelpers // Disallow creating an instance of this object TestHelpers() {} }; - -template -class test_lambda_call -{ -public: - test_lambda_call(const test_lambda_call&) = delete; - test_lambda_call& operator=(const test_lambda_call&) = delete; - test_lambda_call& operator=(test_lambda_call&& other) = delete; - - explicit test_lambda_call(TLambda&& lambda) noexcept : _lambda(std::move(lambda)) - { - static_assert(std::is_same::value, "scope_exit lambdas must not have a return value"); - static_assert(!std::is_lvalue_reference::value && !std::is_rvalue_reference::value, - "scope_exit should only be directly used with a lambda"); - } - - test_lambda_call(test_lambda_call&& other) noexcept : - _lambda(std::move(other._lambda)), - _fCall(other._fCall) - { - other._fCall = false; - } - - ~test_lambda_call() noexcept - { - reset(); - } - - // Ensures the scope_exit lambda will not be called - void release() noexcept - { - _fCall = false; - } - - // Executes the scope_exit lambda immediately if not yet run; ensures it will not run again - void reset() noexcept - { - if (_fCall) - { - _fCall = false; - _lambda(); - } - } - - // Returns true if the scope_exit lambda is still going to be executed - explicit operator bool() const noexcept - { - return _fCall; - } - -protected: - TLambda _lambda; - bool _fCall { true }; -}; - -// Returns an object that executes the given lambda when destroyed. -// Capture the object with 'auto'; use reset() to execute the lambda early or release() to avoid -// execution. Exceptions thrown in the lambda will fail-fast; use scope_exit_log to avoid. -template -inline auto test_scope_exit(TLambda&& lambda) noexcept -{ - return test_lambda_call(std::forward(lambda)); -} diff --git a/common/lib-dotestutil/do_test_helpers.h b/common/lib-dotestutil/do_test_helpers.h index 247a5fce..645f574f 100644 --- a/common/lib-dotestutil/do_test_helpers.h +++ b/common/lib-dotestutil/do_test_helpers.h @@ -65,5 +65,68 @@ class DOTestException : public std::exception const char* what() const noexcept override { return _msg.c_str(); } }; +template +class lambda_call +{ +public: + lambda_call(const lambda_call&) = delete; + lambda_call& operator=(const lambda_call&) = delete; + lambda_call& operator=(lambda_call&& other) = delete; + + explicit lambda_call(TLambda&& lambda) noexcept : + _lambda(std::move(lambda)) + { + static_assert(std::is_same::value, "scope_exit lambdas must not have a return value"); + static_assert(!std::is_lvalue_reference::value && !std::is_rvalue_reference::value, + "scope_exit should only be directly used with a lambda"); + } + + lambda_call(lambda_call&& other) noexcept : + _lambda(std::move(other._lambda)), + _fCall(other._fCall) + { + other._fCall = false; + } + + ~lambda_call() noexcept + { + reset(); + } + + // Ensures the scope_exit lambda will not be called + void release() noexcept + { + _fCall = false; + } + + // Executes the scope_exit lambda immediately if not yet run; ensures it will not run again + void reset() noexcept + { + if (_fCall) + { + _fCall = false; + _lambda(); + } + } + + // Returns true if the scope_exit lambda is still going to be executed + explicit operator bool() const noexcept + { + return _fCall; + } + +protected: + TLambda _lambda; + bool _fCall { true }; +}; + +// Returns an object that executes the given lambda when destroyed. +// Capture the object with 'auto'; use reset() to execute the lambda early or release() to avoid execution. +template +inline auto scope_exit(TLambda&& lambda) noexcept +{ + return lambda_call(std::forward(lambda)); +} + } // namespace util } // namespace dotest diff --git a/sdk-cpp/CMakeLists.txt b/sdk-cpp/CMakeLists.txt index b53833a0..6878850d 100644 --- a/sdk-cpp/CMakeLists.txt +++ b/sdk-cpp/CMakeLists.txt @@ -26,15 +26,17 @@ fixup_compile_options_for_arm() # Include external libraries here find_package(Boost COMPONENTS filesystem system REQUIRED) -# Cpprest Issues: -# 1. v2.10.10 min version (see required PR link below). cpprestsdk does not seem to support specifying this through cmake. -# https://github.com/microsoft/cpprestsdk/pull/1019/files -# -# 2. Installing libcpprest-dev via apt installs cpprest's cmake config files to a non-default cmake search path before v2.10.9 -# See: https://github.com/microsoft/cpprestsdk/issues/686 -# This issue has been patched but has not made its way to the stable branch for Ubuntu -# Since we are statically linking to v2.10.16 we no longer need to worry about the above as cpprest is patched to provide the proper package configuration metadata -find_package(cpprestsdk CONFIG REQUIRED) +find_path( + boostbeast_INCLUDE_DIR + NAMES beast.hpp + PATH_SUFFIXES include/boost +) + +if (boostbeast_INCLUDE_DIR) + message (STATUS "Boost.Beast found at: ${boostbeast_INCLUDE_DIR}") +else() + message (FATAL_ERROR "Boost.Beast NOT FOUND") +endif() if (DO_BUILD_TESTS) add_subdirectory(tests) @@ -63,8 +65,8 @@ if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") "src/internal/rest" "src/internal/util" "src/internal" - ${docs_common_includes} ${include_directories_for_arm} + ${boostbeast_INCLUDE_DIR} ) target_compile_definitions(${DO_SDK_LIB_NAME} PRIVATE DO_PLATFORM_ID=${DO_PLATFORM_ID} @@ -76,7 +78,7 @@ if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") DO_PLUGIN_APT_BIN_NAME="${DO_PLUGIN_APT_BIN_NAME}" ) target_link_libraries(${DO_SDK_LIB_NAME} - PRIVATE doversion cpprestsdk::cpprest + PRIVATE doversion PUBLIC ${Boost_LIBRARIES} ) diff --git a/sdk-cpp/src/do_download.cpp b/sdk-cpp/src/do_download.cpp index 26f65551..b07b7d72 100644 --- a/sdk-cpp/src/do_download.cpp +++ b/sdk-cpp/src/do_download.cpp @@ -107,7 +107,7 @@ void download::download_url_to_path(const std::string& uri, const std::string& d oneShotDownload.start(); download_status status; - bool timedOut; + bool timedOut = false; do { if (isCancelled) diff --git a/sdk-cpp/src/internal/rest/download_rest.cpp b/sdk-cpp/src/internal/rest/download_rest.cpp index a65e8a32..a361c6d5 100644 --- a/sdk-cpp/src/internal/rest/download_rest.cpp +++ b/sdk-cpp/src/internal/rest/download_rest.cpp @@ -1,35 +1,34 @@ - #include "download_rest.h" +#include #include -#include - #include "do_exceptions_internal.h" #include "do_exceptions.h" #include "do_http_client.h" +#include "do_url_encode.h" namespace msdo = microsoft::deliveryoptimization; -namespace cpprest_util_conv = utility::conversions; using namespace std::chrono_literals; // NOLINT(build/namespaces) const int g_maxNumRetryAttempts = 3; +const char* const g_downloadUriPart = "download"; + namespace microsoft::deliveryoptimization::details { + CDownloadRest::CDownloadRest(const std::string& uri, const std::string& downloadFilePath) { - web::uri_builder builder(g_downloadUriPart); - builder.append_path(U("create")); - - builder.append_query(U("Uri"), cpprest_util_conv::to_string_t(uri)); - builder.append_query(U("DownloadFilePath"), cpprest_util_conv::to_string_t(downloadFilePath)); + std::stringstream url; + url << g_downloadUriPart << "/create" << "?Uri=" << Url::EncodeDataString(uri)<< "&DownloadFilePath=" + << Url::EncodeDataString(downloadFilePath); for (int retryAttempts = 0; retryAttempts < g_maxNumRetryAttempts; retryAttempts++) { try { - const auto respBody = CHttpClient::GetInstance().SendRequest(web::http::methods::POST, builder.to_string()); + const auto respBody = CHttpClient::GetInstance().SendRequest(CHttpClient::POST, url.str()); _id = respBody.get("Id"); return; } @@ -78,24 +77,23 @@ void CDownloadRest::Abort() msdo::download_status CDownloadRest::GetStatus() { - web::uri_builder builder(g_downloadUriPart); - builder.append_path(U("getstatus")); - builder.append_query(U("Id"), cpprest_util_conv::to_string_t(_id)); + std::stringstream url; + url << g_downloadUriPart << "/getstatus" << "?Id=" << _id; - const auto respBody = CHttpClient::GetInstance().SendRequest(web::http::methods::GET, builder.to_string()); + const auto respBody = CHttpClient::GetInstance().SendRequest(CHttpClient::GET, url.str()); uint64_t bytesTotal = respBody.get("BytesTotal"); uint64_t bytesTransferred = respBody.get("BytesTransferred"); int32_t errorCode = respBody.get("ErrorCode"); int32_t extendedErrorCode = respBody.get("ExtendedErrorCode"); - static const std::map stateMap = - {{ U("Created"), download_state::created }, - { U("Transferring"), download_state::transferring }, - { U("Transferred"), download_state::transferred }, - { U("Finalized"), download_state::finalized }, - { U("Aborted"), download_state::aborted }, - { U("Paused"), download_state::paused }}; + static const std::map stateMap = + {{ "Created", download_state::created }, + { "Transferring", download_state::transferring }, + { "Transferred", download_state::transferred }, + { "Finalized", download_state::finalized }, + { "Aborted", download_state::aborted }, + { "Paused", download_state::paused }}; download_state status = download_state::created; auto it = stateMap.find(respBody.get("Status")); @@ -114,11 +112,9 @@ msdo::download_status CDownloadRest::GetStatus() void CDownloadRest::_DownloadOperationCall(const std::string& type) { - web::uri_builder builder(g_downloadUriPart); - builder.append_path(cpprest_util_conv::to_string_t(type)); - builder.append_query(U("Id"), cpprest_util_conv::to_string_t(_id)); - - (void)CHttpClient::GetInstance().SendRequest(web::http::methods::POST, builder.to_string()); + std::stringstream url; + url << g_downloadUriPart << '/' << type << "?Id=" << _id; + (void)CHttpClient::GetInstance().SendRequest(CHttpClient::POST, url.str()); } } // namespace microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_http_client.cpp b/sdk-cpp/src/internal/util/do_http_client.cpp index db1ddfa4..1bbf8f63 100644 --- a/sdk-cpp/src/internal/util/do_http_client.cpp +++ b/sdk-cpp/src/internal/util/do_http_client.cpp @@ -1,20 +1,86 @@ - #include "do_http_client.h" +#include +#include +#include +#include +#include +#include #include -#include -#include -#include +#include #include "do_exceptions_internal.h" #include "do_exceptions.h" #include "do_port_finder.h" -const utility::string_t g_downloadUriPart(U("/download")); +namespace beast = boost::beast; // from +namespace http = beast::http; // from +namespace net = boost::asio; // from +using tcp = net::ip::tcp; // from namespace microsoft::deliveryoptimization::details { +class CHttpClientImpl +{ +public: + ~CHttpClientImpl() + { + if (_socket.is_open()) + { + // Gracefully close the socket + boost::system::error_code ec; + _socket.shutdown(tcp::socket::shutdown_both, ec); + } + } + + boost::system::error_code Connect(ushort port) + { + tcp::resolver resolver{_ioc}; + const auto endpoints = resolver.resolve("127.0.0.1", std::to_string(port)); + boost::system::error_code ec; + boost::asio::connect(_socket, endpoints, ec); + return ec; + } + + std::pair GetResponse(http::verb method, const std::string& url) + { + http::request req{method, url, 11}; + req.set(http::field::host, "127.0.0.1"); + req.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING); + // std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging + http::write(_socket, req); + + boost::beast::flat_buffer buffer; + http::response res; + http::read(_socket, buffer, res); + // std::cout << "Received response:\n" << res << std::endl; // uncomment for debugging + + std::stringstream responseBodyStream; + responseBodyStream << res.body(); + + boost::property_tree::ptree responseBodyJson; + if (responseBodyStream.rdbuf()->in_avail() > 0) + { + try + { + boost::property_tree::read_json(responseBodyStream, responseBodyJson); + } + catch (...) + { + } + } + + return {res.result(), responseBodyJson}; + } + +private: + boost::asio::io_context _ioc; + boost::asio::ip::tcp::socket _socket{_ioc}; +}; + +CHttpClient::~CHttpClient() = default; + CHttpClient& CHttpClient::GetInstance() { static CHttpClient myInstance; @@ -23,32 +89,31 @@ CHttpClient& CHttpClient::GetInstance() void CHttpClient::_InitializeDOConnection(bool launchClientFirst) { - std::unique_lock lock(_mutex); - const auto port = CPortFinder::GetDOPort(launchClientFirst); - const auto url = "http://127.0.0.1:" + port + "/"; - _httpClient = std::make_unique(url); -} - -void g_ThrowIfHttpError(const web::http::http_response& resp) -{ - if (resp.status_code() != 200) + const auto port = std::strtoul(CPortFinder::GetDOPort(launchClientFirst).data(), nullptr, 10); + auto spImpl = std::make_unique(); + auto ec = spImpl->Connect(gsl::narrow(port)); + if (ec) { - web::json::object respBody = resp.extract_json().get().as_object(); - - int32_t ErrorCode = respBody[U("ErrorCode")].as_integer(); - - ThrowException(ErrorCode); + // TODO(shishirb) Log the actual error when logging is available + ThrowException(microsoft::deliveryoptimization::errc::no_service); } + + std::unique_lock lock(_mutex); + _httpClientImpl = std::move(spImpl); } -boost::property_tree::ptree CHttpClient::SendRequest(const web::http::method& method, const utility::string_t& url, bool retry) +boost::property_tree::ptree CHttpClient::SendRequest(Method method, const std::string& url, bool retry) { - web::http::http_response response; + auto responseStatusCode = http::status::unknown; + boost::property_tree::ptree responseBodyJson; try { - response = _httpClient->request(method, url).get(); + std::unique_lock lock(_mutex); + auto pClient = static_cast(_httpClientImpl.get()); + std::tie(responseStatusCode, responseBodyJson) = pClient->GetResponse( + method == Method::GET ? http::verb::get : http::verb::post, url); } - catch (const web::http::http_exception& e) + catch (const boost::system::system_error& e) { if (retry) { @@ -56,15 +121,16 @@ boost::property_tree::ptree CHttpClient::SendRequest(const web::http::method& me return SendRequest(method, url, false); } - ThrowException(e.error_code()); + ThrowException(e.code().value()); } - g_ThrowIfHttpError(response); + if (responseStatusCode != http::status::ok) + { + auto agentErrorCode = responseBodyJson.get_optional("ErrorCode"); + ThrowException(agentErrorCode ? *agentErrorCode : -1); + } - std::stringstream ss(response.extract_utf8string().get()); - boost::property_tree::ptree returnValue; - boost::property_tree::read_json(ss, returnValue); - return returnValue; + return responseBodyJson; } CHttpClient::CHttpClient() diff --git a/sdk-cpp/src/internal/util/do_http_client.h b/sdk-cpp/src/internal/util/do_http_client.h index db82ed68..bfb50c6c 100644 --- a/sdk-cpp/src/internal/util/do_http_client.h +++ b/sdk-cpp/src/internal/util/do_http_client.h @@ -1,34 +1,32 @@ #pragma once #include - #include -#include -#include - #include "do_noncopyable.h" -namespace web::http::client -{ -class http_client; -} - -extern const utility::string_t g_downloadUriPart; - namespace microsoft::deliveryoptimization::details { +class CHttpClientImpl; + class CHttpClient : CDONoncopyable { public: - static CHttpClient& GetInstance(); + enum Method + { + GET, + POST + }; - boost::property_tree::ptree SendRequest(const web::http::method& method, const utility::string_t& url, bool retry = true); + ~CHttpClient(); + static CHttpClient& GetInstance(); + boost::property_tree::ptree SendRequest(Method method, const std::string& url, bool retry = true); private: CHttpClient(); void _InitializeDOConnection(bool launchClientFirst = false); mutable std::mutex _mutex; - std::unique_ptr _httpClient; + std::unique_ptr _httpClientImpl; }; + } // namespace microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_port_finder.cpp b/sdk-cpp/src/internal/util/do_port_finder.cpp index b2945f0d..fee9b725 100644 --- a/sdk-cpp/src/internal/util/do_port_finder.cpp +++ b/sdk-cpp/src/internal/util/do_port_finder.cpp @@ -17,7 +17,7 @@ const int32_t g_maxNumPortFileReadAttempts = 3; namespace microsoft::deliveryoptimization::details { -std::string g_DiscoverDOPort() +static std::string g_DiscoverDOPort() { const std::string runtimeDirectory = GetRuntimeDirectory(); if (!boost::filesystem::exists(runtimeDirectory)) diff --git a/sdk-cpp/src/internal/util/do_url_encode.h b/sdk-cpp/src/internal/util/do_url_encode.h new file mode 100644 index 00000000..e2f8d908 --- /dev/null +++ b/sdk-cpp/src/internal/util/do_url_encode.h @@ -0,0 +1,45 @@ +#pragma once + +#include +#include + +namespace microsoft::deliveryoptimization::details +{ + +class Url +{ +public: + // Unreserved characters are those that are allowed in a URI but do not have a reserved purpose + static bool IsUnreserved(int c) + { + return isalnum(static_cast(c)) || c == '-' || c == '.' || c == '_' || c == '~'; + } + + static std::string EncodeDataString(const std::string& input) + { + // Credits: cpprestsdk + // https://github.com/microsoft/cpprestsdk/blob/master/Release/src/uri/uri.cpp + + const char* const hex = "0123456789ABCDEF"; + std::string encoded; + for (auto c : input) + { + // for utf8 encoded string, char ASCII can be greater than 127. + const int ch = static_cast(c); + if (!IsUnreserved(ch)) + { + encoded.push_back('%'); + encoded.push_back(hex[(ch >> 4) & 0xF]); + encoded.push_back(hex[ch & 0xF]); + } + else + { + encoded.push_back(static_cast(ch)); + } + } + return encoded; + } + +}; + +} // namespace microsoft::deliveryoptimization::details diff --git a/sdk-cpp/tests/download_tests.cpp b/sdk-cpp/tests/download_tests.cpp index 0cf12bae..f52973e4 100644 --- a/sdk-cpp/tests/download_tests.cpp +++ b/sdk-cpp/tests/download_tests.cpp @@ -11,6 +11,7 @@ #include "do_download.h" #include "do_download_status.h" #include "do_exceptions.h" +#include "do_test_helpers.h" #include "test_data.h" #include "test_helpers.h" @@ -162,6 +163,10 @@ TEST_F(DownloadTests, SimpleDownloadTest_With404UrlAndMalformedPath) ASSERT_EQ(e.error_code(), DO_ERROR_FROM_XPLAT_SYSERR(ENOENT)); ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); } + catch (const std::exception& se) + { + ASSERT_TRUE(false); + } } TEST_F(DownloadTests, Download1PausedDownload2SameDestTest) @@ -432,7 +437,7 @@ TEST_F(DownloadTests, MultipleConcurrentDownloadTest) TEST_F(DownloadTests, MultipleConcurrentDownloadTest_WithCancels) { std::atomic_bool cancelToken { false }; - ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); + std::thread downloadThread([&]() { try @@ -453,6 +458,7 @@ TEST_F(DownloadTests, MultipleConcurrentDownloadTest_WithCancels) } catch (const msdo::exception& e) { + ASSERT_EQ(e.error_code(), static_cast(std::errc::operation_canceled)); } }); std::thread downloadThread3([&]() @@ -467,9 +473,9 @@ TEST_F(DownloadTests, MultipleConcurrentDownloadTest_WithCancels) } }); + std::this_thread::sleep_for(2s); cancelToken = true; - std::this_thread::sleep_for(3s); downloadThread.join(); downloadThread2.join(); downloadThread3.join(); @@ -481,10 +487,11 @@ TEST_F(DownloadTests, MultipleConcurrentDownloadTest_WithCancels) TEST_F(DownloadTests, SimpleBlockingDownloadTest_ClientNotRunning) { -// Enable this after we can start the service either from sdk or tests. -// Right now all further tests will fail because docs daemon will be stopped. -#if 0 - TestHelpers::ShutdownProcess(g_docsProcName); + TestHelpers::StopService("deliveryoptimization-agent.service"); + auto startService = dotest::util::scope_exit([]() + { + TestHelpers::StartService("deliveryoptimization-agent.service"); + }); TestHelpers::DeleteRestPortFiles(); // can be removed if docs deletes file on shutdown ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); @@ -498,9 +505,33 @@ TEST_F(DownloadTests, SimpleBlockingDownloadTest_ClientNotRunning) ASSERT_EQ(ex.error_code(), static_cast(msdo::errc::no_service)); } ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); -#endif } +TEST_F(DownloadTests, SimpleBlockingDownloadTest_ClientNotRunningPortFilePresent) +{ + // TODO(shishirb) Service name should come from cmake + TestHelpers::StopService("deliveryoptimization-agent.service"); + auto startService = dotest::util::scope_exit([]() + { + TestHelpers::StartService("deliveryoptimization-agent.service"); + }); + TestHelpers::DeleteRestPortFiles(); + TestHelpers::CreateRestPortFiles(1); + + ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); + try + { + msdo::download::download_url_to_path(g_smallFileUrl, g_tmpFileName); + ASSERT_TRUE(!"Expected operation to throw exception"); + } + catch (const msdo::exception& ex) + { + ASSERT_EQ(ex.error_code(), static_cast(msdo::errc::no_service)); + } + ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName)); +} + + TEST_F(DownloadTests, MultipleRestPortFileExists_Download) { // Enable after we have the ability to start the daemon after creating rest port files. diff --git a/sdk-cpp/tests/port_discovery_tests.cpp b/sdk-cpp/tests/port_discovery_tests.cpp index 5306a69e..7cd93226 100644 --- a/sdk-cpp/tests/port_discovery_tests.cpp +++ b/sdk-cpp/tests/port_discovery_tests.cpp @@ -55,4 +55,4 @@ TEST_F(PortDiscoveryTests, DiscoverPortTest) { std::string url = msdod::CPortFinder::GetDOPort(false); ASSERT_EQ(url, samplePortNumber); -} \ No newline at end of file +} diff --git a/sdk-cpp/tests/test_helpers.cpp b/sdk-cpp/tests/test_helpers.cpp index 05f47f22..05d4a5f8 100644 --- a/sdk-cpp/tests/test_helpers.cpp +++ b/sdk-cpp/tests/test_helpers.cpp @@ -50,6 +50,16 @@ void TestHelpers::RestartService(const std::string& name) dtu::ExecuteSystemCommand(restartCmd.data()); } +void TestHelpers::StartService(const std::string& name) +{ + dtu::ExecuteSystemCommand(dtu::FormatString("systemctl start %s", name.c_str()).data()); +} + +void TestHelpers::StopService(const std::string& name) +{ + dtu::ExecuteSystemCommand(dtu::FormatString("systemctl stop %s", name.c_str()).data()); +} + int TestHelpers::_KillProcess(int pid, int signal) { try diff --git a/sdk-cpp/tests/test_helpers.h b/sdk-cpp/tests/test_helpers.h index d4e94646..5aa63de3 100644 --- a/sdk-cpp/tests/test_helpers.h +++ b/sdk-cpp/tests/test_helpers.h @@ -8,6 +8,8 @@ class TestHelpers static bool IsActiveProcess(std::string name); static int ShutdownProcess(std::string name); static void RestartService(const std::string& name); + static void StartService(const std::string& name); + static void StopService(const std::string& name); static void CreateRestPortFiles(int numFiles); static void DeleteRestPortFiles(); static void CleanTestDir(); From 8caa7b17015a3f3e3958bae3e7146b5fec503d36 Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Thu, 13 May 2021 22:42:02 -0700 Subject: [PATCH 2/7] Replace boost.beast with custom implementation --- sdk-cpp/CMakeLists.txt | 13 -- sdk-cpp/src/internal/util/do_http_client.cpp | 194 ++++++++++++++++--- sdk-cpp/tests/download_tests.cpp | 1 + 3 files changed, 172 insertions(+), 36 deletions(-) diff --git a/sdk-cpp/CMakeLists.txt b/sdk-cpp/CMakeLists.txt index 6878850d..f977ee39 100644 --- a/sdk-cpp/CMakeLists.txt +++ b/sdk-cpp/CMakeLists.txt @@ -26,18 +26,6 @@ fixup_compile_options_for_arm() # Include external libraries here find_package(Boost COMPONENTS filesystem system REQUIRED) -find_path( - boostbeast_INCLUDE_DIR - NAMES beast.hpp - PATH_SUFFIXES include/boost -) - -if (boostbeast_INCLUDE_DIR) - message (STATUS "Boost.Beast found at: ${boostbeast_INCLUDE_DIR}") -else() - message (FATAL_ERROR "Boost.Beast NOT FOUND") -endif() - if (DO_BUILD_TESTS) add_subdirectory(tests) endif() @@ -66,7 +54,6 @@ if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") "src/internal/util" "src/internal" ${include_directories_for_arm} - ${boostbeast_INCLUDE_DIR} ) target_compile_definitions(${DO_SDK_LIB_NAME} PRIVATE DO_PLATFORM_ID=${DO_PLATFORM_ID} diff --git a/sdk-cpp/src/internal/util/do_http_client.cpp b/sdk-cpp/src/internal/util/do_http_client.cpp index 1bbf8f63..dff7b1d7 100644 --- a/sdk-cpp/src/internal/util/do_http_client.cpp +++ b/sdk-cpp/src/internal/util/do_http_client.cpp @@ -1,11 +1,11 @@ #include "do_http_client.h" +#include +#include #include #include +#include #include -#include -#include -#include #include #include @@ -13,8 +13,6 @@ #include "do_exceptions.h" #include "do_port_finder.h" -namespace beast = boost::beast; // from -namespace http = beast::http; // from namespace net = boost::asio; // from using tcp = net::ip::tcp; // from @@ -37,27 +35,176 @@ class CHttpClientImpl boost::system::error_code Connect(ushort port) { tcp::resolver resolver{_ioc}; - const auto endpoints = resolver.resolve("127.0.0.1", std::to_string(port)); + const auto endpoints = resolver.resolve({ "127.0.0.1", std::to_string(port) }); boost::system::error_code ec; boost::asio::connect(_socket, endpoints, ec); return ec; } - std::pair GetResponse(http::verb method, const std::string& url) + std::pair GetResponse(const char* method, const std::string& url) { - http::request req{method, url, 11}; - req.set(http::field::host, "127.0.0.1"); - req.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING); - // std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging - http::write(_socket, req); + std::stringstream request; + request << method << ' ' << url << ' ' << "HTTP/1.1\r\n"; + request << "Host: 127.0.0.1\r\n"; + request << "User-Agent: DO-SDK-CPP\r\n"; + request << "\r\n"; - boost::beast::flat_buffer buffer; - http::response res; - http::read(_socket, buffer, res); - // std::cout << "Received response:\n" << res << std::endl; // uncomment for debugging + const auto req = request.str(); + std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging + net::write(_socket, net::buffer(req.data(), req.size())); + // Agent response is a deterministic, fairly small, size. + // Read everything and then parse as HTTP message. + std::vector response; + std::vector readBuf(4 * 1024); + size_t bytesRead = 0; + enum class ParserState + { + StatusLine, + Fields, + Body, + Complete + }; + auto state = ParserState::StatusLine; + size_t skip = 0; + unsigned int statusCode = 0; + size_t contentLength = 0; std::stringstream responseBodyStream; - responseBodyStream << res.body(); + do + { + bytesRead = _socket.read_some(net::buffer(readBuf.data(), readBuf.size())); + if ((response.size() + bytesRead) > 8192) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + response.insert(response.end(), readBuf.data(), readBuf.data() + bytesRead); + + switch (state) + { + case ParserState::StatusLine: + { + // Find \r\n + auto it = std::find(response.begin(), response.end(), '\r'); + if (it != response.end()) + { + if ((it + 1) == response.end()) + { + break; // need more data + } + + if (*(it + 1) != '\n') + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + std::string statusLine{response.begin(), it}; + std::cout << "Status line: " << statusLine << std::endl; + + std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; + std::cmatch matches; + if (!std::regex_match(statusLine.data(), matches, rxStatusLine)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Result: " << statusCode << std::endl; + + state = ParserState::Fields; + skip = std::distance(response.begin(), it + 2); + // fallthrough + } + else + { + break; + } + } + + case ParserState::Fields: + { + bool needMoreData = (response.size() <= skip); + if (!needMoreData) + { + // Find \r\n, look for Content-Length. + // Empty field indicates end of fields. + while (true) + { + auto itStart = response.begin() + skip; + auto itEnd = std::find(itStart, response.end(), '\r'); + if (itEnd != response.end()) + { + if ((itEnd + 1) == response.end()) + { + needMoreData = true; + break; // need more data + } + + if (*(itEnd + 1) != '\n') + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + skip = std::distance(response.begin(), itEnd + 2); + if (itStart == itEnd) + { + state = ParserState::Body; // empty field == end of headers + break; + } + + std::string field{itStart, itEnd}; + std::cout << "Field: " << field << std::endl; + if (field.find("Content-Length") != std::string::npos) + { + std::regex rxContentLength{".*:[ ]*(\\d+).*"}; + std::cmatch matches; + if (!std::regex_match(field.data(), matches, rxContentLength)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Body size: " << contentLength << std::endl; + } + } + else + { + needMoreData = true; + break; // need more data + } + } + } + + if (needMoreData) + { + break; + } + // else fallthrough + assert(state == ParserState::Body); + } + + case ParserState::Body: + { + if (contentLength == 0) + { + state = ParserState::Complete; + break; + } + const auto availableBodySize = gsl::narrow(std::distance(response.begin() + skip, response.end())); + if (availableBodySize == contentLength) + { + responseBodyStream.write(&(*(response.begin() + skip)), contentLength); + std::cout << "Body: " << responseBodyStream.str() << std::endl; + state = ParserState::Complete; + } + // else need more data + break; + } + + case ParserState::Complete: + break; + } + + } while (state != ParserState::Complete); boost::property_tree::ptree responseBodyJson; if (responseBodyStream.rdbuf()->in_avail() > 0) @@ -71,12 +218,13 @@ class CHttpClientImpl } } - return {res.result(), responseBodyJson}; + return {statusCode, responseBodyJson}; + // return {404, responseBodyJson}; } private: - boost::asio::io_context _ioc; - boost::asio::ip::tcp::socket _socket{_ioc}; + net::io_service _ioc; + net::ip::tcp::socket _socket{_ioc}; }; CHttpClient::~CHttpClient() = default; @@ -104,14 +252,14 @@ void CHttpClient::_InitializeDOConnection(bool launchClientFirst) boost::property_tree::ptree CHttpClient::SendRequest(Method method, const std::string& url, bool retry) { - auto responseStatusCode = http::status::unknown; + auto responseStatusCode = 0u; boost::property_tree::ptree responseBodyJson; try { std::unique_lock lock(_mutex); auto pClient = static_cast(_httpClientImpl.get()); std::tie(responseStatusCode, responseBodyJson) = pClient->GetResponse( - method == Method::GET ? http::verb::get : http::verb::post, url); + method == Method::GET ? "GET" : "POST", url); } catch (const boost::system::system_error& e) { @@ -124,7 +272,7 @@ boost::property_tree::ptree CHttpClient::SendRequest(Method method, const std::s ThrowException(e.code().value()); } - if (responseStatusCode != http::status::ok) + if (responseStatusCode != 200) { auto agentErrorCode = responseBodyJson.get_optional("ErrorCode"); ThrowException(agentErrorCode ? *agentErrorCode : -1); diff --git a/sdk-cpp/tests/download_tests.cpp b/sdk-cpp/tests/download_tests.cpp index f52973e4..51e38bd7 100644 --- a/sdk-cpp/tests/download_tests.cpp +++ b/sdk-cpp/tests/download_tests.cpp @@ -165,6 +165,7 @@ TEST_F(DownloadTests, SimpleDownloadTest_With404UrlAndMalformedPath) } catch (const std::exception& se) { + std::cout << "Unexpected exception: " << se.what() << std::endl; ASSERT_TRUE(false); } } From 846b1edf9658c249ac41ee8c4e9eb5369c895dca Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Fri, 14 May 2021 00:18:00 -0700 Subject: [PATCH 3/7] Refactor: new file do_http_message --- sdk-cpp/src/internal/rest/download_rest.cpp | 6 +- sdk-cpp/src/internal/util/do_http_client.cpp | 194 +---------------- sdk-cpp/src/internal/util/do_http_client.h | 9 +- sdk-cpp/src/internal/util/do_http_message.cpp | 198 ++++++++++++++++++ sdk-cpp/src/internal/util/do_http_message.h | 56 +++++ 5 files changed, 269 insertions(+), 194 deletions(-) create mode 100644 sdk-cpp/src/internal/util/do_http_message.cpp create mode 100644 sdk-cpp/src/internal/util/do_http_message.h diff --git a/sdk-cpp/src/internal/rest/download_rest.cpp b/sdk-cpp/src/internal/rest/download_rest.cpp index a361c6d5..7e3ec93c 100644 --- a/sdk-cpp/src/internal/rest/download_rest.cpp +++ b/sdk-cpp/src/internal/rest/download_rest.cpp @@ -28,7 +28,7 @@ CDownloadRest::CDownloadRest(const std::string& uri, const std::string& download { try { - const auto respBody = CHttpClient::GetInstance().SendRequest(CHttpClient::POST, url.str()); + const auto respBody = CHttpClient::GetInstance().SendRequest(HttpRequest::POST, url.str()); _id = respBody.get("Id"); return; } @@ -80,7 +80,7 @@ msdo::download_status CDownloadRest::GetStatus() std::stringstream url; url << g_downloadUriPart << "/getstatus" << "?Id=" << _id; - const auto respBody = CHttpClient::GetInstance().SendRequest(CHttpClient::GET, url.str()); + const auto respBody = CHttpClient::GetInstance().SendRequest(HttpRequest::GET, url.str()); uint64_t bytesTotal = respBody.get("BytesTotal"); uint64_t bytesTransferred = respBody.get("BytesTransferred"); @@ -114,7 +114,7 @@ void CDownloadRest::_DownloadOperationCall(const std::string& type) { std::stringstream url; url << g_downloadUriPart << '/' << type << "?Id=" << _id; - (void)CHttpClient::GetInstance().SendRequest(CHttpClient::POST, url.str()); + (void)CHttpClient::GetInstance().SendRequest(HttpRequest::POST, url.str()); } } // namespace microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_http_client.cpp b/sdk-cpp/src/internal/util/do_http_client.cpp index dff7b1d7..0fce5aea 100644 --- a/sdk-cpp/src/internal/util/do_http_client.cpp +++ b/sdk-cpp/src/internal/util/do_http_client.cpp @@ -1,16 +1,13 @@ #include "do_http_client.h" -#include -#include #include #include -#include #include -#include #include -#include "do_exceptions_internal.h" #include "do_exceptions.h" +#include "do_exceptions_internal.h" +#include "do_http_message.h" #include "do_port_finder.h" namespace net = boost::asio; // from @@ -41,185 +38,15 @@ class CHttpClientImpl return ec; } - std::pair GetResponse(const char* method, const std::string& url) + std::pair GetResponse(HttpRequest::Method method, const std::string& url) { - std::stringstream request; - request << method << ' ' << url << ' ' << "HTTP/1.1\r\n"; - request << "Host: 127.0.0.1\r\n"; - request << "User-Agent: DO-SDK-CPP\r\n"; - request << "\r\n"; - - const auto req = request.str(); - std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging - net::write(_socket, net::buffer(req.data(), req.size())); - - // Agent response is a deterministic, fairly small, size. - // Read everything and then parse as HTTP message. - std::vector response; - std::vector readBuf(4 * 1024); - size_t bytesRead = 0; - enum class ParserState - { - StatusLine, - Fields, - Body, - Complete - }; - auto state = ParserState::StatusLine; - size_t skip = 0; - unsigned int statusCode = 0; - size_t contentLength = 0; - std::stringstream responseBodyStream; - do - { - bytesRead = _socket.read_some(net::buffer(readBuf.data(), readBuf.size())); - if ((response.size() + bytesRead) > 8192) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - response.insert(response.end(), readBuf.data(), readBuf.data() + bytesRead); - - switch (state) - { - case ParserState::StatusLine: - { - // Find \r\n - auto it = std::find(response.begin(), response.end(), '\r'); - if (it != response.end()) - { - if ((it + 1) == response.end()) - { - break; // need more data - } - - if (*(it + 1) != '\n') - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - std::string statusLine{response.begin(), it}; - std::cout << "Status line: " << statusLine << std::endl; - - std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; - std::cmatch matches; - if (!std::regex_match(statusLine.data(), matches, rxStatusLine)) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Result: " << statusCode << std::endl; - - state = ParserState::Fields; - skip = std::distance(response.begin(), it + 2); - // fallthrough - } - else - { - break; - } - } + HttpRequest request{method, url}; + request.Serialize(_socket); - case ParserState::Fields: - { - bool needMoreData = (response.size() <= skip); - if (!needMoreData) - { - // Find \r\n, look for Content-Length. - // Empty field indicates end of fields. - while (true) - { - auto itStart = response.begin() + skip; - auto itEnd = std::find(itStart, response.end(), '\r'); - if (itEnd != response.end()) - { - if ((itEnd + 1) == response.end()) - { - needMoreData = true; - break; // need more data - } - - if (*(itEnd + 1) != '\n') - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - skip = std::distance(response.begin(), itEnd + 2); - if (itStart == itEnd) - { - state = ParserState::Body; // empty field == end of headers - break; - } - - std::string field{itStart, itEnd}; - std::cout << "Field: " << field << std::endl; - if (field.find("Content-Length") != std::string::npos) - { - std::regex rxContentLength{".*:[ ]*(\\d+).*"}; - std::cmatch matches; - if (!std::regex_match(field.data(), matches, rxContentLength)) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Body size: " << contentLength << std::endl; - } - } - else - { - needMoreData = true; - break; // need more data - } - } - } - - if (needMoreData) - { - break; - } - // else fallthrough - assert(state == ParserState::Body); - } - - case ParserState::Body: - { - if (contentLength == 0) - { - state = ParserState::Complete; - break; - } - const auto availableBodySize = gsl::narrow(std::distance(response.begin() + skip, response.end())); - if (availableBodySize == contentLength) - { - responseBodyStream.write(&(*(response.begin() + skip)), contentLength); - std::cout << "Body: " << responseBodyStream.str() << std::endl; - state = ParserState::Complete; - } - // else need more data - break; - } - - case ParserState::Complete: - break; - } - - } while (state != ParserState::Complete); - - boost::property_tree::ptree responseBodyJson; - if (responseBodyStream.rdbuf()->in_avail() > 0) - { - try - { - boost::property_tree::read_json(responseBodyStream, responseBodyJson); - } - catch (...) - { - } - } + HttpResponse response; + response.Deserialize(_socket); - return {statusCode, responseBodyJson}; - // return {404, responseBodyJson}; + return {response.StatusCode(), response.ExtractJsonBody()}; } private: @@ -250,7 +77,7 @@ void CHttpClient::_InitializeDOConnection(bool launchClientFirst) _httpClientImpl = std::move(spImpl); } -boost::property_tree::ptree CHttpClient::SendRequest(Method method, const std::string& url, bool retry) +boost::property_tree::ptree CHttpClient::SendRequest(HttpRequest::Method method, const std::string& url, bool retry) { auto responseStatusCode = 0u; boost::property_tree::ptree responseBodyJson; @@ -258,8 +85,7 @@ boost::property_tree::ptree CHttpClient::SendRequest(Method method, const std::s { std::unique_lock lock(_mutex); auto pClient = static_cast(_httpClientImpl.get()); - std::tie(responseStatusCode, responseBodyJson) = pClient->GetResponse( - method == Method::GET ? "GET" : "POST", url); + std::tie(responseStatusCode, responseBodyJson) = pClient->GetResponse(method, url); } catch (const boost::system::system_error& e) { diff --git a/sdk-cpp/src/internal/util/do_http_client.h b/sdk-cpp/src/internal/util/do_http_client.h index bfb50c6c..ac7d2384 100644 --- a/sdk-cpp/src/internal/util/do_http_client.h +++ b/sdk-cpp/src/internal/util/do_http_client.h @@ -2,6 +2,7 @@ #include #include +#include "do_http_message.h" #include "do_noncopyable.h" namespace microsoft::deliveryoptimization::details @@ -11,15 +12,9 @@ class CHttpClientImpl; class CHttpClient : CDONoncopyable { public: - enum Method - { - GET, - POST - }; - ~CHttpClient(); static CHttpClient& GetInstance(); - boost::property_tree::ptree SendRequest(Method method, const std::string& url, bool retry = true); + boost::property_tree::ptree SendRequest(HttpRequest::Method method, const std::string& url, bool retry = true); private: CHttpClient(); diff --git a/sdk-cpp/src/internal/util/do_http_message.cpp b/sdk-cpp/src/internal/util/do_http_message.cpp new file mode 100644 index 00000000..03c04f4a --- /dev/null +++ b/sdk-cpp/src/internal/util/do_http_message.cpp @@ -0,0 +1,198 @@ +#include "do_http_message.h" + +#include +#include +#include +#include +#include +#include "do_exceptions.h" +#include "do_exceptions_internal.h" + +namespace net = boost::asio; // from + +namespace microsoft::deliveryoptimization::details +{ + +HttpRequest::HttpRequest(Method method, const std::string& url) : + _method(method), + _url(url.data()) +{ +} + +void HttpRequest::Serialize(boost::asio::ip::tcp::socket& socket) const +{ + std::stringstream request; + const char* pVerb = (_method == Method::GET) ? "GET" : "POST"; + request << pVerb << ' ' << _url << ' ' << "HTTP/1.1\r\n"; + request << "Host: 127.0.0.1\r\n"; + request << "User-Agent: DO-SDK-CPP\r\n"; + request << "\r\n"; + + const auto req = request.str(); + std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging + net::write(socket, net::buffer(req.data(), req.size())); +} + +HttpResponse::HttpResponse() +{ + _responseBuf.reserve(2048); +} + +void HttpResponse::Deserialize(boost::asio::ip::tcp::socket& socket) +{ + // Agent response is always with a JSON body, couple hundred bytes at max. + std::vector readBuf(1024); + do + { + auto bytesRead = socket.read_some(net::buffer(readBuf.data(), readBuf.size())); + if ((_responseBuf.size() + bytesRead) > _responseBuf.capacity()) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + _responseBuf.insert(_responseBuf.end(), readBuf.data(), readBuf.data() + bytesRead); + + switch (_state) + { + case ParserState::StatusLine: + { + // Find \r\n + auto it = std::find(_responseBuf.begin(), _responseBuf.end(), '\r'); + if (it != _responseBuf.end()) + { + if ((it + 1) == _responseBuf.end()) + { + break; // need more data + } + + if (*(it + 1) != '\n') + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + std::string statusLine{_responseBuf.begin(), it}; + std::cout << "Status line: " << statusLine << std::endl; + + std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; + std::cmatch matches; + if (!std::regex_match(statusLine.data(), matches, rxStatusLine)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + _statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Result: " << _statusCode << std::endl; + + _state = ParserState::Fields; + _skip = std::distance(_responseBuf.begin(), it + 2); + // fallthrough + } + else + { + break; + } + } + + case ParserState::Fields: + { + bool needMoreData = (_responseBuf.size() <= _skip); + if (!needMoreData) + { + // Find \r\n, look for Content-Length. + // Empty field indicates end of fields. + while (true) + { + auto itStart = _responseBuf.begin() + _skip; + auto itEnd = std::find(itStart, _responseBuf.end(), '\r'); + if (itEnd != _responseBuf.end()) + { + if ((itEnd + 1) == _responseBuf.end()) + { + needMoreData = true; + break; // need more data + } + + if (*(itEnd + 1) != '\n') + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + _skip = std::distance(_responseBuf.begin(), itEnd + 2); + if (itStart == itEnd) + { + _state = ParserState::Body; // empty field == end of headers + break; + } + + std::string field{itStart, itEnd}; + std::cout << "Field: " << field << std::endl; + if (field.find("Content-Length") != std::string::npos) + { + std::regex rxContentLength{".*:[ ]*(\\d+).*"}; + std::cmatch matches; + if (!std::regex_match(field.data(), matches, rxContentLength)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + _contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Body size: " << _contentLength << std::endl; + } + } + else + { + needMoreData = true; + break; // need more data + } + } + } + + if (needMoreData) + { + break; + } + // else fallthrough + assert(_state == ParserState::Body); + } + + case ParserState::Body: + { + if (_contentLength == 0) + { + _state = ParserState::Complete; + break; + } + const auto availableBodySize = gsl::narrow(std::distance(_responseBuf.begin() + _skip, _responseBuf.end())); + if (availableBodySize == _contentLength) + { + _body.write(&(*(_responseBuf.begin() + _skip)), _contentLength); + std::cout << "Body: " << _body.str() << std::endl; + _state = ParserState::Complete; + } + // else need more data + break; + } + + case ParserState::Complete: + break; + } + + } while (_state != ParserState::Complete); +} + +boost::property_tree::ptree HttpResponse::ExtractJsonBody() +{ + boost::property_tree::ptree responseBodyJson; + if (_body.rdbuf()->in_avail() > 0) + { + try + { + boost::property_tree::read_json(_body, responseBodyJson); + } + catch (...) + { + } + } + return responseBodyJson; +} + +} // microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_http_message.h b/sdk-cpp/src/internal/util/do_http_message.h new file mode 100644 index 00000000..b2edd8ac --- /dev/null +++ b/sdk-cpp/src/internal/util/do_http_message.h @@ -0,0 +1,56 @@ +#pragma once + +#include +#include +#include +#include + +namespace microsoft::deliveryoptimization::details +{ + +class HttpRequest +{ +public: + enum Method + { + GET, + POST + }; + + HttpRequest(Method method, const std::string& url); + void Serialize(boost::asio::ip::tcp::socket& socket) const; + +private: + Method _method; + const char* _url; +}; + +class HttpResponse +{ +public: + HttpResponse(); + void Deserialize(boost::asio::ip::tcp::socket& socket); + + unsigned int StatusCode() const { return _statusCode; } + boost::property_tree::ptree ExtractJsonBody(); + +private: + unsigned int _statusCode { 0 }; + size_t _contentLength { 0 }; + std::stringstream _body; + + // Parsing info + enum class ParserState + { + StatusLine, + Fields, + Body, + Complete + }; + + ParserState _state { ParserState::StatusLine }; + std::vector _responseBuf; + size_t _skip { 0 }; +}; + +} // microsoft::deliveryoptimization::details From 400099c8f3c5121f095d5467e191ad8ed781e751 Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Fri, 14 May 2021 01:56:24 -0700 Subject: [PATCH 4/7] Refactor: Move parser to its own file --- sdk-cpp/src/internal/util/do_http_message.cpp | 146 +---------------- sdk-cpp/src/internal/util/do_http_message.h | 14 -- sdk-cpp/src/internal/util/do_http_parser.cpp | 150 ++++++++++++++++++ sdk-cpp/src/internal/util/do_http_parser.h | 45 ++++++ 4 files changed, 199 insertions(+), 156 deletions(-) create mode 100644 sdk-cpp/src/internal/util/do_http_parser.cpp create mode 100644 sdk-cpp/src/internal/util/do_http_parser.h diff --git a/sdk-cpp/src/internal/util/do_http_message.cpp b/sdk-cpp/src/internal/util/do_http_message.cpp index 03c04f4a..fb486401 100644 --- a/sdk-cpp/src/internal/util/do_http_message.cpp +++ b/sdk-cpp/src/internal/util/do_http_message.cpp @@ -1,12 +1,9 @@ #include "do_http_message.h" #include -#include #include #include -#include -#include "do_exceptions.h" -#include "do_exceptions_internal.h" +#include "do_http_parser.h" namespace net = boost::asio; // from @@ -33,150 +30,15 @@ void HttpRequest::Serialize(boost::asio::ip::tcp::socket& socket) const net::write(socket, net::buffer(req.data(), req.size())); } -HttpResponse::HttpResponse() -{ - _responseBuf.reserve(2048); -} - void HttpResponse::Deserialize(boost::asio::ip::tcp::socket& socket) { - // Agent response is always with a JSON body, couple hundred bytes at max. + HttpResponseParser parser{_statusCode, _contentLength, _body}; std::vector readBuf(1024); do { auto bytesRead = socket.read_some(net::buffer(readBuf.data(), readBuf.size())); - if ((_responseBuf.size() + bytesRead) > _responseBuf.capacity()) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - _responseBuf.insert(_responseBuf.end(), readBuf.data(), readBuf.data() + bytesRead); - - switch (_state) - { - case ParserState::StatusLine: - { - // Find \r\n - auto it = std::find(_responseBuf.begin(), _responseBuf.end(), '\r'); - if (it != _responseBuf.end()) - { - if ((it + 1) == _responseBuf.end()) - { - break; // need more data - } - - if (*(it + 1) != '\n') - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - std::string statusLine{_responseBuf.begin(), it}; - std::cout << "Status line: " << statusLine << std::endl; - - std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; - std::cmatch matches; - if (!std::regex_match(statusLine.data(), matches, rxStatusLine)) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - _statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Result: " << _statusCode << std::endl; - - _state = ParserState::Fields; - _skip = std::distance(_responseBuf.begin(), it + 2); - // fallthrough - } - else - { - break; - } - } - - case ParserState::Fields: - { - bool needMoreData = (_responseBuf.size() <= _skip); - if (!needMoreData) - { - // Find \r\n, look for Content-Length. - // Empty field indicates end of fields. - while (true) - { - auto itStart = _responseBuf.begin() + _skip; - auto itEnd = std::find(itStart, _responseBuf.end(), '\r'); - if (itEnd != _responseBuf.end()) - { - if ((itEnd + 1) == _responseBuf.end()) - { - needMoreData = true; - break; // need more data - } - - if (*(itEnd + 1) != '\n') - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - _skip = std::distance(_responseBuf.begin(), itEnd + 2); - if (itStart == itEnd) - { - _state = ParserState::Body; // empty field == end of headers - break; - } - - std::string field{itStart, itEnd}; - std::cout << "Field: " << field << std::endl; - if (field.find("Content-Length") != std::string::npos) - { - std::regex rxContentLength{".*:[ ]*(\\d+).*"}; - std::cmatch matches; - if (!std::regex_match(field.data(), matches, rxContentLength)) - { - ThrowException(microsoft::deliveryoptimization::errc::unexpected); - } - - _contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Body size: " << _contentLength << std::endl; - } - } - else - { - needMoreData = true; - break; // need more data - } - } - } - - if (needMoreData) - { - break; - } - // else fallthrough - assert(_state == ParserState::Body); - } - - case ParserState::Body: - { - if (_contentLength == 0) - { - _state = ParserState::Complete; - break; - } - const auto availableBodySize = gsl::narrow(std::distance(_responseBuf.begin() + _skip, _responseBuf.end())); - if (availableBodySize == _contentLength) - { - _body.write(&(*(_responseBuf.begin() + _skip)), _contentLength); - std::cout << "Body: " << _body.str() << std::endl; - _state = ParserState::Complete; - } - // else need more data - break; - } - - case ParserState::Complete: - break; - } - - } while (_state != ParserState::Complete); + parser.OnData(readBuf.data(), bytesRead); + } while (!parser.Done()); } boost::property_tree::ptree HttpResponse::ExtractJsonBody() diff --git a/sdk-cpp/src/internal/util/do_http_message.h b/sdk-cpp/src/internal/util/do_http_message.h index b2edd8ac..0be45734 100644 --- a/sdk-cpp/src/internal/util/do_http_message.h +++ b/sdk-cpp/src/internal/util/do_http_message.h @@ -28,7 +28,6 @@ class HttpRequest class HttpResponse { public: - HttpResponse(); void Deserialize(boost::asio::ip::tcp::socket& socket); unsigned int StatusCode() const { return _statusCode; } @@ -38,19 +37,6 @@ class HttpResponse unsigned int _statusCode { 0 }; size_t _contentLength { 0 }; std::stringstream _body; - - // Parsing info - enum class ParserState - { - StatusLine, - Fields, - Body, - Complete - }; - - ParserState _state { ParserState::StatusLine }; - std::vector _responseBuf; - size_t _skip { 0 }; }; } // microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_http_parser.cpp b/sdk-cpp/src/internal/util/do_http_parser.cpp new file mode 100644 index 00000000..05b612fa --- /dev/null +++ b/sdk-cpp/src/internal/util/do_http_parser.cpp @@ -0,0 +1,150 @@ +#include "do_http_parser.h" + +#include +#include +#include +#include "do_exceptions.h" +#include "do_exceptions_internal.h" + +namespace microsoft::deliveryoptimization::details +{ + +HttpResponseParser::HttpResponseParser(unsigned int& statusCodeBuf, size_t& contentLenBuf, std::stringstream& bodyBuf) : + _statusCode(statusCodeBuf), + _contentLength(contentLenBuf), + _body(bodyBuf) +{ + // Agent response is always with a JSON body, small in size. + _responseBuf.reserve(2048); +} + +void HttpResponseParser::OnData(const char* pData, size_t cb) +{ + if ((_responseBuf.size() + cb) > _responseBuf.capacity()) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + _responseBuf.insert(_responseBuf.end(), pData, pData + cb); + + while (_ParseBuf()) + { + } +} + +// Returns true if more processing can be done, false if processing is done or cannot continue until more data is received +bool HttpResponseParser::_ParseBuf() +{ + const auto oldState = _state; + switch (_state) + { + case ParserState::StatusLine: + { + auto itCR = _FindCRLF(_responseBuf.begin()); + if (itCR != _responseBuf.end()) + { + std::string statusLine{_responseBuf.begin(), itCR}; + std::cout << "Status line: " << statusLine << std::endl; + + std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; + std::cmatch matches; + if (!std::regex_match(statusLine.data(), matches, rxStatusLine)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + _statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Result: " << _statusCode << std::endl; + + _state = ParserState::Fields; + _itParseFrom = itCR + 2; + } + break; + } + + case ParserState::Fields: + { + while (_ParseNextField()) + { + } + break; + } + + case ParserState::Body: + { + if (_contentLength == 0) + { + _state = ParserState::Complete; + } + else + { + const auto availableBodySize = gsl::narrow(std::distance(_itParseFrom, _responseBuf.end())); + // Agent response is a JSON body, couple hundred bytes at max. Read everything at once. + if (availableBodySize == _contentLength) + { + _body.write(&(*_itParseFrom), _contentLength); + std::cout << "Body: " << _body.str() << std::endl; + _state = ParserState::Complete; + _itParseFrom = _responseBuf.end(); + } + } + break; + } + + case ParserState::Complete: + break; + } + + return ((oldState != _state) && !Done()); +} + +// Returns true if there are more fields to process, false otherwise +bool HttpResponseParser::_ParseNextField() +{ + // Find \r\n, look for Content-Length. + auto itCR = _FindCRLF(_itParseFrom); + if (itCR == _responseBuf.end()) + { + return false; // need more data + } + + if (_itParseFrom == itCR) + { + _state = ParserState::Body; // empty field == end of headers + _itParseFrom = itCR + 2; + return false; + } + + std::string field{_itParseFrom, itCR}; + std::cout << "Field: " << field << std::endl; + if (field.find("Content-Length") != std::string::npos) + { + std::regex rxContentLength{".*:[ ]*(\\d+).*"}; + std::cmatch matches; + if (!std::regex_match(field.data(), matches, rxContentLength)) + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + + _contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); + std::cout << "Body size: " << _contentLength << std::endl; + } + // else, field not interesting + _itParseFrom = itCR + 2; + return true; +} + +std::vector::iterator HttpResponseParser::_FindCRLF(std::vector::iterator itStart) +{ + auto itCR = std::find(itStart, _responseBuf.end(), '\r'); + if (itCR == _responseBuf.end() || (itCR + 1) == _responseBuf.end()) + { + return _responseBuf.end(); // need more data + } + if (*(itCR + 1) != '\n') + { + ThrowException(microsoft::deliveryoptimization::errc::unexpected); + } + return itCR; +} + +} // microsoft::deliveryoptimization::details diff --git a/sdk-cpp/src/internal/util/do_http_parser.h b/sdk-cpp/src/internal/util/do_http_parser.h new file mode 100644 index 00000000..ba847a16 --- /dev/null +++ b/sdk-cpp/src/internal/util/do_http_parser.h @@ -0,0 +1,45 @@ +#pragma once + +#include +#include +#include + +namespace microsoft::deliveryoptimization::details +{ + +class HttpResponseParser +{ +public: + HttpResponseParser(unsigned int& statusCodeBuf, size_t& contentLenBuf, std::stringstream& bodyBuf); + + void OnData(const char* pData, size_t cb); + + bool Done() const noexcept + { + return (_state == ParserState::Complete); + } + +private: + bool _ParseBuf(); + bool _ParseNextField(); + std::vector::iterator _FindCRLF(std::vector::iterator itStart); + + // Parsing info + enum class ParserState + { + StatusLine, + Fields, + Body, + Complete + }; + + ParserState _state { ParserState::StatusLine }; + std::vector _responseBuf; + std::vector::iterator _itParseFrom; + + unsigned int& _statusCode; + size_t& _contentLength; + std::stringstream& _body; +}; + +} // microsoft::deliveryoptimization::details From f08bc0ebcc295d2e07e6cd93be8b2f7c340a1a48 Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Fri, 14 May 2021 02:00:30 -0700 Subject: [PATCH 5/7] Refactor: Parser need not check Done() --- sdk-cpp/src/internal/util/do_http_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk-cpp/src/internal/util/do_http_parser.cpp b/sdk-cpp/src/internal/util/do_http_parser.cpp index 05b612fa..f8018cbf 100644 --- a/sdk-cpp/src/internal/util/do_http_parser.cpp +++ b/sdk-cpp/src/internal/util/do_http_parser.cpp @@ -94,7 +94,7 @@ bool HttpResponseParser::_ParseBuf() break; } - return ((oldState != _state) && !Done()); + return (oldState != _state); } // Returns true if there are more fields to process, false otherwise From 56a5aa83edc8aab51679435f39031b596c7f0f28 Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Fri, 14 May 2021 02:01:50 -0700 Subject: [PATCH 6/7] Comment out debug std::out, remove bootstrap changes --- build/bootstrap/bootstrap-ubuntu-18.04.sh | 18 +----------------- sdk-cpp/src/internal/util/do_http_message.cpp | 2 +- sdk-cpp/src/internal/util/do_http_parser.cpp | 12 ++++++------ 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/build/bootstrap/bootstrap-ubuntu-18.04.sh b/build/bootstrap/bootstrap-ubuntu-18.04.sh index 220c8f3e..0c0fcbfe 100644 --- a/build/bootstrap/bootstrap-ubuntu-18.04.sh +++ b/build/bootstrap/bootstrap-ubuntu-18.04.sh @@ -30,22 +30,6 @@ cmake -G Ninja -DCMAKE_BUILD_TYPE=minsizerel -DCMAKE_POSITION_INDEPENDENT_CODE=O ninja ninja install -# Boost.Beast is available starting only in Boost 1.66 where as Ubuntu 18.04 only has 1.65. -git clone https://github.com/boostorg/beast.git /tmp/boost_beast -pushd /tmp/boost_beast -git checkout tags/boost-1.66.0 -popd - -# Boost.Beast depends on corresponding version of Boost.Asio. -git clone https://github.com/boostorg/asio.git /tmp/boost_asio -pushd /tmp/boost_asio -git checkout tags/boost-1.66.0 -popd - -# Both are header-only libs -cp -r /tmp/boost_beast/include/boost/* "/usr/include/boost/" -cp -r /tmp/boost_asio/include/boost/* "/usr/include/boost/" - # libgtest-dev is a source package and requires manual installation mkdir /tmp/build_gtest/ cd /tmp/build_gtest @@ -69,7 +53,7 @@ else apt-get -y install qemu binfmt-support qemu-user-static # Register qemu with docker to more easily run cross-arch containers - docker run --rm --privileged multiarch/qemu-user-static --reset -p yes + docker run --rm --privileged multiarch/qemu-user-static --reset -p yes fi echo "Finished bootstrapping" diff --git a/sdk-cpp/src/internal/util/do_http_message.cpp b/sdk-cpp/src/internal/util/do_http_message.cpp index fb486401..9e596348 100644 --- a/sdk-cpp/src/internal/util/do_http_message.cpp +++ b/sdk-cpp/src/internal/util/do_http_message.cpp @@ -26,7 +26,7 @@ void HttpRequest::Serialize(boost::asio::ip::tcp::socket& socket) const request << "\r\n"; const auto req = request.str(); - std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging + // std::cout << "Sending request:\n" << req << std::endl; // uncomment for debugging net::write(socket, net::buffer(req.data(), req.size())); } diff --git a/sdk-cpp/src/internal/util/do_http_parser.cpp b/sdk-cpp/src/internal/util/do_http_parser.cpp index f8018cbf..e3eb73ce 100644 --- a/sdk-cpp/src/internal/util/do_http_parser.cpp +++ b/sdk-cpp/src/internal/util/do_http_parser.cpp @@ -1,6 +1,6 @@ #include "do_http_parser.h" -#include +// #include #include #include #include "do_exceptions.h" @@ -43,7 +43,7 @@ bool HttpResponseParser::_ParseBuf() if (itCR != _responseBuf.end()) { std::string statusLine{_responseBuf.begin(), itCR}; - std::cout << "Status line: " << statusLine << std::endl; + // std::cout << "Status line: " << statusLine << std::endl; std::regex rxStatusLine{"[hHtTpP/1\\.]+ (\\d+) [a-zA-Z0-9 ]+"}; std::cmatch matches; @@ -53,7 +53,7 @@ bool HttpResponseParser::_ParseBuf() } _statusCode = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Result: " << _statusCode << std::endl; + // std::cout << "Result: " << _statusCode << std::endl; _state = ParserState::Fields; _itParseFrom = itCR + 2; @@ -82,7 +82,7 @@ bool HttpResponseParser::_ParseBuf() if (availableBodySize == _contentLength) { _body.write(&(*_itParseFrom), _contentLength); - std::cout << "Body: " << _body.str() << std::endl; + // std::cout << "Body: " << _body.str() << std::endl; _state = ParserState::Complete; _itParseFrom = _responseBuf.end(); } @@ -115,7 +115,7 @@ bool HttpResponseParser::_ParseNextField() } std::string field{_itParseFrom, itCR}; - std::cout << "Field: " << field << std::endl; + // std::cout << "Field: " << field << std::endl; if (field.find("Content-Length") != std::string::npos) { std::regex rxContentLength{".*:[ ]*(\\d+).*"}; @@ -126,7 +126,7 @@ bool HttpResponseParser::_ParseNextField() } _contentLength = static_cast(std::strtoul(matches[1].str().data(), nullptr, 10)); - std::cout << "Body size: " << _contentLength << std::endl; + // std::cout << "Body size: " << _contentLength << std::endl; } // else, field not interesting _itParseFrom = itCR + 2; From 53ca20fa2af134216d19e92ada7d8254f704191c Mon Sep 17 00:00:00 2001 From: Shishir Bhat Date: Fri, 14 May 2021 13:10:12 -0700 Subject: [PATCH 7/7] Credit comments --- sdk-cpp/src/internal/rest/download_rest.cpp | 2 +- sdk-cpp/src/internal/util/do_http_message.h | 1 + sdk-cpp/src/internal/util/do_http_parser.cpp | 2 +- sdk-cpp/src/internal/util/do_http_parser.h | 2 ++ 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk-cpp/src/internal/rest/download_rest.cpp b/sdk-cpp/src/internal/rest/download_rest.cpp index 7e3ec93c..97e6c6d1 100644 --- a/sdk-cpp/src/internal/rest/download_rest.cpp +++ b/sdk-cpp/src/internal/rest/download_rest.cpp @@ -21,7 +21,7 @@ namespace microsoft::deliveryoptimization::details CDownloadRest::CDownloadRest(const std::string& uri, const std::string& downloadFilePath) { std::stringstream url; - url << g_downloadUriPart << "/create" << "?Uri=" << Url::EncodeDataString(uri)<< "&DownloadFilePath=" + url << g_downloadUriPart << "/create" << "?Uri=" << Url::EncodeDataString(uri) << "&DownloadFilePath=" << Url::EncodeDataString(downloadFilePath); for (int retryAttempts = 0; retryAttempts < g_maxNumRetryAttempts; retryAttempts++) diff --git a/sdk-cpp/src/internal/util/do_http_message.h b/sdk-cpp/src/internal/util/do_http_message.h index 0be45734..10ca4461 100644 --- a/sdk-cpp/src/internal/util/do_http_message.h +++ b/sdk-cpp/src/internal/util/do_http_message.h @@ -8,6 +8,7 @@ namespace microsoft::deliveryoptimization::details { +// Credit: Code takes a little inspiration from Boost.Beast. Too bad it is not available on Ubuntu 18.04. class HttpRequest { public: diff --git a/sdk-cpp/src/internal/util/do_http_parser.cpp b/sdk-cpp/src/internal/util/do_http_parser.cpp index e3eb73ce..35d55ae6 100644 --- a/sdk-cpp/src/internal/util/do_http_parser.cpp +++ b/sdk-cpp/src/internal/util/do_http_parser.cpp @@ -31,7 +31,7 @@ void HttpResponseParser::OnData(const char* pData, size_t cb) } } -// Returns true if more processing can be done, false if processing is done or cannot continue until more data is received +// Returns true if more processing can be done, false if processing is done or cannot continue until more data is received. bool HttpResponseParser::_ParseBuf() { const auto oldState = _state; diff --git a/sdk-cpp/src/internal/util/do_http_parser.h b/sdk-cpp/src/internal/util/do_http_parser.h index ba847a16..85d26d19 100644 --- a/sdk-cpp/src/internal/util/do_http_parser.h +++ b/sdk-cpp/src/internal/util/do_http_parser.h @@ -7,6 +7,8 @@ namespace microsoft::deliveryoptimization::details { +// Very limited parsing abilities, just enough to support the Agent's responses. +// Credit: Code takes a little inspiration from Boost.Beast. Too bad it is not available on Ubuntu 18.04. class HttpResponseParser { public: