From ac2ecc79b514ed7ad22c69346e6513e96a0fb295 Mon Sep 17 00:00:00 2001 From: nidietr_MSFT Date: Fri, 6 Dec 2024 12:00:52 +0100 Subject: [PATCH] Squashed 'src/SfsClient/sfs-client/' changes from be733af9..44d81dc8 44d81dc8 Using early-return in UrlBuilder::GetQuery() 9e9b27e3 Fixing "Use of string after lifetime ends" 21bf6b98 Adding functional proxy tests 46388eaa Adding a ProxyServer implementation 7a0bfbec Exposing Path and Query via UrlBuilder b2014df3 Adding unit tests for proxy validation cdd840b0 Adding proxy input string support 216210ab Adding required permissions to enable uploading of CodeQL results (#214) fb953d6e Bump github/codeql-action from 2 to 3 (#215) 52af7124 Enabling CodeQL scanning (#211) e555d764 Bump clang-format from 18.1.5 to 19.1.1 (#210) ab8f0e72 Setup: improving build tools installation (#207) git-subtree-dir: src/SfsClient/sfs-client git-subtree-split: 44d81dc8e7614c0be8777db22431e5065aa7a6b8 --- .../workflows/initialize-codeql/action.yml | 12 ++ .github/workflows/main-build-ubuntu.yml | 8 + .github/workflows/main-build-windows.yml | 10 +- .github/workflows/pr.yml | 14 +- client/include/sfsclient/RequestParams.h | 5 + client/src/details/UrlBuilder.cpp | 25 +++ client/src/details/UrlBuilder.h | 3 + .../details/connection/ConnectionConfig.cpp | 1 + .../src/details/connection/ConnectionConfig.h | 3 + .../src/details/connection/CurlConnection.cpp | 5 + client/tests/CMakeLists.txt | 2 + client/tests/functional/SFSClientTests.cpp | 20 +++ .../details/CurlConnectionTests.cpp | 77 +++++++-- client/tests/mock/MockWebServer.cpp | 155 +----------------- client/tests/mock/ProxyServer.cpp | 97 +++++++++++ client/tests/mock/ProxyServer.h | 34 ++++ client/tests/mock/ServerCommon.cpp | 121 ++++++++++++++ client/tests/mock/ServerCommon.h | 84 ++++++++++ client/tests/unit/SFSClientTests.cpp | 38 +++++ scripts/Setup.ps1 | 41 +++-- scripts/pip.requirements.txt | 2 +- 21 files changed, 584 insertions(+), 173 deletions(-) create mode 100644 .github/workflows/initialize-codeql/action.yml create mode 100644 client/tests/mock/ProxyServer.cpp create mode 100644 client/tests/mock/ProxyServer.h create mode 100644 client/tests/mock/ServerCommon.cpp create mode 100644 client/tests/mock/ServerCommon.h diff --git a/.github/workflows/initialize-codeql/action.yml b/.github/workflows/initialize-codeql/action.yml new file mode 100644 index 00000000000..990b0603bf9 --- /dev/null +++ b/.github/workflows/initialize-codeql/action.yml @@ -0,0 +1,12 @@ +name: Initialize CodeQL + +description: Initializes CodeQL action to be used in build workflows + +runs: + using: "composite" + + steps: + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: cpp \ No newline at end of file diff --git a/.github/workflows/main-build-ubuntu.yml b/.github/workflows/main-build-ubuntu.yml index 1d12f117231..59ee8521c9e 100644 --- a/.github/workflows/main-build-ubuntu.yml +++ b/.github/workflows/main-build-ubuntu.yml @@ -5,8 +5,10 @@ on: branches: [ "main" ] # Permissions and environment values to be able to update the dependency graph with vcpkg information +# and to enable the writing/uploading of CodeQL scan results permissions: contents: write + security-events: write env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -19,6 +21,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Initialize CodeQL + uses: ./.github/workflows/initialize-codeql + - name: Setup run: source ./scripts/setup.sh @@ -36,3 +41,6 @@ jobs: run: | ./scripts/build.sh --build-type Release ./scripts/test.sh --output-on-failure + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/main-build-windows.yml b/.github/workflows/main-build-windows.yml index 1a57d5689e7..0c2a6cf6533 100644 --- a/.github/workflows/main-build-windows.yml +++ b/.github/workflows/main-build-windows.yml @@ -5,8 +5,10 @@ on: branches: [ "main" ] # Permissions and environment values to be able to update the dependency graph with vcpkg information +# and to enable the writing/uploading of CodeQL scan results permissions: contents: write + security-events: write env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -19,12 +21,15 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Initialize CodeQL + uses: ./.github/workflows/initialize-codeql + - name: Install Winget uses: ./.github/workflows/install-winget - name: Setup shell: pwsh - run: .\scripts\Setup.ps1 -NoBuildTools + run: .\scripts\Setup.ps1 - name: Build and Test (no test overrides) shell: pwsh @@ -43,3 +48,6 @@ jobs: run: | .\scripts\Build.ps1 -BuildType Release .\scripts\Test.ps1 -OutputOnFailure + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 8b629209436..f3740d36c41 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -22,12 +22,15 @@ jobs: core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || ''); core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || ''); + - name: Initialize CodeQL + uses: ./.github/workflows/initialize-codeql + - name: Install Winget uses: ./.github/workflows/install-winget - name: Setup shell: pwsh - run: .\scripts\Setup.ps1 -NoBuildTools + run: .\scripts\Setup.ps1 - name: Check formatting shell: pwsh @@ -45,6 +48,9 @@ jobs: .\scripts\Build.ps1 -EnableTestOverrides .\scripts\Test.ps1 -OutputOnFailure + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + build-ubuntu: runs-on: ubuntu-latest @@ -58,6 +64,9 @@ jobs: core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || ''); core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || ''); + - name: Initialize CodeQL + uses: ./.github/workflows/initialize-codeql + - name: Setup run: source ./scripts/setup.sh @@ -73,3 +82,6 @@ jobs: run: | ./scripts/build.sh --enable-test-overrides ./scripts/test.sh --output-on-failure + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/client/include/sfsclient/RequestParams.h b/client/include/sfsclient/RequestParams.h index a1a1b7e855b..b2961f5c999 100644 --- a/client/include/sfsclient/RequestParams.h +++ b/client/include/sfsclient/RequestParams.h @@ -35,6 +35,11 @@ struct RequestParams /// @note If not provided, a new CorrelationVector will be generated std::optional baseCV; + /// @brief Proxy setting which can be used to establish connections with the server (optional) + /// @note The string can be a hostname or dotted numerical IP address. It can be suffixed with the port number + /// like :[port], and can be prefixed with [scheme]://. If not provided, no proxy will be used. + std::optional proxy; + /// @brief Retry for a web request after a failed attempt. If true, client will retry up to c_maxRetries times bool retryOnError{true}; }; diff --git a/client/src/details/UrlBuilder.cpp b/client/src/details/UrlBuilder.cpp index a1a1b10ef17..95d2abd8f58 100644 --- a/client/src/details/UrlBuilder.cpp +++ b/client/src/details/UrlBuilder.cpp @@ -66,6 +66,31 @@ std::string UrlBuilder::GetUrl() const return urlPtr; } +std::string UrlBuilder::GetPath() const +{ + CurlCharPtr path; + char* pathPtr = path.get(); + THROW_IF_CURL_URL_SETUP_ERROR(curl_url_get(m_handle, CURLUPART_PATH, &pathPtr, 0 /*flags*/)); + return pathPtr; +} + +std::string UrlBuilder::GetQuery() const +{ + CurlCharPtr query; + char* queryPtr = query.get(); + const auto queryResult = curl_url_get(m_handle, CURLUPART_QUERY, &queryPtr, 0 /*flags*/); + switch (queryResult) + { + case CURLUE_OK: + return queryPtr; + case CURLUE_NO_QUERY: + return {}; + default: + THROW_IF_CURL_URL_SETUP_ERROR(queryResult); + } + return {}; +} + UrlBuilder& UrlBuilder::SetScheme(Scheme scheme) { switch (scheme) diff --git a/client/src/details/UrlBuilder.h b/client/src/details/UrlBuilder.h index f0b786defc2..cf738dab1e8 100644 --- a/client/src/details/UrlBuilder.h +++ b/client/src/details/UrlBuilder.h @@ -39,6 +39,9 @@ class UrlBuilder std::string GetUrl() const; + std::string GetPath() const; + std::string GetQuery() const; + /** * @brief Set the scheme for the URL * @param scheme The scheme to set for the URL Ex: Https diff --git a/client/src/details/connection/ConnectionConfig.cpp b/client/src/details/connection/ConnectionConfig.cpp index 7279ccf5aa9..319d5a132c6 100644 --- a/client/src/details/connection/ConnectionConfig.cpp +++ b/client/src/details/connection/ConnectionConfig.cpp @@ -11,5 +11,6 @@ using namespace SFS::details; ConnectionConfig::ConnectionConfig(const SFS::RequestParams& requestParams) : maxRetries(requestParams.retryOnError ? c_maxRetries : 0) , baseCV(requestParams.baseCV) + , proxy(requestParams.proxy) { } diff --git a/client/src/details/connection/ConnectionConfig.h b/client/src/details/connection/ConnectionConfig.h index 64fb42645b7..30d963ae9f0 100644 --- a/client/src/details/connection/ConnectionConfig.h +++ b/client/src/details/connection/ConnectionConfig.h @@ -22,6 +22,9 @@ struct ConnectionConfig /// @brief The correlation vector to use for requests std::optional baseCV; + + /// @brief Proxy setting which can be used to establish connections with the server + std::optional proxy; }; } // namespace details } // namespace SFS diff --git a/client/src/details/connection/CurlConnection.cpp b/client/src/details/connection/CurlConnection.cpp index 354dcab3b7e..c7c06831415 100644 --- a/client/src/details/connection/CurlConnection.cpp +++ b/client/src/details/connection/CurlConnection.cpp @@ -284,6 +284,11 @@ CurlConnection::CurlConnection(const ConnectionConfig& config, const ReportingHa m_handler, "Failed to set up curl"); + if (config.proxy) + { + THROW_IF_CURL_SETUP_ERROR(curl_easy_setopt(m_handle, CURLOPT_PROXY, config.proxy->c_str())); + } + // TODO #41: Pass AAD token in the header if it is available // TODO #42: Cert pinning with service } diff --git a/client/tests/CMakeLists.txt b/client/tests/CMakeLists.txt index 91f4fe0a132..5782701e101 100644 --- a/client/tests/CMakeLists.txt +++ b/client/tests/CMakeLists.txt @@ -17,6 +17,8 @@ target_sources( functional/details/SFSClientImplTests.cpp functional/SFSClientTests.cpp mock/MockWebServer.cpp + mock/ProxyServer.cpp + mock/ServerCommon.cpp unit/AppContentTests.cpp unit/AppFileTests.cpp unit/ApplicabilityDetailsTests.cpp diff --git a/client/tests/functional/SFSClientTests.cpp b/client/tests/functional/SFSClientTests.cpp index 5ec016a88f1..60f95a9fe4f 100644 --- a/client/tests/functional/SFSClientTests.cpp +++ b/client/tests/functional/SFSClientTests.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "../mock/MockWebServer.h" +#include "../mock/ProxyServer.h" #include "../util/TestHelper.h" #include "TestOverride.h" #include "sfsclient/SFSClient.h" @@ -112,6 +113,19 @@ TEST("Testing SFSClient::GetLatestDownloadInfo()") CheckMockContent(contents[0], c_version); } + SECTION("No attributes + proxy") + { + test::ProxyServer proxy; + + params.productRequests = {{c_productName, {}}}; + params.proxy = proxy.GetBaseUrl(); + REQUIRE(sfsClient->GetLatestDownloadInfo(params, contents) == Result::Success); + REQUIRE(contents.size() == 1); + CheckMockContent(contents[0], c_version); + + REQUIRE(proxy.Stop() == Result::Success); + } + SECTION("With attributes") { const TargetingAttributes attributes{{"attr1", "value"}}; @@ -143,6 +157,8 @@ TEST("Testing SFSClient::GetLatestDownloadInfo()") CheckMockContent(contents[0], c_nextVersion); } } + + REQUIRE(server.Stop() == Result::Success); } TEST("Testing SFSClient::GetLatestAppDownloadInfo()") @@ -244,6 +260,8 @@ TEST("Testing SFSClient::GetLatestAppDownloadInfo()") "Unexpected content type [Generic] returned by the service does not match the expected [App]"); REQUIRE(contents.empty()); } + + REQUIRE(server.Stop() == Result::Success); } TEST("Testing SFSClient retry behavior") @@ -437,4 +455,6 @@ TEST("Testing SFSClient retry behavior") } } } + + REQUIRE(server.Stop() == Result::Success); } diff --git a/client/tests/functional/details/CurlConnectionTests.cpp b/client/tests/functional/details/CurlConnectionTests.cpp index 1d8a21ad681..77c114395ac 100644 --- a/client/tests/functional/details/CurlConnectionTests.cpp +++ b/client/tests/functional/details/CurlConnectionTests.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "../../mock/MockWebServer.h" +#include "../../mock/ProxyServer.h" #include "../../util/SFSExceptionMatcher.h" #include "../../util/TestHelper.h" #include "ReportingHandler.h" @@ -102,20 +103,46 @@ TEST("Testing CurlConnection()") { const std::string url = urlBuilder.GetSpecificVersionUrl(c_productName, c_version); - // Before registering the product, the URL returns 404 Not Found - REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound); + json expectedResponse; + expectedResponse["ContentId"] = {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_version}}; - // Register the product - server.RegisterProduct(c_productName, c_version); + SECTION("Direct connection") + { + // Before registering the product, the URL returns 404 Not Found + REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound); - // After registering the product, the URL returns 200 OK - std::string out; - REQUIRE_NOTHROW(out = connection->Get(url)); + // Register the product + server.RegisterProduct(c_productName, c_version); - json expectedResponse; - expectedResponse["ContentId"] = {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_version}}; + // After registering the product, the URL returns 200 OK + std::string out; + REQUIRE_NOTHROW(out = connection->Get(url)); + + REQUIRE(json::parse(out) == expectedResponse); + } + + SECTION("With proxy") + { + test::ProxyServer proxy; + + ConnectionConfig config; + config.proxy = proxy.GetBaseUrl(); + connection = connectionManager.MakeConnection(config); + + // Before registering the product, the URL returns 404 Not Found + REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound); + + // Register the product + server.RegisterProduct(c_productName, c_version); + + // After registering the product, the URL returns 200 OK + std::string out; + REQUIRE_NOTHROW(out = connection->Get(url)); - REQUIRE(json::parse(out) == expectedResponse); + REQUIRE(json::parse(out) == expectedResponse); + + REQUIRE(proxy.Stop() == Result::Success); + } } SECTION("Testing CurlConnection::Post()") @@ -153,6 +180,21 @@ TEST("Testing CurlConnection()") expectedResponse.push_back( {{"ContentId", {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_nextVersion}}}}); REQUIRE(json::parse(out) == expectedResponse); + + SECTION("Testing with proxy") + { + test::ProxyServer proxy; + + ConnectionConfig config; + config.proxy = proxy.GetBaseUrl(); + connection = connectionManager.MakeConnection(config); + + REQUIRE_NOTHROW(out = connection->Post(url, body.dump())); + + REQUIRE(json::parse(out) == expectedResponse); + + REQUIRE(proxy.Stop() == Result::Success); + } } SECTION("With GetDownloadInfo mock") @@ -187,6 +229,21 @@ TEST("Testing CurlConnection()") {"IntegrityCheckInfo", {{"PiecesHashFileUrl", "http://localhost/2.bin"}, {"HashOfHashes", "abcd"}}}}; REQUIRE(json::parse(out) == expectedResponse); + + SECTION("Testing with proxy") + { + test::ProxyServer proxy; + + ConnectionConfig config; + config.proxy = proxy.GetBaseUrl(); + connection = connectionManager.MakeConnection(config); + + REQUIRE_NOTHROW(out = connection->Post(url)); + + REQUIRE(json::parse(out) == expectedResponse); + + REQUIRE(proxy.Stop() == Result::Success); + } } } diff --git a/client/tests/mock/MockWebServer.cpp b/client/tests/mock/MockWebServer.cpp index b077dbdc161..be085ac05d2 100644 --- a/client/tests/mock/MockWebServer.cpp +++ b/client/tests/mock/MockWebServer.cpp @@ -5,6 +5,7 @@ #include "../util/TestHelper.h" #include "ErrorHandling.h" +#include "ServerCommon.h" #include "Util.h" #include "connection/HttpHeader.h" @@ -13,7 +14,6 @@ #include #endif -#include #include #include @@ -27,46 +27,10 @@ using namespace SFS::details; using namespace SFS::details::util; using namespace SFS::test; using namespace SFS::test::details; -using namespace std::string_literals; using json = nlohmann::json; -#define BUILD_BUFFERED_LOG_DATA(message) \ - BufferedLogData \ - { \ - message, __FILE__, __LINE__, __FUNCTION__, std::chrono::system_clock::now() \ - } - -#define BUFFER_LOG(message) BufferLog(BUILD_BUFFERED_LOG_DATA(message)) - -const char* c_listenHostName = "localhost"; - namespace { -std::string ToString(httplib::StatusCode status) -{ - return std::to_string(status) + " " + std::string(httplib::status_message(status)); -} - -class StatusCodeException : public std::exception -{ - public: - StatusCodeException(httplib::StatusCode status) : m_status(status) - { - } - - const char* what() const noexcept override - { - return ToString(m_status).c_str(); - } - - httplib::StatusCode GetStatusCode() const - { - return m_status; - } - - private: - httplib::StatusCode m_status; -}; struct App { @@ -230,20 +194,6 @@ json GeneratePostAppDownloadInfo(const std::string& name) return response; } -struct BufferedLogData -{ - std::string message; - std::string file; - unsigned line; - std::string function; - std::chrono::time_point time; -}; - -LogData ToLogData(const BufferedLogData& data) -{ - return {LogSeverity::Info, data.message.c_str(), data.file.c_str(), data.line, data.function.c_str(), data.time}; -} - void CheckApiVersion(const httplib::Request& req, std::string_view apiVersion) { if (util::AreNotEqualI(req.path_params.at("apiVersion"), apiVersion)) @@ -255,7 +205,7 @@ void CheckApiVersion(const httplib::Request& req, std::string_view apiVersion) namespace SFS::test::details { -class MockWebServerImpl +class MockWebServerImpl : public BaseServerImpl { public: MockWebServerImpl() = default; @@ -264,11 +214,6 @@ class MockWebServerImpl MockWebServerImpl(const MockWebServerImpl&) = delete; MockWebServerImpl& operator=(const MockWebServerImpl&) = delete; - void Start(); - Result Stop(); - - std::string GetUrl() const; - void RegisterProduct(std::string&& name, std::string&& version); void RegisterAppProduct(std::string&& name, std::string&& version, std::vector&& prerequisites); void RegisterExpectedRequestHeader(std::string&& header, std::string&& value); @@ -276,8 +221,8 @@ class MockWebServerImpl void SetResponseHeaders(std::unordered_map headersByCode); private: - void ConfigureServerSettings(); - void ConfigureRequestHandlers(); + void ConfigureRequestHandlers() override; + std::string GetLogIdentifier() override; void ConfigurePostLatestVersion(); void ConfigurePostLatestVersionBatch(); @@ -291,16 +236,6 @@ class MockWebServerImpl const std::function& callback); void CheckRequestHeaders(const httplib::Request& req); - void BufferLog(const BufferedLogData& data); - void ProcessBufferedLogs(); - - httplib::Server m_server; - int m_port{-1}; - - std::optional m_lastException; - - std::thread m_listenerThread; - using VersionList = std::set; std::unordered_map m_products; @@ -310,9 +245,6 @@ class MockWebServerImpl std::unordered_map m_expectedRequestHeaders; std::queue m_forcedHttpErrors; std::unordered_map m_headersByCode; - - std::vector m_bufferedLog; - std::mutex m_logMutex; }; } // namespace SFS::test::details @@ -369,49 +301,6 @@ void MockWebServer::SetResponseHeaders(std::unordered_map h m_impl->SetResponseHeaders(std::move(headersByCode)); } -void MockWebServerImpl::Start() -{ - ConfigureServerSettings(); - ConfigureRequestHandlers(); - - m_port = m_server.bind_to_any_port(c_listenHostName); - m_listenerThread = std::thread([&]() { m_server.listen_after_bind(); }); -} - -void MockWebServerImpl::ConfigureServerSettings() -{ - m_server.set_logger([&](const httplib::Request& req, const httplib::Response& res) { - BUFFER_LOG("Request: " + req.method + " " + req.path + " " + req.version); - BUFFER_LOG("Request Body: " + req.body); - - BUFFER_LOG("Response: " + res.version + " " + ::ToString(static_cast(res.status)) + " " + - res.reason); - BUFFER_LOG("Response body: " + res.body); - }); - - m_server.set_exception_handler([&](const httplib::Request&, httplib::Response& res, std::exception_ptr ep) { - try - { - std::rethrow_exception(ep); - } - catch (std::exception& e) - { - m_lastException = Result(Result::HttpUnexpected, e.what()); - } - catch (...) - { - m_lastException = Result(Result::HttpUnexpected, "Unknown Exception"); - } - - ProcessBufferedLogs(); - - res.status = httplib::StatusCode::InternalServerError_500; - }); - - // Keeping this interval to a minimum ensures tests run quicker - m_server.set_keep_alive_timeout(1); // 1 second -} - void MockWebServerImpl::ConfigureRequestHandlers() { ConfigurePostLatestVersion(); @@ -420,6 +309,11 @@ void MockWebServerImpl::ConfigureRequestHandlers() ConfigurePostDownloadInfo(); } +std::string MockWebServerImpl::GetLogIdentifier() +{ + return "MockWebServer"; +} + void MockWebServerImpl::ConfigurePostLatestVersion() { // Path: /api//contents//namespaces//names//versions/latest?action=select @@ -764,37 +658,6 @@ void MockWebServerImpl::CheckRequestHeaders(const httplib::Request& req) } } -void MockWebServerImpl::BufferLog(const BufferedLogData& data) -{ - std::lock_guard guard(m_logMutex); - m_bufferedLog.push_back(data); -} - -void MockWebServerImpl::ProcessBufferedLogs() -{ - for (const auto& data : m_bufferedLog) - { - LogCallbackToTest(ToLogData(data)); - } - m_bufferedLog.clear(); -} - -Result MockWebServerImpl::Stop() -{ - if (m_listenerThread.joinable()) - { - m_server.stop(); - m_listenerThread.join(); - } - ProcessBufferedLogs(); - return m_lastException.value_or(Result::Success); -} - -std::string MockWebServerImpl::GetUrl() const -{ - return "http://"s + c_listenHostName + ":"s + std::to_string(m_port); -} - void MockWebServerImpl::RegisterProduct(std::string&& name, std::string&& version) { m_products[std::move(name)].emplace(std::move(version)); diff --git a/client/tests/mock/ProxyServer.cpp b/client/tests/mock/ProxyServer.cpp new file mode 100644 index 00000000000..2ee330a3383 --- /dev/null +++ b/client/tests/mock/ProxyServer.cpp @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#include "ProxyServer.h" + +#include "../util/TestHelper.h" +#include "ErrorHandling.h" +#include "ReportingHandler.h" +#include "ServerCommon.h" +#include "UrlBuilder.h" + +using namespace SFS; +using namespace SFS::details; +using namespace SFS::test; +using namespace SFS::test::details; + +namespace SFS::test::details +{ +class ProxyServerImpl : public BaseServerImpl +{ + private: + void ConfigureRequestHandlers() override; + std::string GetLogIdentifier() override; +}; +} // namespace SFS::test::details + +ProxyServer::ProxyServer() +{ + m_impl = std::make_unique(); + m_impl->Start(); +} + +ProxyServer::~ProxyServer() +{ + const auto ret = Stop(); + if (!ret) + { + TEST_UNSCOPED_INFO("Failed to stop: " + std::string(ToString(ret.GetCode()))); + } +} + +Result ProxyServer::Stop() +{ + return m_impl->Stop(); +} + +std::string ProxyServer::GetBaseUrl() const +{ + return m_impl->GetUrl(); +} + +void ProxyServerImpl::ConfigureRequestHandlers() +{ + auto HandleRequest = [&](const httplib::Request& req, httplib::Response& res) { + // As a proxy, we'll parse the URL and Path/Query so we can reuse them in httplib::Client + ReportingHandler handler; + UrlBuilder urlBuilder(req.target.c_str(), handler); + const std::string path = urlBuilder.GetPath(); + const std::string query = urlBuilder.GetQuery(); + urlBuilder.ResetPath().ResetQuery(); + + // URL may come back from UrlBuilder with a final /, which doesn't work with httplib::Client, so we remove it + auto url = urlBuilder.GetUrl(); + if (url.at(url.size() - 1) == '/') + { + url.pop_back(); + } + const std::string pathAndQuery = path + (query.empty() ? "" : ("?" + query)); + httplib::Client cli(url); + httplib::Result result; + if (req.method == "GET") + { + result = cli.Get(pathAndQuery, req.headers); + } + else if (req.method == "POST") + { + const auto length = std::stoi(req.get_header_value("Content-Length")); + result = + cli.Post(pathAndQuery, req.headers, req.body.c_str(), length, req.get_header_value("Content-Type")); + } + + if (!result) + { + BUFFER_LOG("Client error: " + to_string(result.error())); + throw StatusCodeException(httplib::StatusCode::InternalServerError_500); + } + res = result.value(); + }; + + m_server.Get(".*", HandleRequest); + m_server.Post(".*", HandleRequest); +} + +std::string ProxyServerImpl::GetLogIdentifier() +{ + return "ProxyServer"; +} diff --git a/client/tests/mock/ProxyServer.h b/client/tests/mock/ProxyServer.h new file mode 100644 index 00000000000..eb833b2985a --- /dev/null +++ b/client/tests/mock/ProxyServer.h @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#pragma once + +#include "Result.h" + +#include + +namespace SFS::test +{ +namespace details +{ +class ProxyServerImpl; +} + +// Proxy Server implementation that redirects GET and POST requests directly +class ProxyServer +{ + public: + ProxyServer(); + ~ProxyServer(); + + ProxyServer(const ProxyServer&) = delete; + ProxyServer& operator=(const ProxyServer&) = delete; + + Result Stop(); + + std::string GetBaseUrl() const; + + private: + std::unique_ptr m_impl; +}; +} // namespace SFS::test diff --git a/client/tests/mock/ServerCommon.cpp b/client/tests/mock/ServerCommon.cpp new file mode 100644 index 00000000000..bdf3bb2d3dd --- /dev/null +++ b/client/tests/mock/ServerCommon.cpp @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#include "ServerCommon.h" + +#include "../util/TestHelper.h" +#include "Result.h" + +using SFS::test::BufferedLogData; +using SFS::test::StatusCodeException; +using SFS::test::details::BaseServerImpl; +using namespace SFS; +using namespace std::string_literals; + +static constexpr const char* c_listenHostName = "localhost"; + +static std::string ToString(httplib::StatusCode status) +{ + return std::to_string(status) + " " + std::string(httplib::status_message(status)); +} + +StatusCodeException::StatusCodeException(httplib::StatusCode status) : m_status(status), m_message(::ToString(m_status)) +{ +} + +const char* StatusCodeException::what() const noexcept +{ + return m_message.c_str(); +} + +httplib::StatusCode StatusCodeException::GetStatusCode() const +{ + return m_status; +} + +static SFS::LogData ToLogData(const BufferedLogData& data) +{ + return {LogSeverity::Info, data.message.c_str(), data.file.c_str(), data.line, data.function.c_str(), data.time}; +} + +void BaseServerImpl::Start() +{ + ConfigureServerSettings(); + ConfigureRequestHandlers(); + + m_port = m_server.bind_to_any_port(c_listenHostName); + m_listenerThread = std::thread([&]() { m_server.listen_after_bind(); }); +} + +void BaseServerImpl::ConfigureServerSettings() +{ + m_server.set_logger([&](const httplib::Request& req, const httplib::Response& res) { + BUFFER_LOG("Request: " + req.method + " " + req.path + " " + req.version); + BUFFER_LOG("Request Body: " + req.body); + + BUFFER_LOG("Response: " + res.version + " " + ::ToString(static_cast(res.status)) + " " + + res.reason); + BUFFER_LOG("Response body: " + res.body); + }); + + m_server.set_exception_handler([&](const httplib::Request&, httplib::Response& res, std::exception_ptr ep) { + try + { + std::rethrow_exception(ep); + } + catch (std::exception& e) + { + m_lastException = Result(Result::HttpUnexpected, e.what()); + } + catch (...) + { + m_lastException = Result(Result::HttpUnexpected, "Unknown Exception"); + } + + ProcessBufferedLogs(); + + res.status = httplib::StatusCode::InternalServerError_500; + }); + + // Keeping this interval to a minimum ensures tests run quicker + m_server.set_keep_alive_timeout(1); // 1 second +} + +void BaseServerImpl::BufferLog(const BufferedLogData& data) +{ + std::lock_guard guard(m_logMutex); + m_bufferedLog.push_back(data); +} + +BufferedLogData BaseServerImpl::BuildBufferedLogData(const std::string& message, + const char* file, + unsigned line, + const char* function) +{ + return BufferedLogData{GetLogIdentifier() + ": " + message, file, line, function, std::chrono::system_clock::now()}; +} + +void BaseServerImpl::ProcessBufferedLogs() +{ + for (const auto& data : m_bufferedLog) + { + LogCallbackToTest(ToLogData(data)); + } + m_bufferedLog.clear(); +} + +Result BaseServerImpl::Stop() +{ + if (m_listenerThread.joinable()) + { + m_server.stop(); + m_listenerThread.join(); + } + ProcessBufferedLogs(); + return m_lastException.value_or(Result::Success); +} + +std::string BaseServerImpl::GetUrl() const +{ + return "http://"s + c_listenHostName + ":"s + std::to_string(m_port); +} diff --git a/client/tests/mock/ServerCommon.h b/client/tests/mock/ServerCommon.h new file mode 100644 index 00000000000..bd3cca9fcd6 --- /dev/null +++ b/client/tests/mock/ServerCommon.h @@ -0,0 +1,84 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#pragma once + +#include "Logging.h" +#include "Result.h" + +#include + +#include +#include +#include + +namespace SFS::test +{ +class StatusCodeException : public std::exception +{ + public: + StatusCodeException(httplib::StatusCode status); + + const char* what() const noexcept override; + + httplib::StatusCode GetStatusCode() const; + + private: + httplib::StatusCode m_status; + std::string m_message; +}; + +#define BUILD_BUFFERED_LOG_DATA(message) BuildBufferedLogData(message, __FILE__, __LINE__, __FUNCTION__) + +#define BUFFER_LOG(message) BufferLog(BUILD_BUFFERED_LOG_DATA(message)) + +struct BufferedLogData +{ + std::string message; + std::string file; + unsigned line; + std::string function; + std::chrono::time_point time; +}; + +namespace details +{ +class BaseServerImpl +{ + public: + BaseServerImpl() = default; + ~BaseServerImpl() = default; + + BaseServerImpl(const BaseServerImpl&) = delete; + BaseServerImpl& operator=(const BaseServerImpl&) = delete; + + void Start(); + Result Stop(); + + std::string GetUrl() const; + + protected: + void ConfigureServerSettings(); + virtual void ConfigureRequestHandlers() = 0; + + virtual std::string GetLogIdentifier() = 0; + + void BufferLog(const BufferedLogData& data); + BufferedLogData BuildBufferedLogData(const std::string& message, + const char* file, + unsigned line, + const char* function); + void ProcessBufferedLogs(); + + httplib::Server m_server; + int m_port{-1}; + + std::optional m_lastException; + + std::thread m_listenerThread; + + std::vector m_bufferedLog; + std::mutex m_logMutex; +}; +} // namespace details +} // namespace SFS::test diff --git a/client/tests/unit/SFSClientTests.cpp b/client/tests/unit/SFSClientTests.cpp index 496482b97e3..33cc0d5cc31 100644 --- a/client/tests/unit/SFSClientTests.cpp +++ b/client/tests/unit/SFSClientTests.cpp @@ -261,6 +261,44 @@ void TestProductInRequestParams(const std::function .\scripts\Setup.ps1 #> -param ( - [switch] $NoBuildTools -) $ErrorActionPreference = "Stop" @@ -71,19 +68,33 @@ function Install-CMake { } function Install-CppBuildTools { - if ($NoBuildTools) { - Write-Host -ForegroundColor Yellow "`nSkipping C++ Build Tools installation" - return - } + Write-Host -ForegroundColor Cyan "`nInstalling C++ Builds tools if they are not installed" - Write-Host -ForegroundColor Cyan "`nInstalling C++ Build Tools" + # Instaling vswhere, which will be used to query for the required build tools + try { + vswhere -? 2>&1 | Out-Null + } + catch { + winget install vswhere + if (!$?) { + Write-Host -ForegroundColor Red "Failed to install vswhere" + exit 1 + } + } # - Microsoft.VisualStudio.Workload.VCTools is the C++ workload in the Visual Studio Build Tools - # --wait makes the install synchronous - winget install Microsoft.VisualStudio.2022.BuildTools --silent --override "--wait --add Microsoft.VisualStudio.Workload.VCTools --includeRecommended --remove Microsoft.VisualStudio.Component.VC.CMake.Project" - if (!$?) { - Write-Host -ForegroundColor Red "Failed to install build tools" - exit 1 + # - Microsoft.VisualStudio.Workload.NativeDesktop is the C++ workload that comes pre-installed in the github runner image + $ExistingBuildTools = vswhere -products * -requires Microsoft.VisualStudio.Workload.VCTools Microsoft.VisualStudio.Workload.NativeDesktop -requiresAny -format json | ConvertFrom-Json + if ($null -eq $ExistingBuildTools) + { + Write-Host "`nTools not found, installing..." + + # --wait makes the install synchronous + winget install Microsoft.VisualStudio.2022.BuildTools --silent --override "--wait --quiet --add Microsoft.VisualStudio.Workload.VCTools --includeRecommended --remove Microsoft.VisualStudio.Component.VC.CMake.Project" + if (!$?) { + Write-Host -ForegroundColor Red "Failed to install build tools" + exit 1 + } } } @@ -93,13 +104,15 @@ function Install-Vcpkg { if (!(Test-Path vcpkg)) { Write-Host "Cloning vcpkg repo" git clone https://github.com/microsoft/vcpkg $GitRoot\vcpkg - & "$GitRoot\vcpkg\bootstrap-vcpkg.bat" } else { Write-Host "Checking if vcpkg repo has new commits" $NoUpdatesString = "Already up to date." git -C "$GitRoot\vcpkg" pull --show-forced-updates | Select-String -Pattern $NoUpdatesString -NotMatch } + + # Bootstrapping on every setup updates the vcpkg.exe and solves potential issues with VS Build Tools not being found + & "$GitRoot\vcpkg\bootstrap-vcpkg.bat" } function Set-GitHooks { diff --git a/scripts/pip.requirements.txt b/scripts/pip.requirements.txt index 377f1e824fa..bae1499f627 100644 --- a/scripts/pip.requirements.txt +++ b/scripts/pip.requirements.txt @@ -1,2 +1,2 @@ -clang-format==18.1.5 +clang-format==19.1.1 cmake-format==0.6.13 \ No newline at end of file