Skip to content

libstore: make FileTransfer injectable into HttpBinaryCacheStore #15206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ericson2314 merged 1 commit into from
Feb 12, 2026
Merged

libstore: make FileTransfer injectable into HttpBinaryCacheStore #15206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions src/libstore-test-support/https-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ void TestHttpBinaryCacheStore::init()
BinaryCacheStore::init();
}

ref<TestHttpBinaryCacheStore> TestHttpBinaryCacheStoreConfig::openTestStore() const
ref<TestHttpBinaryCacheStore> TestHttpBinaryCacheStoreConfig::openTestStore(ref<FileTransfer> fileTransfer) const
{
auto store = make_ref<TestHttpBinaryCacheStore>(
ref{// FIXME we shouldn't actually need a mutable config
std::const_pointer_cast<HttpBinaryCacheStore::Config>(shared_from_this())});
std::const_pointer_cast<HttpBinaryCacheStore::Config>(shared_from_this())},
fileTransfer);
store->init();
return store;
}
Expand Down Expand Up @@ -79,16 +80,18 @@ void HttpsBinaryCacheStoreTest::SetUp()
for the port explicitly - this is enough. */
std::this_thread::sleep_for(std::chrono::milliseconds(50));

/* FIXME: Don't use global settings. Tests are not run concurrently, so this is fine for now. */
oldCaCert = fileTransferSettings.caFile;
fileTransferSettings.caFile = caCert.string();
/* Create custom FileTransferSettings with our test CA certificate.
This avoids mutating global settings. */
testFileTransferSettings = std::make_unique<FileTransferSettings>();
testFileTransferSettings->caFile = caCert;
testFileTransfer = makeFileTransfer(*testFileTransferSettings);
}

void HttpsBinaryCacheStoreTest::TearDown()
{
fileTransferSettings.caFile = oldCaCert;
serverPid.kill();
delTmpDir.reset();
testFileTransferSettings.reset();
}

std::vector<std::string> HttpsBinaryCacheStoreTest::serverArgs()
Expand All @@ -115,11 +118,17 @@ std::vector<std::string> HttpsBinaryCacheStoreMtlsTest::serverArgs()
return args;
}

ref<TestHttpBinaryCacheStoreConfig> HttpsBinaryCacheStoreTest::makeConfig(BinaryCacheStoreConfig::Params params)
ref<TestHttpBinaryCacheStoreConfig> HttpsBinaryCacheStoreTest::makeConfig()
{
auto res = make_ref<TestHttpBinaryCacheStoreConfig>("https", fmt("localhost:%d", port), std::move(params));
auto res = make_ref<TestHttpBinaryCacheStoreConfig>(
"https", fmt("localhost:%d", port), TestHttpBinaryCacheStoreConfig::Params{});
res->pathInfoCacheSize = 0; /* We don't want any caching in tests. */
return res;
}

ref<TestHttpBinaryCacheStore> HttpsBinaryCacheStoreTest::openStore(ref<TestHttpBinaryCacheStoreConfig> config)
{
return config->openTestStore(ref<FileTransfer>{testFileTransfer});
}

} // namespace nix::testing
27 changes: 17 additions & 10 deletions src/libstore-test-support/include/nix/store/tests/https-store.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ public:
TestHttpBinaryCacheStore & operator=(const TestHttpBinaryCacheStore &) = delete;
TestHttpBinaryCacheStore & operator=(TestHttpBinaryCacheStore &&) = delete;

TestHttpBinaryCacheStore(ref<HttpBinaryCacheStoreConfig> config)
TestHttpBinaryCacheStore(ref<HttpBinaryCacheStoreConfig> config, ref<FileTransfer> fileTransfer)
: Store{*config}
, BinaryCacheStore{*config}
, HttpBinaryCacheStore(config)
, HttpBinaryCacheStore(config, fileTransfer)
{
diskCache = nullptr; /* Disable caching, we'll be creating a new binary cache for each test. */
}
Expand All @@ -49,12 +49,7 @@ public:
{
}

ref<TestHttpBinaryCacheStore> openTestStore() const;

ref<Store> openStore() const override
{
return openTestStore();
}
ref<TestHttpBinaryCacheStore> openTestStore(ref<FileTransfer> fileTransfer) const;
};

class HttpsBinaryCacheStoreTest : public virtual LibStoreNetworkTest
Expand All @@ -71,17 +66,29 @@ protected:
std::filesystem::path tmpDir, cacheDir;
std::filesystem::path caCert, caKey, serverCert, serverKey;
std::filesystem::path clientCert, clientKey;
std::optional<std::filesystem::path> oldCaCert;
Pid serverPid;
uint16_t port = 8443;
std::shared_ptr<Store> localCacheStore;

