From c7927e616ff306a9764a12a23211fa575dcf07f1 Mon Sep 17 00:00:00 2001 From: Nicolas Dietrich Date: Mon, 2 Dec 2024 19:27:18 +0100 Subject: [PATCH 1/7] Adds SFS Client Proxy support Requires ingesting sfs-client update from https://github.com/microsoft/sfs-client/pull/218 --- src/AppInstallerCommonCore/MSStoreDownload.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp index 4c7dc63253..a4acad13ac 100644 --- a/src/AppInstallerCommonCore/MSStoreDownload.cpp +++ b/src/AppInstallerCommonCore/MSStoreDownload.cpp @@ -12,6 +12,7 @@ #include "winget/Rest.h" #include "winget/HttpClientHelper.h" #include "winget/UserSettings.h" +#include "winget/NetworkSettings.h" #ifndef WINGET_DISABLE_FOR_FUZZING #include #endif @@ -910,6 +911,13 @@ namespace AppInstaller::MSStore { SFS::RequestParams sfsClientRequest; sfsClientRequest.productRequests = { {std::string{ wuCategoryId }, {}} }; + if (AppInstaller::Settings::Network().GetProxyUri()) + { + std::string proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); + + sfsClientRequest.proxy = proxyUri; + AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << proxyUri); + } auto requestResult = GetSfsClientInstance()->GetLatestAppDownloadInfo(sfsClientRequest, appContents); if (!requestResult) From e5fed4c6bef04c184f3bd547412513e658937533 Mon Sep 17 00:00:00 2001 From: nidietr_MSFT Date: Mon, 2 Dec 2024 21:38:02 +0100 Subject: [PATCH 2/7] Adds SFS Client Proxy support Requires ingesting sfs-client update including https://github.com/microsoft/sfs-client/pull/218 --- src/AppInstallerCommonCore/MSStoreDownload.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp index 4c7dc63253..a4acad13ac 100644 --- a/src/AppInstallerCommonCore/MSStoreDownload.cpp +++ b/src/AppInstallerCommonCore/MSStoreDownload.cpp @@ -12,6 +12,7 @@ #include "winget/Rest.h" #include "winget/HttpClientHelper.h" #include "winget/UserSettings.h" +#include "winget/NetworkSettings.h" #ifndef WINGET_DISABLE_FOR_FUZZING #include #endif @@ -910,6 +911,13 @@ namespace AppInstaller::MSStore { SFS::RequestParams sfsClientRequest; sfsClientRequest.productRequests = { {std::string{ wuCategoryId }, {}} }; + if (AppInstaller::Settings::Network().GetProxyUri()) + { + std::string proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); + + sfsClientRequest.proxy = proxyUri; + AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << proxyUri); + } auto requestResult = GetSfsClientInstance()->GetLatestAppDownloadInfo(sfsClientRequest, appContents); if (!requestResult) From 5426ef4a368e58bb84739b174c990a9c10f9a0b8 Mon Sep 17 00:00:00 2001 From: nidietr_MSFT Date: Fri, 6 Dec 2024 19:35:51 +0100 Subject: [PATCH 3/7] Fixed GetProxyUri() to return a reference --- src/AppInstallerCommonCore/MSStoreDownload.cpp | 2 +- src/AppInstallerCommonCore/Public/winget/NetworkSettings.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp index a4acad13ac..f9dc355d99 100644 --- a/src/AppInstallerCommonCore/MSStoreDownload.cpp +++ b/src/AppInstallerCommonCore/MSStoreDownload.cpp @@ -913,7 +913,7 @@ namespace AppInstaller::MSStore sfsClientRequest.productRequests = { {std::string{ wuCategoryId }, {}} }; if (AppInstaller::Settings::Network().GetProxyUri()) { - std::string proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); + const auto& proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); sfsClientRequest.proxy = proxyUri; AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << proxyUri); diff --git a/src/AppInstallerCommonCore/Public/winget/NetworkSettings.h b/src/AppInstallerCommonCore/Public/winget/NetworkSettings.h index 856055fcc8..ea90ab7f1d 100644 --- a/src/AppInstallerCommonCore/Public/winget/NetworkSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/NetworkSettings.h @@ -16,7 +16,7 @@ namespace AppInstaller::Settings { static NetworkSettings& Instance(); - const std::optional GetProxyUri() const { return m_proxyUri; } + const std::optional& GetProxyUri() const { return m_proxyUri; } // Sets the proxy URI; may do nothing depending on admin settings and group policy void SetProxyUri(const std::optional& proxyUri); @@ -30,4 +30,4 @@ namespace AppInstaller::Settings }; NetworkSettings& Network(); -} \ No newline at end of file +} From 5d5dd308a0b367b327039694689ac6618b0b309f Mon Sep 17 00:00:00 2001 From: Nicolas Dietrich Date: Tue, 10 Dec 2024 12:15:15 +0100 Subject: [PATCH 4/7] Squashed 'src/SfsClient/sfs-client/' changes from be733af9..0e27525d 0e27525d Bumping version to 1.1.0 (#222) 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: 0e27525d597c730e71646fd0b15bdc8c8503f24d --- .../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 +- CMakeLists.txt | 2 +- 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 +- 27 files changed, 622 insertions(+), 206 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/CMakeLists.txt b/CMakeLists.txt index 0d33a25954..31bb1010cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,7 +27,7 @@ endif() # 1. MAJOR version when you make incompatible API changes # 2. MINOR version when you add functionality in a backward compatible manner # 3. PATCH version when you make backward compatible bug fixes -set(SFS_LIBRARY_VERSION "1.0.0") +set(SFS_LIBRARY_VERSION "1.1.0") project( sfsclient 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 From 3d1402c38b5ecd1cf2c65dbb80f173cc9f47cc90 Mon Sep 17 00:00:00 2001 From: Nicolas Dietrich Date: Tue, 10 Dec 2024 12:38:58 +0100 Subject: [PATCH 5/7] Update readme.md & SfsClient project files for SfsClient 1.1.0 --- src/SfsClient/SfsClient.vcxproj | 24 ++++++++++++------------ src/SfsClient/readme.md | 9 +++++++-- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/SfsClient/SfsClient.vcxproj b/src/SfsClient/SfsClient.vcxproj index 5f1e0d4801..4135d5eefb 100644 --- a/src/SfsClient/SfsClient.vcxproj +++ b/src/SfsClient/SfsClient.vcxproj @@ -241,7 +241,7 @@ - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";WIN32;_DEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";WIN32;_DEBUG;%(PreprocessorDefinitions) TurnOffAllWarnings ProgramDatabase stdcpp17 @@ -257,7 +257,7 @@ - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";WIN32;NDEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";WIN32;NDEBUG;%(PreprocessorDefinitions) Level3 ProgramDatabase stdcpp17 @@ -274,7 +274,7 @@ - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";WIN32;NDEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";WIN32;NDEBUG;%(PreprocessorDefinitions) Level3 ProgramDatabase stdcpp17 @@ -294,7 +294,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true @@ -304,7 +304,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) MultiThreaded @@ -315,7 +315,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true @@ -325,7 +325,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) MultiThreaded @@ -336,7 +336,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true @@ -346,7 +346,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";NDEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) Level3 $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) MultiThreaded @@ -357,7 +357,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";_DEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";_DEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) TurnOffAllWarnings $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true @@ -367,7 +367,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";_DEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";_DEBUG;%(PreprocessorDefinitions)%(PreprocessorDefinitions) TurnOffAllWarnings $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true @@ -377,7 +377,7 @@ stdcpp17 NotUsing - CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.0.0";_DEBUG;%(PreprocessorDefinitions) + CURL_STATICLIB;GUID_WINDOWS;SFS_VERSION="1.1.0";_DEBUG;%(PreprocessorDefinitions) TurnOffAllWarnings $(ProjectDir)sfs-client\client\include\sfsclient;%(AdditionalIncludeDirectories) true diff --git a/src/SfsClient/readme.md b/src/SfsClient/readme.md index 382a316136..84fcea431b 100644 --- a/src/SfsClient/readme.md +++ b/src/SfsClient/readme.md @@ -1,10 +1,15 @@ ## SfsClient -Do not change code under the sfs-client directory; it contains sfs-client source code from commit [be733af](https://github.com/microsoft/sfs-client/commits/be733af). -It is created using git subtree command: +Do not change code under the sfs-client directory; it contains sfs-client source code from release 1.1.0 (https://github.com/microsoft/sfs-client/releases/tag/1.1.0). +It was initially created using git subtree command: ``` git subtree add --prefix=src/SfsClient/sfs-client https://github.com/microsoft/sfs-client.git be733af9e5c8e9227f2018ff618800bf08a31180 --squash ``` +Then updated to release 1.1.0 using: +``` + git subtree pull -P src/SfsClient/sfs-client https://github.com/microsoft/sfs-client 1.1.0 --squash +``` + ### Update To update, run the following command, then update the above commit for reference. 'master' can be replaced with the appropriate commit spec as desired. From 7f6eb7504f5e5c92aaa9c1b49c228f887791fb4a Mon Sep 17 00:00:00 2001 From: Nicolas Dietrich Date: Tue, 10 Dec 2024 14:41:32 +0100 Subject: [PATCH 6/7] Logging before effectively setting the proxy. --- src/AppInstallerCommonCore/MSStoreDownload.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp index a4acad13ac..1791468464 100644 --- a/src/AppInstallerCommonCore/MSStoreDownload.cpp +++ b/src/AppInstallerCommonCore/MSStoreDownload.cpp @@ -915,8 +915,8 @@ namespace AppInstaller::MSStore { std::string proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); - sfsClientRequest.proxy = proxyUri; AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << proxyUri); + sfsClientRequest.proxy = proxyUri; } auto requestResult = GetSfsClientInstance()->GetLatestAppDownloadInfo(sfsClientRequest, appContents); From 4f0550821cccf2a2edfb6bb3251bfa738ed3f596 Mon Sep 17 00:00:00 2001 From: Nicolas Dietrich Date: Tue, 10 Dec 2024 20:03:30 +0100 Subject: [PATCH 7/7] Review comments: a) alphabetic order for includes, b) access the std::optional once --- src/AppInstallerCommonCore/MSStoreDownload.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp index bdc2dbda80..dc4f409949 100644 --- a/src/AppInstallerCommonCore/MSStoreDownload.cpp +++ b/src/AppInstallerCommonCore/MSStoreDownload.cpp @@ -6,13 +6,13 @@ #include #include "AppInstallerMsixInfo.h" #include "AppInstallerRuntime.h" -#include "winget/Locale.h" +#include "winget/HttpClientHelper.h" #include "winget/JsonUtil.h" +#include "winget/Locale.h" #include "winget/MSStoreDownload.h" +#include "winget/NetworkSettings.h" #include "winget/Rest.h" -#include "winget/HttpClientHelper.h" #include "winget/UserSettings.h" -#include "winget/NetworkSettings.h" #ifndef WINGET_DISABLE_FOR_FUZZING #include #endif @@ -911,11 +911,11 @@ namespace AppInstaller::MSStore { SFS::RequestParams sfsClientRequest; sfsClientRequest.productRequests = { {std::string{ wuCategoryId }, {}} }; - if (AppInstaller::Settings::Network().GetProxyUri()) + const auto& proxyUri = AppInstaller::Settings::Network().GetProxyUri(); + if (proxyUri) { - const auto& proxyUri = AppInstaller::Settings::Network().GetProxyUri().value(); - AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << proxyUri); - sfsClientRequest.proxy = proxyUri; + AICLI_LOG(Core, Info, << "Passing proxy to SFS client " << *proxyUri); + sfsClientRequest.proxy = *proxyUri; } auto requestResult = GetSfsClientInstance()->GetLatestAppDownloadInfo(sfsClientRequest, appContents);