Skip to content

Commit

Permalink
Squashed 'src/SfsClient/sfs-client/' changes from be733af9..c639a506
Browse files Browse the repository at this point in the history
c639a506 Adding support for a custom proxy input (microsoft#218)
258d189b Improve logging when the content type is wrong (microsoft#221)
216210ab Adding required permissions to enable uploading of CodeQL results (microsoft#214)
fb953d6e Bump github/codeql-action from 2 to 3 (microsoft#215)
52af7124 Enabling CodeQL scanning (microsoft#211)
e555d764 Bump clang-format from 18.1.5 to 19.1.1 (microsoft#210)
ab8f0e72 Setup: improving build tools installation (microsoft#207)

git-subtree-dir: src/SfsClient/sfs-client
git-subtree-split: c639a506e712dbd29ca7ca0c78d5216658e78748
  • Loading branch information
nidietr-MSFT committed Dec 6, 2024
1 parent d5ae2a9 commit 48c2def
Show file tree
Hide file tree
Showing 26 changed files with 621 additions and 205 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/initialize-codeql/action.yml
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions .github/workflows/main-build-ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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

Expand All @@ -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
10 changes: 9 additions & 1 deletion .github/workflows/main-build-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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
Expand All @@ -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
14 changes: 13 additions & 1 deletion .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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
5 changes: 5 additions & 0 deletions client/include/sfsclient/RequestParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ struct RequestParams
/// @note If not provided, a new CorrelationVector will be generated
std::optional<std::string> 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<std::string> proxy;

/// @brief Retry for a web request after a failed attempt. If true, client will retry up to c_maxRetries times
bool retryOnError{true};
};
Expand Down
25 changes: 25 additions & 0 deletions client/src/details/UrlBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions client/src/details/UrlBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions client/src/details/connection/ConnectionConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}
3 changes: 3 additions & 0 deletions client/src/details/connection/ConnectionConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ struct ConnectionConfig

/// @brief The correlation vector to use for requests
std::optional<std::string> baseCV;

/// @brief Proxy setting which can be used to establish connections with the server
std::optional<std::string> proxy;
};
} // namespace details
} // namespace SFS
5 changes: 5 additions & 0 deletions client/src/details/connection/CurlConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
21 changes: 1 addition & 20 deletions client/src/details/entity/ContentType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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) + "]");
}
6 changes: 3 additions & 3 deletions client/src/details/entity/ContentType.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@

#pragma once

#include <string>

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
14 changes: 12 additions & 2 deletions client/src/details/entity/FileEntity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> FileEntity::FromJson(const nlohmann::json& file, const ReportingHandler& handler)
Expand Down Expand Up @@ -204,7 +214,7 @@ ContentType GenericFileEntity::GetContentType() const

std::unique_ptr<File> GenericFileEntity::ToFile(FileEntity&& entity, const ReportingHandler& handler)
{
ValidateContentType(entity.GetContentType(), ContentType::Generic, handler);
ValidateContentType(entity, ContentType::Generic, handler);

std::unordered_map<HashType, std::string> hashes;
for (auto& [hashType, hashValue] : entity.hashes)
Expand Down Expand Up @@ -237,7 +247,7 @@ ContentType AppFileEntity::GetContentType() const

std::unique_ptr<AppFile> AppFileEntity::ToAppFile(FileEntity&& entity, const ReportingHandler& handler)
{
ValidateContentType(entity.GetContentType(), ContentType::App, handler);
ValidateContentType(entity, ContentType::App, handler);

auto appEntity = dynamic_cast<AppFileEntity&&>(entity);

Expand Down
15 changes: 14 additions & 1 deletion client/src/details/entity/VersionEntity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> VersionEntity::FromJson(const nlohmann::json& data, const ReportingHandler& handler)
{
// Expected format for a generic version entity:
Expand Down Expand Up @@ -135,6 +148,6 @@ ContentType AppVersionEntity::GetContentType() const
AppVersionEntity* AppVersionEntity::GetAppVersionEntityPtr(std::unique_ptr<VersionEntity>& versionEntity,
const ReportingHandler& handler)
{
ValidateContentType(versionEntity->GetContentType(), ContentType::App, handler);
ValidateContentType(*versionEntity, ContentType::App, handler);
return dynamic_cast<AppVersionEntity*>(versionEntity.get());
}
2 changes: 2 additions & 0 deletions client/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 23 additions & 2 deletions client/tests/functional/SFSClientTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"}};
Expand Down Expand Up @@ -143,6 +157,8 @@ TEST("Testing SFSClient::GetLatestDownloadInfo()")
CheckMockContent(contents[0], c_nextVersion);
}
}

REQUIRE(server.Stop() == Result::Success);
}

TEST("Testing SFSClient::GetLatestAppDownloadInfo()")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -437,4 +456,6 @@ TEST("Testing SFSClient retry behavior")
}
}
}

REQUIRE(server.Stop() == Result::Success);
}
Loading

0 comments on commit 48c2def

Please sign in to comment.