/**
* Custom FileTransferSettings with the test CA certificate.
* This is used instead of modifying global settings.
*/
std::unique_ptr<FileTransferSettings> testFileTransferSettings;

/**
* FileTransfer instance using our test settings.
* Initialized in SetUp().
*/
std::shared_ptr<FileTransfer> testFileTransfer;

static void openssl(Strings args);
void SetUp() override;
void TearDown() override;

virtual std::vector<std::string> serverArgs();
ref<TestHttpBinaryCacheStoreConfig> makeConfig(BinaryCacheStoreConfig::Params params);
ref<TestHttpBinaryCacheStoreConfig> makeConfig();
ref<TestHttpBinaryCacheStore> openStore(ref<TestHttpBinaryCacheStoreConfig> config);
};

class HttpsBinaryCacheStoreMtlsTest : public HttpsBinaryCacheStoreTest
Expand Down
48 changes: 17 additions & 31 deletions src/libstore-tests/http-binary-cache-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,38 +45,27 @@ using namespace std::string_literals;

TEST_F(HttpsBinaryCacheStoreTest, queryPathInfo)
{
auto config = makeConfig({});
auto store = config->openStore();
auto store = openStore(makeConfig());
StringSource dump{"test"sv};
auto path = localCacheStore->addToStoreFromDump(dump, "test-name", FileSerialisationMethod::Flat);
EXPECT_NO_THROW(store->queryPathInfo(path));
}

auto withNoRetries()
{
auto oldTries = fileTransferSettings.tries.get();
Finally restoreTries = [=]() { fileTransferSettings.tries = oldTries; };
fileTransferSettings.tries = 1; /* FIXME: Don't use global settings. */
return restoreTries;
}

TEST_F(HttpsBinaryCacheStoreMtlsTest, queryPathInfo)
{
auto config = makeConfig({
{"tls-certificate"s, clientCert.string()},
{"tls-private-key"s, clientKey.string()},
});
auto store = config->openStore();
auto config = makeConfig();
config->tlsCert = clientCert;
config->tlsKey = clientKey;
auto store = openStore(config);
StringSource dump{"test"sv};
auto path = localCacheStore->addToStoreFromDump(dump, "test-name", FileSerialisationMethod::Flat);
EXPECT_NO_THROW(store->queryPathInfo(path));
}

TEST_F(HttpsBinaryCacheStoreMtlsTest, rejectsWithoutClientCert)
{
auto restoreTries = withNoRetries();
auto config = makeConfig({});
EXPECT_THROW(config->openStore(), Error);
testFileTransferSettings->tries = 1;
EXPECT_THROW(openStore(makeConfig()), Error);
}

TEST_F(HttpsBinaryCacheStoreMtlsTest, rejectsWrongClientCert)
Expand All @@ -89,12 +78,11 @@ TEST_F(HttpsBinaryCacheStoreMtlsTest, rejectsWrongClientCert)
openssl({"req", "-new", "-x509", "-days", "1", "-key", wrongKey.string(), "-out", wrongCert.string(), "-subj", "/CN=WrongClient"});
// clang-format on

auto config = makeConfig({
{"tls-certificate"s, wrongCert.string()},
{"tls-private-key"s, wrongKey.string()},
});
auto restoreTries = withNoRetries();
EXPECT_THROW(config->openStore(), Error);
auto config = makeConfig();
config->tlsCert = wrongCert;
config->tlsKey = wrongKey;
testFileTransferSettings->tries = 1;
EXPECT_THROW(openStore(config), Error);
}

TEST_F(HttpsBinaryCacheStoreMtlsTest, doesNotSendCertOnRedirectToDifferentAuthority)
Expand All @@ -109,13 +97,11 @@ TEST_F(HttpsBinaryCacheStoreMtlsTest, doesNotSendCertOnRedirectToDifferentAuthor
writeFile(entry.path(), content);
}

auto config = makeConfig({
{"tls-certificate"s, clientCert.string()},
{"tls-private-key"s, clientKey.string()},
});
auto store = config->openStore();

auto restoreTries = withNoRetries();
auto config = makeConfig();
config->tlsCert = clientCert;
config->tlsKey = clientKey;
testFileTransferSettings->tries = 1;
auto store = openStore(config);
auto info = store->queryPathInfo(path);
NullSink null;
EXPECT_THROW(store->narFromPath(path, null), Error);
Expand Down
37 changes: 21 additions & 16 deletions src/libstore/filetransfer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ struct curlMultiError : Error

