diff --git a/src/libstore-test-support/https-store.cc b/src/libstore-test-support/https-store.cc index aeba8f0e0f4..79548f61f3f 100644 --- a/src/libstore-test-support/https-store.cc +++ b/src/libstore-test-support/https-store.cc @@ -38,8 +38,7 @@ void HttpsBinaryCacheStoreTest::SetUp() delTmpDir = std::make_unique(tmpDir); localCacheStore = - make_ref("file", cacheDir.string(), LocalBinaryCacheStoreConfig::Params{}) - ->openStore(); + make_ref(cacheDir, LocalBinaryCacheStoreConfig::Params{})->openStore(); caCert = tmpDir / "ca.crt"; caKey = tmpDir / "ca.key"; diff --git a/src/libstore-tests/http-binary-cache-store.cc b/src/libstore-tests/http-binary-cache-store.cc index 33369cd9b76..8495db4b5ab 100644 --- a/src/libstore-tests/http-binary-cache-store.cc +++ b/src/libstore-tests/http-binary-cache-store.cc @@ -7,24 +7,31 @@ namespace nix { +using Authority = ParsedURL::Authority; + TEST(HttpBinaryCacheStore, constructConfig) { - HttpBinaryCacheStoreConfig config{"http", "foo.bar.baz", {}}; + HttpBinaryCacheStoreConfig config{ + { + .scheme = "http", + .authority = Authority{.host = "foo.bar.baz"}, + }, + {}, + }; EXPECT_EQ(config.cacheUri.to_string(), "http://foo.bar.baz"); } TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash) { - HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", {}}; - + HttpBinaryCacheStoreConfig config{parseURL("https://foo.bar.baz/a/b/"), {}}; EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b"); } TEST(HttpBinaryCacheStore, constructConfigWithParams) { StoreConfig::Params params{{"compression", "xz"}}; - HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", params}; + HttpBinaryCacheStoreConfig config{parseURL("https://foo.bar.baz/a/b/"), params}; EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b"); EXPECT_EQ(config.getReference().params, params); } @@ -32,7 +39,7 @@ TEST(HttpBinaryCacheStore, constructConfigWithParams) TEST(HttpBinaryCacheStore, constructConfigWithParamsAndUrlWithParams) { StoreConfig::Params params{{"compression", "xz"}}; - HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b?some-param=some-value", params}; + HttpBinaryCacheStoreConfig config{parseURL("https://foo.bar.baz/a/b?some-param=some-value"), params}; EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b?some-param=some-value"); EXPECT_EQ(config.getReference().params, params); } diff --git a/src/libstore-tests/legacy-ssh-store.cc b/src/libstore-tests/legacy-ssh-store.cc index d60ecc424c5..35543cf4006 100644 --- a/src/libstore-tests/legacy-ssh-store.cc +++ b/src/libstore-tests/legacy-ssh-store.cc @@ -7,8 +7,7 @@ namespace nix { TEST(LegacySSHStore, constructConfig) { LegacySSHStoreConfig config( - "ssh", - "me@localhost:2222", + ParsedURL::Authority::parse("me@localhost:2222"), StoreConfig::Params{ { "remote-program", diff --git a/src/libstore-tests/local-binary-cache-store.cc b/src/libstore-tests/local-binary-cache-store.cc index 01f514e89aa..295976488b5 100644 --- a/src/libstore-tests/local-binary-cache-store.cc +++ b/src/libstore-tests/local-binary-cache-store.cc @@ -6,8 +6,7 @@ namespace nix { TEST(LocalBinaryCacheStore, constructConfig) { - LocalBinaryCacheStoreConfig config{"local", "/foo/bar/baz", {}}; - + LocalBinaryCacheStoreConfig config{std::filesystem::path("/foo/bar/baz"), {}}; EXPECT_EQ(config.binaryCacheDir, "/foo/bar/baz"); } diff --git a/src/libstore-tests/local-overlay-store.cc b/src/libstore-tests/local-overlay-store.cc index 175e5d0f44e..c207564255f 100644 --- a/src/libstore-tests/local-overlay-store.cc +++ b/src/libstore-tests/local-overlay-store.cc @@ -7,7 +7,6 @@ namespace nix { TEST(LocalOverlayStore, constructConfig_rootQueryParam) { LocalOverlayStoreConfig config{ - "local-overlay", "", { { @@ -22,7 +21,7 @@ TEST(LocalOverlayStore, constructConfig_rootQueryParam) TEST(LocalOverlayStore, constructConfig_rootPath) { - LocalOverlayStoreConfig config{"local-overlay", "/foo/bar", {}}; + LocalOverlayStoreConfig config{"/foo/bar", {}}; EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); } diff --git a/src/libstore-tests/local-store.cc b/src/libstore-tests/local-store.cc index d008888974b..554d42efc81 100644 --- a/src/libstore-tests/local-store.cc +++ b/src/libstore-tests/local-store.cc @@ -13,7 +13,6 @@ namespace nix { TEST(LocalStore, constructConfig_rootQueryParam) { LocalStoreConfig config{ - "local", "", { { @@ -28,14 +27,14 @@ TEST(LocalStore, constructConfig_rootQueryParam) TEST(LocalStore, constructConfig_rootPath) { - LocalStoreConfig config{"local", "/foo/bar", {}}; + LocalStoreConfig config{"/foo/bar", {}}; EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); } TEST(LocalStore, constructConfig_to_string) { - LocalStoreConfig config{"local", "", {}}; + LocalStoreConfig config{"", {}}; EXPECT_EQ(config.getReference().to_string(), "local"); } diff --git a/src/libstore-tests/s3-binary-cache-store.cc b/src/libstore-tests/s3-binary-cache-store.cc index 59090a589f0..9aa9b2dd1a3 100644 --- a/src/libstore-tests/s3-binary-cache-store.cc +++ b/src/libstore-tests/s3-binary-cache-store.cc @@ -9,7 +9,7 @@ namespace nix { TEST(S3BinaryCacheStore, constructConfig) { - S3BinaryCacheStoreConfig config{"s3", "foobar", {}}; + S3BinaryCacheStoreConfig config{"foobar", {}}; // The bucket name is stored as the host part of the authority in cacheUri EXPECT_EQ( @@ -23,7 +23,7 @@ TEST(S3BinaryCacheStore, constructConfig) TEST(S3BinaryCacheStore, constructConfigWithRegion) { Store::Config::Params params{{"region", "eu-west-1"}}; - S3BinaryCacheStoreConfig config{"s3", "my-bucket", params}; + S3BinaryCacheStoreConfig config{"my-bucket", params}; EXPECT_EQ( config.cacheUri, @@ -37,7 +37,7 @@ TEST(S3BinaryCacheStore, constructConfigWithRegion) TEST(S3BinaryCacheStore, defaultSettings) { - S3BinaryCacheStoreConfig config{"s3", "test-bucket", {}}; + S3BinaryCacheStoreConfig config{"test-bucket", {}}; EXPECT_EQ( config.cacheUri, @@ -62,7 +62,7 @@ TEST(S3BinaryCacheStore, s3StoreConfigPreservesParameters) params["region"] = "eu-west-1"; params["endpoint"] = "custom.s3.com"; - S3BinaryCacheStoreConfig config("s3", "test-bucket", params); + S3BinaryCacheStoreConfig config("test-bucket", params); // The config should preserve S3-specific parameters EXPECT_EQ( @@ -99,7 +99,7 @@ TEST(S3BinaryCacheStore, parameterFiltering) params["want-mass-query"] = "true"; // Non-S3 store parameter params["priority"] = "10"; // Non-S3 store parameter - S3BinaryCacheStoreConfig config("s3", "test-bucket", params); + S3BinaryCacheStoreConfig config("test-bucket", params); // Only S3-specific params should be in cacheUri.query EXPECT_EQ( @@ -127,7 +127,7 @@ TEST(S3BinaryCacheStore, parameterFiltering) */ TEST(S3BinaryCacheStore, storageClassDefault) { - S3BinaryCacheStoreConfig config{"s3", "test-bucket", {}}; + S3BinaryCacheStoreConfig config{"test-bucket", {}}; EXPECT_EQ(config.storageClass.get(), std::nullopt); } @@ -136,7 +136,7 @@ TEST(S3BinaryCacheStore, storageClassConfiguration) StringMap params; params["storage-class"] = "GLACIER"; - S3BinaryCacheStoreConfig config("s3", "test-bucket", params); + S3BinaryCacheStoreConfig config("test-bucket", params); EXPECT_EQ(config.storageClass.get(), std::optional("GLACIER")); } diff --git a/src/libstore-tests/ssh-store.cc b/src/libstore-tests/ssh-store.cc index a156da52b71..2d4548d6ae2 100644 --- a/src/libstore-tests/ssh-store.cc +++ b/src/libstore-tests/ssh-store.cc @@ -9,8 +9,7 @@ namespace nix { TEST(SSHStore, constructConfig) { SSHStoreConfig config{ - "ssh-ng", - "me@localhost:2222", + ParsedURL::Authority::parse("me@localhost:2222"), StoreConfig::Params{ { "remote-program", @@ -35,8 +34,7 @@ TEST(SSHStore, constructConfig) TEST(MountedSSHStore, constructConfig) { MountedSSHStoreConfig config{ - "mounted-ssh", - "localhost", + {.host = "localhost"}, StoreConfig::Params{ { "remote-program", diff --git a/src/libstore-tests/uds-remote-store.cc b/src/libstore-tests/uds-remote-store.cc index 415dfc4ac94..88af22dbb22 100644 --- a/src/libstore-tests/uds-remote-store.cc +++ b/src/libstore-tests/uds-remote-store.cc @@ -6,26 +6,21 @@ namespace nix { TEST(UDSRemoteStore, constructConfig) { - UDSRemoteStoreConfig config{"unix", "/tmp/socket", {}}; + UDSRemoteStoreConfig config{"/tmp/socket", {}}; EXPECT_EQ(config.path, "/tmp/socket"); } -TEST(UDSRemoteStore, constructConfigWrongScheme) -{ - EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError); -} - TEST(UDSRemoteStore, constructConfig_to_string) { - UDSRemoteStoreConfig config{"unix", "", {}}; + UDSRemoteStoreConfig config{"", {}}; EXPECT_EQ(config.getReference().to_string(), "daemon"); } TEST(UDSRemoteStore, constructConfigWithParams) { StoreConfig::Params params{{"max-connections", "1"}}; - UDSRemoteStoreConfig config{"unix", "/tmp/socket", params}; + UDSRemoteStoreConfig config{"/tmp/socket", params}; auto storeReference = config.getReference(); EXPECT_EQ(storeReference.to_string(), "unix:///tmp/socket?max-connections=1"); EXPECT_EQ(storeReference.render(/*withParams=*/false), "unix:///tmp/socket"); @@ -35,7 +30,7 @@ TEST(UDSRemoteStore, constructConfigWithParams) TEST(UDSRemoteStore, constructConfigWithParamsNoPath) { StoreConfig::Params params{{"max-connections", "1"}}; - UDSRemoteStoreConfig config{"unix", "", params}; + UDSRemoteStoreConfig config{"", params}; auto storeReference = config.getReference(); EXPECT_EQ(storeReference.to_string(), "daemon?max-connections=1"); EXPECT_EQ(storeReference.render(/*withParams=*/false), "daemon"); diff --git a/src/libstore/common-ssh-store-config.cc b/src/libstore/common-ssh-store-config.cc index 12f187b4c9e..fd61b12efee 100644 --- a/src/libstore/common-ssh-store-config.cc +++ b/src/libstore/common-ssh-store-config.cc @@ -1,17 +1,9 @@ -#include - #include "nix/store/common-ssh-store-config.hh" #include "nix/store/ssh.hh" namespace nix { -CommonSSHStoreConfig::CommonSSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) - : CommonSSHStoreConfig(scheme, ParsedURL::Authority::parse(authority), params) -{ -} - -CommonSSHStoreConfig::CommonSSHStoreConfig( - std::string_view scheme, const ParsedURL::Authority & authority, const Params & params) +CommonSSHStoreConfig::CommonSSHStoreConfig(const ParsedURL::Authority & authority, const Params & params) : StoreConfig(params) , authority(authority) { diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index c1bee215ce3..b3678ae4fdf 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -20,23 +20,13 @@ StringSet HttpBinaryCacheStoreConfig::uriSchemes() return ret; } -HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig( - std::string_view scheme, std::string_view _cacheUri, const Params & params) - : HttpBinaryCacheStoreConfig( - parseURL( - std::string{scheme} + "://" - + (!_cacheUri.empty() - ? _cacheUri - : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))), - params) -{ -} - HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig(ParsedURL _cacheUri, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) , cacheUri(std::move(_cacheUri)) { + if (!uriSchemes().contains("file") && (!cacheUri.authority || cacheUri.authority->host.empty())) + throw UsageError("`%s` Store requires a non-empty authority in Store URL", cacheUri.scheme); while (!cacheUri.path.empty() && cacheUri.path.back() == "") cacheUri.path.pop_back(); } diff --git a/src/libstore/include/nix/store/common-ssh-store-config.hh b/src/libstore/include/nix/store/common-ssh-store-config.hh index 7f8fc97b185..1fdb1675bdb 100644 --- a/src/libstore/include/nix/store/common-ssh-store-config.hh +++ b/src/libstore/include/nix/store/common-ssh-store-config.hh @@ -12,8 +12,7 @@ struct CommonSSHStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - CommonSSHStoreConfig(std::string_view scheme, const ParsedURL::Authority & authority, const Params & params); - CommonSSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); + CommonSSHStoreConfig(const ParsedURL::Authority & authority, const Params & params); Setting sshKey{ this, "", "ssh-key", "Path to the SSH private key used to authenticate to the remote machine."}; diff --git a/src/libstore/include/nix/store/http-binary-cache-store.hh b/src/libstore/include/nix/store/http-binary-cache-store.hh index 701eb431f6f..7138c56667f 100644 --- a/src/libstore/include/nix/store/http-binary-cache-store.hh +++ b/src/libstore/include/nix/store/http-binary-cache-store.hh @@ -16,9 +16,6 @@ struct HttpBinaryCacheStoreConfig : std::enable_shared_from_this { using CommonSSHStoreConfig::CommonSSHStoreConfig; - LegacySSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); + LegacySSHStoreConfig(const ParsedURL::Authority & authority, const Params & params); #ifndef _WIN32 // Hack for getting remote build log output. diff --git a/src/libstore/include/nix/store/local-binary-cache-store.hh b/src/libstore/include/nix/store/local-binary-cache-store.hh index 2438b37155c..8c4e68d2639 100644 --- a/src/libstore/include/nix/store/local-binary-cache-store.hh +++ b/src/libstore/include/nix/store/local-binary-cache-store.hh @@ -12,7 +12,7 @@ struct LocalBinaryCacheStoreConfig : std::enable_shared_from_this> rootDir = makeRootDirSetting(*this, std::nullopt); diff --git a/src/libstore/include/nix/store/local-overlay-store.hh b/src/libstore/include/nix/store/local-overlay-store.hh index 3c035007396..007ef0eed0c 100644 --- a/src/libstore/include/nix/store/local-overlay-store.hh +++ b/src/libstore/include/nix/store/local-overlay-store.hh @@ -8,14 +8,14 @@ namespace nix { struct LocalOverlayStoreConfig : virtual LocalStoreConfig { LocalOverlayStoreConfig(const StringMap & params) - : LocalOverlayStoreConfig("local-overlay", "", params) + : LocalOverlayStoreConfig("", params) { } - LocalOverlayStoreConfig(std::string_view scheme, PathView path, const Params & params) + LocalOverlayStoreConfig(const std::filesystem::path & path, const Params & params) : StoreConfig(params) , LocalFSStoreConfig(path, params) - , LocalStoreConfig(scheme, path, params) + , LocalStoreConfig(path, params) { } diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index eb4d971a18a..3d3ca6e589d 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -82,7 +82,7 @@ struct LocalStoreConfig : std::enable_shared_from_this, { using LocalFSStoreConfig::LocalFSStoreConfig; - LocalStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); + LocalStoreConfig(const std::filesystem::path & path, const Params & params); private: diff --git a/src/libstore/include/nix/store/s3-binary-cache-store.hh b/src/libstore/include/nix/store/s3-binary-cache-store.hh index 4fa3cdcbdb0..0edb1ea3547 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -11,7 +11,9 @@ struct S3BinaryCacheStoreConfig : HttpBinaryCacheStoreConfig { using HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig; - S3BinaryCacheStoreConfig(std::string_view uriScheme, std::string_view bucketName, const Params & params); + S3BinaryCacheStoreConfig(ParsedURL cacheUri, const Params & params); + + S3BinaryCacheStoreConfig(std::string_view bucketName, const Params & params); Setting profile{ this, diff --git a/src/libstore/include/nix/store/ssh-store.hh b/src/libstore/include/nix/store/ssh-store.hh index 2c27bdb13ba..f10068ee4e2 100644 --- a/src/libstore/include/nix/store/ssh-store.hh +++ b/src/libstore/include/nix/store/ssh-store.hh @@ -15,7 +15,7 @@ struct SSHStoreConfig : std::enable_shared_from_this, using CommonSSHStoreConfig::CommonSSHStoreConfig; using RemoteStoreConfig::RemoteStoreConfig; - SSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); + SSHStoreConfig(const ParsedURL::Authority & authority, const Params & params); Setting remoteProgram{ this, {"nix-daemon"}, "remote-program", "Path to the `nix-daemon` executable on the remote machine."}; @@ -40,7 +40,7 @@ struct SSHStoreConfig : std::enable_shared_from_this, struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig { MountedSSHStoreConfig(StringMap params); - MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params); + MountedSSHStoreConfig(const ParsedURL::Authority & authority, StringMap params); static const std::string name() { diff --git a/src/libstore/include/nix/store/store-reference.hh b/src/libstore/include/nix/store/store-reference.hh index b35c27f82ec..37ff2cdee3d 100644 --- a/src/libstore/include/nix/store/store-reference.hh +++ b/src/libstore/include/nix/store/store-reference.hh @@ -56,6 +56,7 @@ struct StoreReference /** * General case, a regular `scheme://authority` URL. + * @todo Consider making this pluggable instead of passing through the encoded authority + path. */ struct Specified { diff --git a/src/libstore/include/nix/store/store-registration.hh b/src/libstore/include/nix/store/store-registration.hh index 8b0f344ba38..8d6578cac05 100644 --- a/src/libstore/include/nix/store/store-registration.hh +++ b/src/libstore/include/nix/store/store-registration.hh @@ -14,6 +14,7 @@ */ #include "nix/store/store-api.hh" +#include "nix/util/url.hh" namespace nix { @@ -65,7 +66,16 @@ struct Implementations .uriSchemes = TConfig::uriSchemes(), .experimentalFeature = TConfig::experimentalFeature(), .parseConfig = ([](auto scheme, auto uri, auto & params) -> ref { - return make_ref(scheme, uri, params); + if constexpr (std::is_constructible_v) { + std::filesystem::path path = percentDecode(uri); + return make_ref(path.empty() ? std::filesystem::path{} : canonPath(path), params); + } else if constexpr (std::is_constructible_v) { + return make_ref(parseURL(concatStrings(scheme, "://", uri)), params); + } else if constexpr (std::is_constructible_v) { + return make_ref(ParsedURL::Authority::parse(uri), params); + } else { + return make_ref(scheme, uri, params); + } }), .getConfig = ([]() -> ref { return make_ref(Store::Config::Params{}); }), }; diff --git a/src/libstore/include/nix/store/uds-remote-store.hh b/src/libstore/include/nix/store/uds-remote-store.hh index 3b8dfadd10c..64672ee3adf 100644 --- a/src/libstore/include/nix/store/uds-remote-store.hh +++ b/src/libstore/include/nix/store/uds-remote-store.hh @@ -11,15 +11,10 @@ struct UDSRemoteStoreConfig : std::enable_shared_from_this virtual LocalFSStoreConfig, virtual RemoteStoreConfig { - // TODO(fzakaria): Delete this constructor once moved over to the factory pattern - // outlined in https://github.com/NixOS/nix/issues/10766 using LocalFSStoreConfig::LocalFSStoreConfig; using RemoteStoreConfig::RemoteStoreConfig; - /** - * @param authority is the socket path. - */ - UDSRemoteStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); + UDSRemoteStoreConfig(const std::filesystem::path & path, const Params & params); UDSRemoteStoreConfig(const Params & params); diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index b2f2cf8b243..81f1facb90d 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -18,9 +18,9 @@ namespace nix { -LegacySSHStoreConfig::LegacySSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) +LegacySSHStoreConfig::LegacySSHStoreConfig(const ParsedURL::Authority & authority, const Params & params) : StoreConfig(params) - , CommonSSHStoreConfig(scheme, ParsedURL::Authority::parse(authority), params) + , CommonSSHStoreConfig(authority, params) { } diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index ce4f2358d52..8dba477315f 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -8,8 +8,25 @@ namespace nix { +static std::filesystem::path checkBinaryCachePath(const std::filesystem::path & root, const std::string & path) +{ + auto p = std::filesystem::path(requireCString(path)); + if (p.empty()) + throw Error("local binary cache path must not be empty"); + + if (p.is_absolute()) + throw Error("local binary cache path '%s' must not be absolute", path); + + for (const auto & segment : p) { + if (segment.native() == OS_STR("..") || segment.native() == OS_STR(".")) + throw Error("local binary cache path '%s' must not contain '..' or '.' segments", path); + } + + return root / p.relative_path(); +} + LocalBinaryCacheStoreConfig::LocalBinaryCacheStoreConfig( - std::string_view scheme, PathView binaryCacheDir, const StoreReference::Params & params) + const std::filesystem::path & binaryCacheDir, const StoreReference::Params & params) : Store::Config{params} , BinaryCacheStoreConfig{params} , binaryCacheDir(binaryCacheDir) @@ -29,7 +46,7 @@ StoreReference LocalBinaryCacheStoreConfig::getReference() const .variant = StoreReference::Specified{ .scheme = "file", - .authority = binaryCacheDir.string(), + .authority = encodeUrlPath(pathToUrlPath(binaryCacheDir)), }, }; } @@ -56,23 +73,22 @@ struct LocalBinaryCacheStore : virtual BinaryCacheStore void upsertFile( const std::string & path, RestartableSource & source, const std::string & mimeType, uint64_t sizeHint) override { - assert(!std::filesystem::path(path).is_absolute()); - auto path2 = config->binaryCacheDir / path; + auto path2 = checkBinaryCachePath(config->binaryCacheDir, path); static std::atomic counter{0}; createDirs(path2.parent_path()); auto tmp = path2; tmp += fmt(".tmp.%d.%d", getpid(), ++counter); AutoDelete del(tmp, false); - writeFile(tmp, source); + writeFile(tmp, source); /* TODO: Don't follow symlinks? */ std::filesystem::rename(tmp, path2); del.cancel(); } void getFile(const std::string & path, Sink & sink) override { - assert(!std::filesystem::path(path).is_absolute()); try { - readFile(config->binaryCacheDir / path, sink); + /* TODO: Don't follow symlinks? */ + readFile(checkBinaryCachePath(config->binaryCacheDir, path), sink); } catch (SystemError & e) { if (e.is(std::errc::no_such_file_or_directory)) throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache", path); @@ -113,8 +129,7 @@ void LocalBinaryCacheStore::init() bool LocalBinaryCacheStore::fileExists(const std::string & path) { - assert(!std::filesystem::path(path).is_absolute()); - return pathExists(config->binaryCacheDir / path); + return pathExists(checkBinaryCachePath(config->binaryCacheDir, path)); } StringSet LocalBinaryCacheStoreConfig::uriSchemes() diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 89925098f16..410385ee113 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -18,7 +18,7 @@ std::filesystem::path LocalFSStoreConfig::getDefaultLogDir() return settings.getLogFileSettings().nixLogDir; } -LocalFSStoreConfig::LocalFSStoreConfig(PathView rootDir, const Params & params) +LocalFSStoreConfig::LocalFSStoreConfig(const std::filesystem::path & rootDir, const Params & params) : StoreConfig(params) /* Default `?root` from `rootDir` if non set * NOTE: We would like to just do rootDir.set(...), which would take care of diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 784c5770316..61dcec6f37b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -56,9 +56,9 @@ namespace nix { -LocalStoreConfig::LocalStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) +LocalStoreConfig::LocalStoreConfig(const std::filesystem::path & path, const Params & params) : StoreConfig(params) - , LocalFSStoreConfig(authority, params) + , LocalFSStoreConfig(path, params) { } @@ -468,11 +468,13 @@ StoreReference LocalStoreConfig::getReference() const /* Back-compatibility kludge. Tools like nix-output-monitor expect 'local' and can't parse 'local://'. */ if (params.empty()) + /* TODO: Add the rootDir here as the authority? */ return {.variant = StoreReference::Local{}}; return { .variant = StoreReference::Specified{ .scheme = *uriSchemes().begin(), + /* TODO: Add the rootDir here as the authority? */ }, .params = std::move(params), }; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 2a3be047463..8c3ce56fcfc 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -416,10 +416,9 @@ StringSet S3BinaryCacheStoreConfig::uriSchemes() return {"s3"}; } -S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( - std::string_view scheme, std::string_view _cacheUri, const Params & params) +S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig(ParsedURL cacheUri_, const Params & params) : StoreConfig(params) - , HttpBinaryCacheStoreConfig(scheme, _cacheUri, params) + , HttpBinaryCacheStoreConfig(std::move(cacheUri_), params) { assert(cacheUri.query.empty()); assert(cacheUri.scheme == "s3"); @@ -455,6 +454,12 @@ S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( } } +S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig(std::string_view bucketName, const Params & params) + : S3BinaryCacheStoreConfig( + ParsedURL{.scheme = "s3", .authority = ParsedURL::Authority{.host = std::string(bucketName)}}, params) +{ +} + std::string S3BinaryCacheStoreConfig::getHumanReadableURI() const { auto reference = getReference(); diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index aab9741640c..22a023cf569 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -11,10 +11,10 @@ namespace nix { -SSHStoreConfig::SSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) +SSHStoreConfig::SSHStoreConfig(const ParsedURL::Authority & authority, const Params & params) : Store::Config{params} , RemoteStore::Config{params} - , CommonSSHStoreConfig{scheme, authority, params} + , CommonSSHStoreConfig{authority, params} { } @@ -97,11 +97,11 @@ MountedSSHStoreConfig::MountedSSHStoreConfig(StringMap params) { } -MountedSSHStoreConfig::MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params) +MountedSSHStoreConfig::MountedSSHStoreConfig(const ParsedURL::Authority & authority, StringMap params) : StoreConfig(params) , RemoteStoreConfig(params) - , CommonSSHStoreConfig(scheme, host, params) - , SSHStoreConfig(scheme, host, params) + , CommonSSHStoreConfig(authority, params) + , SSHStoreConfig(authority, params) , LocalFSStoreConfig(params) { } diff --git a/src/libstore/store-registration.cc b/src/libstore/store-registration.cc index 42caa819034..e4d5843ed9b 100644 --- a/src/libstore/store-registration.cc +++ b/src/libstore/store-registration.cc @@ -53,7 +53,7 @@ ref resolveStoreConfig(StoreReference && storeURI) } else debug( "%s does not exist, so Nix will use %s as a chroot store", stateDir, PathFmt(chrootStore)); - return make_ref("local", chrootStore.string(), params); + return make_ref(std::filesystem::path(chrootStore), params); } #endif else diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 7c4595c634e..98e6a29332b 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -19,16 +19,12 @@ namespace nix { -UDSRemoteStoreConfig::UDSRemoteStoreConfig( - std::string_view scheme, std::string_view authority, const StoreReference::Params & params) +UDSRemoteStoreConfig::UDSRemoteStoreConfig(const std::filesystem::path & path, const StoreReference::Params & params) : Store::Config{params} , LocalFSStore::Config{params} , RemoteStore::Config{params} - , path{authority.empty() ? settings.nixDaemonSocketFile : std::filesystem::path{authority}} + , path{path.empty() ? settings.nixDaemonSocketFile : path} { - if (uriSchemes().count(scheme) == 0) { - throw UsageError("Scheme must be 'unix'"); - } } std::string UDSRemoteStoreConfig::doc() @@ -43,7 +39,7 @@ std::string UDSRemoteStoreConfig::doc() // don't we just wire it all through? I believe there are cases where it // will live reload so we want to continue to account for that. UDSRemoteStoreConfig::UDSRemoteStoreConfig(const Params & params) - : UDSRemoteStoreConfig(*uriSchemes().begin(), "", params) + : UDSRemoteStoreConfig("", params) { } @@ -69,7 +65,7 @@ StoreReference UDSRemoteStoreConfig::getReference() const .variant = StoreReference::Specified{ .scheme = *uriSchemes().begin(), - .authority = path.string(), + .authority = encodeUrlPath(pathToUrlPath(path)), }, .params = getQueryParams(), }; diff --git a/src/libutil/include/nix/util/url.hh b/src/libutil/include/nix/util/url.hh index 32c7ee7ecb8..b1cc1155a66 100644 --- a/src/libutil/include/nix/util/url.hh +++ b/src/libutil/include/nix/util/url.hh @@ -280,7 +280,7 @@ std::string encodeQuery(const StringMap & query); /** * Parse a URL into a ParsedURL. * - * @parm lenient Also allow some long-supported Nix URIs that are not quite compliant with RFC3986. + * @param lenient Also allow some long-supported Nix URIs that are not quite compliant with RFC3986. * Here are the deviations: * - Fragments can contain unescaped (not URL encoded) '^', '"' or space literals. * - Queries may contain unescaped '"' or spaces. diff --git a/src/nix/nix-copy-closure/nix-copy-closure.cc b/src/nix/nix-copy-closure/nix-copy-closure.cc index 8141a7f62ac..43cf11e4faf 100644 --- a/src/nix/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix/nix-copy-closure/nix-copy-closure.cc @@ -49,7 +49,11 @@ static int main_nix_copy_closure(int argc, char ** argv) if (sshHost.empty()) throw UsageError("no host name specified"); - auto remoteConfig = make_ref("ssh", sshHost, LegacySSHStoreConfig::Params{}); + auto remoteConfig = + /* FIXME: This doesn't go through the back-compat machinery for IPv6 unbracketed URLs that + is in StoreReference::parse. TODO: Maybe add a authority parsing function specifically + for SSH reference parsing? */ + make_ref(ParsedURL::Authority::parse(sshHost), LegacySSHStoreConfig::Params{}); remoteConfig->compress |= gzip; auto to = toMode ? remoteConfig->openStore() : openStore(); auto from = toMode ? openStore() : remoteConfig->openStore(); diff --git a/tests/functional/binary-cache.sh b/tests/functional/binary-cache.sh index 066fa026ed9..58025b78973 100755 --- a/tests/functional/binary-cache.sh +++ b/tests/functional/binary-cache.sh @@ -38,6 +38,10 @@ nix log --substituters "file://$cacheDir" "$outPath" | grep FOO nix store copy-log --from "file://$cacheDir" "$(nix-store -qd "$outPath")"^'*' nix log "$outPath" | grep FOO +# Test that plus sign in the URL path is handled correctly. +cacheDir2="$TEST_ROOT/binary+cache" +nix copy --to "file://$cacheDir2" "$outPath" && [[ -d "$cacheDir2" ]] + basicDownloadTests() { # No uploading tests bcause upload with force HTTP doesn't work. diff --git a/tests/functional/chroot-store.sh b/tests/functional/chroot-store.sh index 7300f04ba75..cbb80c8710a 100755 --- a/tests/functional/chroot-store.sh +++ b/tests/functional/chroot-store.sh @@ -42,6 +42,24 @@ PATH2=$(nix path-info --store "$TEST_ROOT/x" "$CORRECT_PATH") PATH3=$(nix path-info --store "local?root=$TEST_ROOT/x" "$CORRECT_PATH") [ "$CORRECT_PATH" == "$PATH3" ] +# Test chroot store path with + symbols in it to exercise pct-encoding issues. +cp -r "$TEST_ROOT/x" "$TEST_ROOT/x+chroot" + +PATH4=$(nix path-info --store "local://$TEST_ROOT/x+chroot" "$CORRECT_PATH") +[ "$CORRECT_PATH" == "$PATH4" ] + +PATH5=$(nix path-info --store "$TEST_ROOT/x+chroot" "$CORRECT_PATH") +[ "$CORRECT_PATH" == "$PATH5" ] + +# Params are pct-encoded. +PATH6=$(nix path-info --store "local?root=$TEST_ROOT/x%2Bchroot" "$CORRECT_PATH") +[ "$CORRECT_PATH" == "$PATH6" ] + +PATH7=$(nix path-info --store "local://$TEST_ROOT/x%2Bchroot" "$CORRECT_PATH") +[ "$CORRECT_PATH" == "$PATH7" ] +# Path gets decoded. +[[ ! -d "$TEST_ROOT/x%2Bchroot" ]] + # Ensure store info trusted works with local store nix --store "$TEST_ROOT/x" store info --json | jq -e '.trusted'