Ensure all store types support "real" URIs

In particular `local://<path>` and `unix://` (without any path) now work, and mean the same things as `local` and `daemon`, respectively. We thus now have the opportunity to desguar `local` and `daemon` early. This will allow me to make a change to NixOS#9839 requested during review to desugar those earlier. Co-authored-by: Théophane Hufschmitt <[email protected]>
Ericson2314 · May 21, 2024 · 470c050 · 470c050
1 parent 3a7d625
commit 470c050
Show file tree

Hide file tree

Showing 19 changed files with 170 additions and 75 deletions.
diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc
@@ -18,9 +18,12 @@ struct DummyStoreConfig : virtual StoreConfig {
 
 struct DummyStore : public virtual DummyStoreConfig, public virtual Store
 {
-    DummyStore(const std::string scheme, const std::string uri, const Params & params)
+    DummyStore(std::string_view scheme, std::string_view authority, const Params & params)
         : DummyStore(params)
-    { }
+    {
+        if (!authority.empty())
+            throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority);
+    }
 
     DummyStore(const Params & params)
         : StoreConfig(params)

diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
@@ -39,15 +39,20 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v
 public:
 
     HttpBinaryCacheStore(
-        const std::string & scheme,
-        const Path & _cacheUri,
+        std::string_view scheme,
+        PathView _cacheUri,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
         , HttpBinaryCacheStoreConfig(params)
         , Store(params)
         , BinaryCacheStore(params)
-        , cacheUri(scheme + "://" + _cacheUri)
+        , cacheUri(
+            std::string { scheme }
+            + "://"
+            + (!_cacheUri.empty()
+                ? _cacheUri
+                : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme)))
     {
         while (!cacheUri.empty() && cacheUri.back() == '/')
             cacheUri.pop_back();

diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
@@ -28,8 +28,18 @@ struct LegacySSHStore::Connection : public ServeProto::BasicClientConnection
     bool good = true;
 };
 
+LegacySSHStore::LegacySSHStore(
+    std::string_view scheme,
+    std::string_view host,
+    const Params & params)
+    : LegacySSHStore{scheme, LegacySSHStoreConfig::extractConnStr(scheme, host), params}
+{
+}
 
-LegacySSHStore::LegacySSHStore(const std::string & scheme, const std::string & host, const Params & params)
+LegacySSHStore::LegacySSHStore(
+    std::string_view scheme,
+    std::string host,
+    const Params & params)
     : StoreConfig(params)
     , CommonSSHStoreConfig(params)
     , LegacySSHStoreConfig(params)
@@ -42,8 +52,8 @@ LegacySSHStore::LegacySSHStore(const std::string & scheme, const std::string & h
         ))
     , master(
         host,
-        sshKey,
-        sshPublicHostKey,
+        sshKey.get(),
+        sshPublicHostKey.get(),
         // Use SSH master only if using more than 1 connection.
         connections->capacity() > 1,
         compress,

diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh
@@ -41,7 +41,17 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
 
     static std::set<std::string> uriSchemes() { return {"ssh"}; }
 
-    LegacySSHStore(const std::string & scheme, const std::string & host, const Params & params);
+    LegacySSHStore(
+        std::string_view scheme,
+        std::string_view host,
+        const Params & params);
+
+private:
+    LegacySSHStore(
+        std::string_view scheme,
+        std::string host,
+        const Params & params);
+public:
 
     ref<Connection> openConnection();
 

diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
@@ -28,9 +28,13 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
 
 public:
 
+    /**
+     * @param binaryCacheDir `file://` is a short-hand for `file:///`
+     * for now.
+     */
     LocalBinaryCacheStore(
-        const std::string scheme,
-        const Path & binaryCacheDir,
+        std::string_view scheme,
+        PathView binaryCacheDir,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -463,10 +463,20 @@ LocalStore::LocalStore(const Params & params)
 }
 
 
-LocalStore::LocalStore(std::string scheme, std::string path, const Params & params)
-    : LocalStore(params)
+LocalStore::LocalStore(
+    std::string_view scheme,
+    PathView path,
+    const Params & _params)
+    : LocalStore([&]{
+        // Default `?root` from `path` if non set
+        if (!path.empty() && _params.count("root") == 0) {
+            auto params = _params;
+            params.insert_or_assign("root", std::string { path });
+            return params;
+        }
+        return _params;
+    }())
 {
-    throw UnimplementedError("LocalStore");
 }
 
 

diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
@@ -137,12 +137,15 @@ public:
      * necessary.
      */
     LocalStore(const Params & params);
-    LocalStore(std::string scheme, std::string path, const Params & params);
+    LocalStore(
+        std::string_view scheme,
+        PathView path,
+        const Params & params);
 
     ~LocalStore();
 
     static std::set<std::string> uriSchemes()
-    { return {}; }
+    { return {"local"}; }
 
     /**
      * Implementations of abstract store API methods.

diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc
@@ -213,7 +213,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           support it.
 
           > **Note**
-          > 
+          >
           > HTTPS should be used if the cache might contain sensitive
           > information.
         )"};
@@ -224,7 +224,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           Do not specify this setting if you're using Amazon S3.
 
           > **Note**
-          > 
+          >
           > This endpoint must support HTTPS and will use path-based
           > addressing instead of virtual host based addressing.
         )"};
@@ -269,8 +269,8 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual
     S3Helper s3Helper;
 
     S3BinaryCacheStoreImpl(
-        const std::string & uriScheme,
-        const std::string & bucketName,
+        std::string_view uriScheme,
+        std::string_view bucketName,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
@@ -281,6 +281,8 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual
         , bucketName(bucketName)
         , s3Helper(profile, region, scheme, endpoint)
     {
+        if (bucketName.empty())
+            throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme);
         diskCache = getNarInfoDiskCache();
     }
 

diff --git a/src/libstore/ssh-store-config.cc b/src/libstore/ssh-store-config.cc
@@ -0,0 +1,24 @@
+#include <regex>
+
+#include "ssh-store-config.hh"
+
+namespace nix {
+
+std::string CommonSSHStoreConfig::extractConnStr(std::string_view scheme, std::string_view _connStr)
+{
+    if (_connStr.empty())
+        throw UsageError("`%s` store requires a valid SSH host as the authority part in Store URI", scheme);
+
+    std::string connStr{_connStr};
+
+    std::smatch result;
+    static std::regex v6AddrRegex("^((.*)@)?\\[(.*)\\]$");
+
+    if (std::regex_match(connStr, result, v6AddrRegex)) {
+        connStr = result[1].matched ? result.str(1) + result.str(3) : result.str(3);
+    }
+
+    return connStr;
+}
+
+}
diff --git a/src/libstore/ssh-store-config.hh b/src/libstore/ssh-store-config.hh
@@ -24,6 +24,29 @@ struct CommonSSHStoreConfig : virtual StoreConfig
           to be used on the remote machine. The default is `auto`
           (i.e. use the Nix daemon or `/nix/store` directly).
         )"};
+
+    /**
+     * The `parseURL` function supports both IPv6 URIs as defined in
+     * RFC2732, but also pure addresses. The latter one is needed here to
+     * connect to a remote store via SSH (it's possible to do e.g. `ssh root@::1`).
+     *
+     * This function now ensures that a usable connection string is available:
+     *
+     * - If the store to be opened is not an SSH store, nothing will be done.
+     *
+     * - If the URL looks like `root@[::1]` (which is allowed by the URL parser and probably
+     *   needed to pass further flags), it
+     *   will be transformed into `root@::1` for SSH (same for `[::1]` -> `::1`).
+     *
+     * - If the URL looks like `root@::1` it will be left as-is.
+     *
+     * - In any other case, the string will be left as-is.
+     *
+     * Will throw an error if `connStr` is empty too.
+     */
+    static std::string extractConnStr(
+        std::string_view scheme,
+        std::string_view connStr);
 };
 
 }
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
@@ -32,9 +32,10 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
 
 class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
 {
-public:
-
-    SSHStore(const std::string & scheme, const std::string & host, const Params & params)
+    SSHStore(
+        std::string_view scheme,
+        std::string host,
+        const Params & params)
         : StoreConfig(params)
         , RemoteStoreConfig(params)
         , CommonSSHStoreConfig(params)
@@ -44,14 +45,24 @@ class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
         , host(host)
         , master(
             host,
-            sshKey,
-            sshPublicHostKey,
+            sshKey.get(),
+            sshPublicHostKey.get(),
             // Use SSH master only if using more than 1 connection.
             connections->capacity() > 1,
             compress)
     {
     }
 
+public:
+
+    SSHStore(
+        std::string_view scheme,
+        std::string_view host,
+        const Params & params)
+        : SSHStore{scheme, SSHStoreConfig::extractConnStr(scheme, host), params}
+    {
+    }
+
     static std::set<std::string> uriSchemes() { return {"ssh-ng"}; }
 
     std::string getUri() override
@@ -141,7 +152,10 @@ class MountedSSHStore : public virtual MountedSSHStoreConfig, public virtual SSH
 {
 public:
 
-    MountedSSHStore(const std::string & scheme, const std::string & host, const Params & params)
+    MountedSSHStore(
+        std::string_view scheme,
+        std::string_view host,
+        const Params & params)
         : StoreConfig(params)
         , RemoteStoreConfig(params)
         , CommonSSHStoreConfig(params)

diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc
@@ -6,7 +6,11 @@
 
 namespace nix {
 
-SSHMaster::SSHMaster(const std::string & host, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD)
+SSHMaster::SSHMaster(
+    std::string_view host,
+    std::string_view keyFile,
+    std::string_view sshPublicHostKey,
+    bool useMaster, bool compress, int logFD)
     : host(host)
     , fakeSSH(host == "localhost")
     , keyFile(keyFile)

diff --git a/src/libstore/ssh.hh b/src/libstore/ssh.hh
@@ -39,7 +39,11 @@ private:
 
 public:
 
-    SSHMaster(const std::string & host, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD = -1);
+    SSHMaster(
+        std::string_view host,
+        std::string_view keyFile,
+        std::string_view sshPublicHostKey,
+        bool useMaster, bool compress, int logFD = -1);
 
     struct Connection
     {

diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
@@ -21,7 +21,6 @@
 #include "users.hh"
 
 #include <nlohmann/json.hpp>
-#include <regex>
 
 using json = nlohmann::json;
 
@@ -1321,9 +1320,7 @@ std::shared_ptr<Store> openFromNonUri(const std::string & uri, const Store::Para
                 warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore);
             } else
                 debug("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore);
-            Store::Params params2;
-            params2["root"] = chrootStore;
-            return std::make_shared<LocalStore>(params2);
+            return std::make_shared<LocalStore>("local", chrootStore, params);
         }
         #endif
         else
@@ -1333,42 +1330,12 @@ std::shared_ptr<Store> openFromNonUri(const std::string & uri, const Store::Para
     } else if (uri == "local") {
         return std::make_shared<LocalStore>(params);
     } else if (isNonUriPath(uri)) {
-        Store::Params params2 = params;
-        params2["root"] = absPath(uri);
-        return std::make_shared<LocalStore>(params2);
+        return std::make_shared<LocalStore>("local", absPath(uri), params);
     } else {
         return nullptr;
     }
 }
 
-// The `parseURL` function supports both IPv6 URIs as defined in
-// RFC2732, but also pure addresses. The latter one is needed here to
-// connect to a remote store via SSH (it's possible to do e.g. `ssh root@::1`).
-//
-// This function now ensures that a usable connection string is available:
-// * If the store to be opened is not an SSH store, nothing will be done.
-// * If the URL looks like `root@[::1]` (which is allowed by the URL parser and probably
-//   needed to pass further flags), it
-//   will be transformed into `root@::1` for SSH (same for `[::1]` -> `::1`).
-// * If the URL looks like `root@::1` it will be left as-is.
-// * In any other case, the string will be left as-is.
-static std::string extractConnStr(const std::string &proto, const std::string &connStr)
-{
-    if (proto.rfind("ssh") != std::string::npos) {
-        std::smatch result;
-        std::regex v6AddrRegex("^((.*)@)?\\[(.*)\\]$");
-
-        if (std::regex_match(connStr, result, v6AddrRegex)) {
-            if (result[1].matched) {
-                return result.str(1) + result.str(3);
-            }
-            return result.str(3);
-        }
-    }
-
-    return connStr;
-}
-
 ref<Store> openStore(const std::string & uri_,
     const Store::Params & extraParams)
 {
@@ -1377,10 +1344,7 @@ ref<Store> openStore(const std::string & uri_,
         auto parsedUri = parseURL(uri_);
         params.insert(parsedUri.query.begin(), parsedUri.query.end());
 
-        auto baseURI = extractConnStr(
-            parsedUri.scheme,
-            parsedUri.authority.value_or("") + parsedUri.path
-        );
+        auto baseURI = parsedUri.authority.value_or("") + parsedUri.path;
 
         for (auto implem : *Implementations::registered) {
             if (implem.uriSchemes.count(parsedUri.scheme)) {