Skip to content

Commit d8dc05a

Browse files
authored
Enable source reference to get thread globals for off-thread logging (microsoft#5780)
## Change Allow `ISourceReference` and `Source` to have `ThreadGlobals` provided so that they can use them for other-thread logging. Enable this for the thread async REST source so that our certificate pinning callback can log results.
1 parent b7cc9bf commit d8dc05a

File tree

10 files changed

+49
-11
lines changed

10 files changed

+49
-11
lines changed

src/AppInstallerCLICore/ExecutionContext.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ namespace AppInstaller::CLI::Execution
5959

6060
std::unique_ptr<Context> Context::CreateSubContext()
6161
{
62-
auto clone = std::make_unique<Context>(Reporter, m_threadGlobals);
62+
auto clone = std::make_unique<Context>(Reporter, *m_threadGlobals);
6363
clone->m_flags = m_flags;
6464
clone->m_executingCommand = m_executingCommand;
6565
// If the parent is hooked up to the CTRL signal, have the clone be as well
@@ -210,13 +210,18 @@ namespace AppInstaller::CLI::Execution
210210
}
211211

212212
AppInstaller::ThreadLocalStorage::WingetThreadGlobals& Context::GetThreadGlobals()
213+
{
214+
return *m_threadGlobals;
215+
}
216+
217+
std::shared_ptr<ThreadLocalStorage::WingetThreadGlobals> Context::GetSharedThreadGlobals()
213218
{
214219
return m_threadGlobals;
215220
}
216221

217222
std::unique_ptr<AppInstaller::ThreadLocalStorage::PreviousThreadGlobals> Context::SetForCurrentThread()
218223
{
219-
return m_threadGlobals.SetForCurrentThread();
224+
return m_threadGlobals->SetForCurrentThread();
220225
}
221226

222227
#ifndef AICLI_DISABLE_TEST_HOOKS

src/AppInstallerCLICore/ExecutionContext.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ namespace AppInstaller::CLI::Execution
100100
// Constructor for creating a sub-context.
101101
Context(Execution::Reporter& reporter, ThreadLocalStorage::WingetThreadGlobals& threadGlobals) :
102102
Reporter(reporter, Execution::Reporter::clone_t{}),
103-
m_threadGlobals(threadGlobals, ThreadLocalStorage::WingetThreadGlobals::create_sub_thread_globals_t{}) {}
103+
m_threadGlobals(std::make_shared<ThreadLocalStorage::WingetThreadGlobals>(threadGlobals, ThreadLocalStorage::WingetThreadGlobals::create_sub_thread_globals_t{})) {}
104104

105105
virtual ~Context();
106106

@@ -163,7 +163,8 @@ namespace AppInstaller::CLI::Execution
163163
virtual void SetExecutionStage(Workflow::ExecutionStage stage);
164164

165165
// Get Globals for Current Context
166-
AppInstaller::ThreadLocalStorage::WingetThreadGlobals& GetThreadGlobals();
166+
ThreadLocalStorage::WingetThreadGlobals& GetThreadGlobals();
167+
std::shared_ptr<ThreadLocalStorage::WingetThreadGlobals> GetSharedThreadGlobals();
167168

168169
std::unique_ptr<AppInstaller::ThreadLocalStorage::PreviousThreadGlobals> SetForCurrentThread();
169170

@@ -209,7 +210,7 @@ namespace AppInstaller::CLI::Execution
209210
size_t m_CtrlSignalCount = 0;
210211
ContextFlag m_flags = ContextFlag::None;
211212
Workflow::ExecutionStage m_executionStage = Workflow::ExecutionStage::Initial;
212-
AppInstaller::ThreadLocalStorage::WingetThreadGlobals m_threadGlobals;
213+
std::shared_ptr<ThreadLocalStorage::WingetThreadGlobals> m_threadGlobals = std::make_shared<ThreadLocalStorage::WingetThreadGlobals>();
213214
AppInstaller::CLI::Command* m_executingCommand = nullptr;
214215
std::unique_ptr<AppInstaller::Checkpoints::CheckpointManager> m_checkpointManager;
215216
};

src/AppInstallerCLICore/Workflows/WorkflowBase.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ namespace AppInstaller::CLI::Workflow
185185
{
186186
source.SetCaller("winget-cli");
187187
source.SetAuthenticationArguments(GetAuthenticationArguments(context));
188+
source.SetThreadGlobals(context.GetSharedThreadGlobals());
188189
return source.Open(progress);
189190
};
190191
auto updateFailures = context.Reporter.ExecuteWithProgress(openFunction, true);

src/AppInstallerCommonCore/HttpClientHelper.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,14 @@ namespace AppInstaller::Http
2222
}
2323
}
2424

25-
void NativeHandleServerCertificateValidation(web::http::client::native_handle handle, const Certificates::PinningConfiguration& pinningConfiguration)
25+
void NativeHandleServerCertificateValidation(web::http::client::native_handle handle, const Certificates::PinningConfiguration& pinningConfiguration, ThreadLocalStorage::ThreadGlobals* threadGlobals)
2626
{
27+
decltype(threadGlobals->SetForCurrentThread()) previousThreadGlobals;
28+
if (threadGlobals)
29+
{
30+
previousThreadGlobals = threadGlobals->SetForCurrentThread();
31+
}
32+
2733
HINTERNET requestHandle = reinterpret_cast<HINTERNET>(handle);
2834

2935
// Get certificate and pass along to pinning config
@@ -176,11 +182,11 @@ namespace AppInstaller::Http
176182
RethrowAsWilException(exception);
177183
}
178184