struct curlFileTransfer : public FileTransfer
{
const FileTransferSettings & settings;

curlMulti curlm;

std::random_device rd;
Expand Down Expand Up @@ -480,10 +482,11 @@ struct curlFileTransfer : public FileTransfer
req,
CURLOPT_USERAGENT,
("curl/" LIBCURL_VERSION " Nix/" + nixVersion
+ (fileTransferSettings.userAgentSuffix != "" ? " " + fileTransferSettings.userAgentSuffix.get() : ""))
+ (fileTransfer.settings.userAgentSuffix != "" ? " " + fileTransfer.settings.userAgentSuffix.get()
: ""))
.c_str());
curl_easy_setopt(req, CURLOPT_PIPEWAIT, 1);
if (fileTransferSettings.enableHttp2)
if (fileTransfer.settings.enableHttp2)
curl_easy_setopt(req, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2TLS);
else
curl_easy_setopt(req, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
Expand All @@ -498,9 +501,9 @@ struct curlFileTransfer : public FileTransfer

curl_easy_setopt(req, CURLOPT_HTTPHEADER, requestHeaders.get());

if (fileTransferSettings.downloadSpeed.get() > 0)
if (fileTransfer.settings.downloadSpeed.get() > 0)
curl_easy_setopt(
req, CURLOPT_MAX_RECV_SPEED_LARGE, (curl_off_t) (fileTransferSettings.downloadSpeed.get() * 1024));
req, CURLOPT_MAX_RECV_SPEED_LARGE, (curl_off_t) (fileTransfer.settings.downloadSpeed.get() * 1024));

if (request.method == HttpMethod::Head)
curl_easy_setopt(req, CURLOPT_NOBODY, 1);
Expand Down Expand Up @@ -529,20 +532,20 @@ struct curlFileTransfer : public FileTransfer
curl_easy_setopt(req, CURLOPT_SEEKDATA, this);
}

if (auto & caFile = fileTransferSettings.caFile.get())
if (auto & caFile = fileTransfer.settings.caFile.get())
curl_easy_setopt(req, CURLOPT_CAINFO, caFile->c_str());

#if !defined(_WIN32)
curl_easy_setopt(req, CURLOPT_SOCKOPTFUNCTION, cloexec_callback);
#endif
curl_easy_setopt(req, CURLOPT_CONNECTTIMEOUT, fileTransferSettings.connectTimeout.get());
curl_easy_setopt(req, CURLOPT_CONNECTTIMEOUT, fileTransfer.settings.connectTimeout.get());

curl_easy_setopt(req, CURLOPT_LOW_SPEED_LIMIT, 1L);
curl_easy_setopt(req, CURLOPT_LOW_SPEED_TIME, fileTransferSettings.stalledDownloadTimeout.get());
curl_easy_setopt(req, CURLOPT_LOW_SPEED_TIME, fileTransfer.settings.stalledDownloadTimeout.get());

/* If no file exist in the specified path, curl continues to work
anyway as if netrc support was disabled. */
curl_easy_setopt(req, CURLOPT_NETRC_FILE, fileTransferSettings.netrcFile.get().c_str());
curl_easy_setopt(req, CURLOPT_NETRC_FILE, fileTransfer.settings.netrcFile.get().c_str());
curl_easy_setopt(req, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);

if (writtenToSink)
Expand Down Expand Up @@ -788,18 +791,20 @@ struct curlFileTransfer : public FileTransfer

std::thread workerThread;

const size_t maxQueueSize = fileTransferSettings.httpConnections.get() * 5;
const size_t maxQueueSize;

curlFileTransfer()
: mt19937(rd())
curlFileTransfer(const FileTransferSettings & settings)
: settings(settings)
, mt19937(rd())
, maxQueueSize(settings.httpConnections.get() * 5)
{
static std::once_flag globalInit;
std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL);

curlm = curlMulti(curl_multi_init());

curl_multi_setopt(curlm.get(), CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
curl_multi_setopt(curlm.get(), CURLMOPT_MAX_TOTAL_CONNECTIONS, fileTransferSettings.httpConnections.get());
curl_multi_setopt(curlm.get(), CURLMOPT_MAX_TOTAL_CONNECTIONS, settings.httpConnections.get());

workerThread = std::thread([&]() { workerThreadEntry(); });
}
Expand Down Expand Up @@ -1000,9 +1005,9 @@ struct curlFileTransfer : public FileTransfer
}
};

ref<curlFileTransfer> makeCurlFileTransfer()
ref<curlFileTransfer> makeCurlFileTransfer(const FileTransferSettings & settings = fileTransferSettings)
{
return make_ref<curlFileTransfer>();
return make_ref<curlFileTransfer>(settings);
}

ref<FileTransfer> getFileTransfer()
Expand All @@ -1015,9 +1020,9 @@ ref<FileTransfer> getFileTransfer()
return fileTransfer;
}

ref<FileTransfer> makeFileTransfer()
ref<FileTransfer> makeFileTransfer(const FileTransferSettings & settings)
{
return makeCurlFileTransfer();
return makeCurlFileTransfer(settings);
}

void FileTransferRequest::setupForS3()
Expand Down
Loading
Loading