From 48c2def26f651a842d071b03cb077aa09acde3d6 Mon Sep 17 00:00:00 2001 From: nidietr_MSFT Date: Fri, 6 Dec 2024 20:07:23 +0100 Subject: [PATCH] Squashed 'src/SfsClient/sfs-client/' changes from be733af9..c639a506 c639a506 Adding support for a custom proxy input (#218) 258d189b Improve logging when the content type is wrong (#221) 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: c639a506e712dbd29ca7ca0c78d5216658e78748 --- .../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/src/details/entity/ContentType.cpp | 21 +-- client/src/details/entity/ContentType.h | 6 +- client/src/details/entity/FileEntity.cpp | 14 +- client/src/details/entity/VersionEntity.cpp | 15 +- client/tests/CMakeLists.txt | 2 + client/tests/functional/SFSClientTests.cpp | 25 ++- .../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 +++++ .../unit/details/entity/FileEntityTests.cpp | 8 +- scripts/Setup.ps1 | 41 +++-- scripts/pip.requirements.txt | 2 +- 26 files changed, 621 insertions(+), 205 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 0000000000..990b0603bf --- /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 1d12f11723..59ee8521c9 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 1a57d5689e..0c2a6cf653 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 8b62920943..f3740d36c4 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 a1a1b7e855..b2961f5c99 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 a1a1b10ef1..95d2abd8f5 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 f0b786defc..cf738dab1e 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 7279ccf5aa..319d5a132c 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 64fb42645b..30d963ae9f 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 354dcab3b7..c7c0683141 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/src/details/entity/ContentType.cpp b/client/src/details/entity/ContentType.cpp index b13a4e0789..ce5680f166 100644 --- a/client/src/details/entity/ContentType.cpp +++ b/client/src/details/entity/ContentType.cpp @@ -3,16 +3,9 @@ #include "ContentType.h" -#include "../ErrorHandling.h" -#include "../ReportingHandler.h" -#include "Result.h" - -using namespace SFS; using namespace SFS::details; -namespace -{ -std::string ToString(ContentType type) +std::string SFS::details::ToString(ContentType type) { switch (type) { @@ -24,15 +17,3 @@ std::string ToString(ContentType type) return "Unknown"; } } -} // namespace - -void SFS::details::ValidateContentType(ContentType currentType, - ContentType expectedType, - const ReportingHandler& handler) -{ - THROW_CODE_IF_LOG(Result::ServiceUnexpectedContentType, - currentType != expectedType, - handler, - "Unexpected content type [" + ::ToString(currentType) + - "] returned by the service does not match the expected [" + ::ToString(expectedType) + "]"); -} diff --git a/client/src/details/entity/ContentType.h b/client/src/details/entity/ContentType.h index 65c6e18250..2b9ccc4aa5 100644 --- a/client/src/details/entity/ContentType.h +++ b/client/src/details/entity/ContentType.h @@ -3,15 +3,15 @@ #pragma once +#include + namespace SFS::details { -class ReportingHandler; - enum class ContentType { Generic, App, }; -void ValidateContentType(ContentType currentType, ContentType expectedType, const ReportingHandler& handler); +std::string ToString(ContentType type); } // namespace SFS::details diff --git a/client/src/details/entity/FileEntity.cpp b/client/src/details/entity/FileEntity.cpp index a427d793a9..53e8f114e7 100644 --- a/client/src/details/entity/FileEntity.cpp +++ b/client/src/details/entity/FileEntity.cpp @@ -66,6 +66,16 @@ Architecture ArchitectureFromString(const std::string& arch, const ReportingHand return Architecture::None; // Unreachable code, but the compiler doesn't know that. } } + +void ValidateContentType(const FileEntity& entity, ContentType expectedType, const ReportingHandler& handler) +{ + THROW_CODE_IF_LOG(Result::ServiceUnexpectedContentType, + entity.GetContentType() != expectedType, + handler, + "The service returned file \"" + entity.fileId + "\" with content type [" + + ToString(entity.GetContentType()) + "] while the expected type was [" + + ToString(expectedType) + "]"); +} } // namespace std::unique_ptr FileEntity::FromJson(const nlohmann::json& file, const ReportingHandler& handler) @@ -204,7 +214,7 @@ ContentType GenericFileEntity::GetContentType() const std::unique_ptr GenericFileEntity::ToFile(FileEntity&& entity, const ReportingHandler& handler) { - ValidateContentType(entity.GetContentType(), ContentType::Generic, handler); + ValidateContentType(entity, ContentType::Generic, handler); std::unordered_map hashes; for (auto& [hashType, hashValue] : entity.hashes) @@ -237,7 +247,7 @@ ContentType AppFileEntity::GetContentType() const std::unique_ptr AppFileEntity::ToAppFile(FileEntity&& entity, const ReportingHandler& handler) { - ValidateContentType(entity.GetContentType(), ContentType::App, handler); + ValidateContentType(entity, ContentType::App, handler); auto appEntity = dynamic_cast(entity); diff --git a/client/src/details/entity/VersionEntity.cpp b/client/src/details/entity/VersionEntity.cpp index f282fe1ab3..eacd847e88 100644 --- a/client/src/details/entity/VersionEntity.cpp +++ b/client/src/details/entity/VersionEntity.cpp @@ -16,6 +16,19 @@ using namespace SFS; using namespace SFS::details; using json = nlohmann::json; +namespace +{ +void ValidateContentType(const VersionEntity& entity, ContentType expectedType, const ReportingHandler& handler) +{ + THROW_CODE_IF_LOG(Result::ServiceUnexpectedContentType, + entity.GetContentType() != expectedType, + handler, + "The service returned entity \"" + entity.contentId.name + "\" with content type [" + + ToString(entity.GetContentType()) + "] while the expected type was [" + + ToString(expectedType) + "]"); +} +} // namespace + std::unique_ptr VersionEntity::FromJson(const nlohmann::json& data, const ReportingHandler& handler) { // Expected format for a generic version entity: @@ -135,6 +148,6 @@ ContentType AppVersionEntity::GetContentType() const AppVersionEntity* AppVersionEntity::GetAppVersionEntityPtr(std::unique_ptr& versionEntity, const ReportingHandler& handler) { - ValidateContentType(versionEntity->GetContentType(), ContentType::App, handler); + ValidateContentType(*versionEntity, ContentType::App, handler); return dynamic_cast(versionEntity.get()); } diff --git a/client/tests/CMakeLists.txt b/client/tests/CMakeLists.txt index 91f4fe0a13..5782701e10 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 5ec016a88f..f5b90bc942 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()") @@ -240,10 +256,13 @@ TEST("Testing SFSClient::GetLatestAppDownloadInfo()") params.productRequests = {{c_productName, {}}}; auto result = sfsClient->GetLatestAppDownloadInfo(params, contents); REQUIRE(result.GetCode() == Result::ServiceUnexpectedContentType); - REQUIRE(result.GetMsg() == - "Unexpected content type [Generic] returned by the service does not match the expected [App]"); + REQUIRE( + result.GetMsg() == + R"(The service returned entity "testProduct" with content type [Generic] while the expected type was [App])"); REQUIRE(contents.empty()); } + + REQUIRE(server.Stop() == Result::Success); } TEST("Testing SFSClient retry behavior") @@ -437,4 +456,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 1d8a21ad68..77c114395a 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 b077dbdc16..be085ac05d 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 0000000000..2ee330a338 --- /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 0000000000..eb833b2985 --- /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 0000000000..bdf3bb2d3d --- /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 0000000000..bd3cca9fcd --- /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 496482b97e..33cc0d5cc3 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 377f1e824f..bae1499f62 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