179-
void HttpClientHelper::SetPinningConfiguration(const Certificates::PinningConfiguration& configuration)
185+
void HttpClientHelper::SetPinningConfiguration(const Certificates::PinningConfiguration& configuration, std::shared_ptr<ThreadLocalStorage::ThreadGlobals> threadGlobals)
180186
{
181-
m_clientConfig.set_nativehandle_servercertificate_validation([pinConfig = configuration](web::http::client::native_handle handle)
187+
m_clientConfig.set_nativehandle_servercertificate_validation([pinConfig = configuration, globals = std::move(threadGlobals)](web::http::client::native_handle handle)
182188
{
183-
NativeHandleServerCertificateValidation(handle, pinConfig);
189+
NativeHandleServerCertificateValidation(handle, pinConfig, globals.get());
184190
});
185191
}
186192

src/AppInstallerCommonCore/Public/winget/HttpClientHelper.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
// Licensed under the MIT License.
33
#pragma once
44
#include <winget/Certificates.h>
5+
#include <winget/SharedThreadGlobals.h>
56
#include <cpprest/http_client.h>
67
#include <cpprest/json.h>
8+
#include <memory>
79
#include <optional>
810
#include <vector>
911

@@ -34,7 +36,7 @@ namespace AppInstaller::Http
3436

3537
std::optional<web::json::value> HandleGet(const utility::string_t& uri, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}, const HttpResponseHandler& customHandler = {}) const;
3638

37-
void SetPinningConfiguration(const Certificates::PinningConfiguration& configuration);
39+
void SetPinningConfiguration(const Certificates::PinningConfiguration& configuration, std::shared_ptr<ThreadLocalStorage::ThreadGlobals> threadGlobals = {});
3840

3941
protected:
4042
std::optional<web::json::value> ValidateAndExtractResponse(const web::http::http_response& response) const;

src/AppInstallerRepositoryCore/ISource.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33
#pragma once
44
#include "Public/winget/RepositorySource.h"
5+
#include <winget/SharedThreadGlobals.h>
56
#include <memory>
67

78
namespace AppInstaller::Repository
@@ -96,6 +97,9 @@ namespace AppInstaller::Repository
9697

9798
// Opens the source. This function should throw upon open failure rather than returning an empty pointer.
9899
virtual std::shared_ptr<ISource> Open(IProgressCallback& progress) = 0;
100+
101+
// Sets thread globals for the source. This allows the option for sources that operate on other threads to log properly.
102+
virtual void SetThreadGlobals(const std::shared_ptr<ThreadLocalStorage::ThreadGlobals>&) {}
99103
};
100104

101105
// Internal interface extension to ISource for databases that can be updated after creation, like InstallingPackages

src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <AppInstallerProgress.h>
77
#include <winget/Certificates.h>
88
#include <winget/Authentication.h>
9+
#include <winget/SharedThreadGlobals.h>
910

1011
#include <chrono>
1112
#include <filesystem>
@@ -275,6 +276,9 @@ namespace AppInstaller::Repository
275276
// Set authentication arguments. Must be set before Open to have effect.
276277
void SetAuthenticationArguments(Authentication::AuthenticationArguments args);
277278

279+
// Set thread globals. Must be set before Open to have effect.
280+
void SetThreadGlobals(const std::shared_ptr<ThreadLocalStorage::ThreadGlobals>& threadGlobals);
281+
278282
// Set background update check interval.
279283
void SetBackgroundUpdateInterval(TimeSpan interval);
280284

src/AppInstallerRepositoryCore/RepositorySource.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,14 @@ namespace AppInstaller::Repository
680680
}
681681
}
682682

683+
void Source::SetThreadGlobals(const std::shared_ptr<ThreadLocalStorage::ThreadGlobals>& threadGlobals)
684+
{
685+
for (auto& sourceReference : m_sourceReferences)
686+
{
687+
sourceReference->SetThreadGlobals(threadGlobals);
688+
}
689+
}
690+
683691
void Source::SetBackgroundUpdateInterval(TimeSpan interval)
684692
{
685693
m_backgroundUpdateInterval = interval;

src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,18 @@ namespace AppInstaller::Repository::Rest
5454
return std::make_shared<RestSource>(m_details, m_information, std::move(restClient));
5555
}
5656

57+
void SetThreadGlobals(const std::shared_ptr<ThreadLocalStorage::ThreadGlobals>& threadGlobals) override
58+
{
59+
m_threadGlobals = threadGlobals;
60+
}
61+
5762
private:
5863
void Initialize()
5964
{
6065
std::call_once(m_initializeFlag,
6166
[&]()
6267
{
63-
m_httpClientHelper.SetPinningConfiguration(m_details.CertificatePinningConfiguration);
68+
m_httpClientHelper.SetPinningConfiguration(m_details.CertificatePinningConfiguration, m_threadGlobals);
6469
m_restClientInformation = RestClient::GetInformation(m_details.Arg, m_customHeader, m_caller, m_httpClientHelper);
6570

6671
m_details.Identifier = m_restClientInformation.SourceIdentifier;
@@ -88,6 +93,7 @@ namespace AppInstaller::Repository::Rest
8893
std::string m_caller;
8994
Authentication::AuthenticationArguments m_authArgs;
9095
std::once_flag m_initializeFlag;
96+
std::shared_ptr<ThreadLocalStorage::ThreadGlobals> m_threadGlobals;
9197
};
9298

9399
// The base class for data that comes from a rest based source.

src/AppInstallerSharedLib/Certificates.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,7 @@ namespace AppInstaller::Certificates
650650
{
651651
if (chain.Validate(chainContext.get()))
652652
{
653+
AICLI_LOG(Core, Verbose, << "Certificate `" << GetSimpleDisplayName(certContext) << "` accepted by pinning configuration:\n" << chain.GetDescription());
653654
result = true;
654655
break;
655656
}

0 commit comments

Comments
 (0)