Systematize fetcher input attribute validation

We now have `schemeName` and `allowedAttrs` functions for this purpose.
We look up the schema with the former; we restrict the set of input
attributes with the latter.

src/libfetchers/fetchers.cc

@@ -5,12 +5,18 @@
 
 namespace nix::fetchers {
 
-std::unique_ptr<std::vector<std::shared_ptr<InputScheme>>> inputSchemes = nullptr;
+using InputSchemeMap = std::map<std::string_view, std::shared_ptr<InputScheme>>;
+
+std::unique_ptr<InputSchemeMap> inputSchemes = nullptr;
 
 void registerInputScheme(std::shared_ptr<InputScheme> && inputScheme)
 {
-    if (!inputSchemes) inputSchemes = std::make_unique<std::vector<std::shared_ptr<InputScheme>>>();
+    if (!inputSchemes)
-    inputSchemes->push_back(std::move(inputScheme));
+        inputSchemes = std::make_unique<InputSchemeMap>();
+    auto schemeName = inputScheme->schemeName();
+    if (inputSchemes->count(schemeName) > 0)
+        throw Error("Input scheme with name %s already registered", schemeName);
+    inputSchemes->insert_or_assign(schemeName, std::move(inputScheme));
 }
 
 Input Input::fromURL(const std::string & url, bool requireTree)
@@ -33,7 +39,7 @@ static void fixupInput(Input & input)
 
 Input Input::fromURL(const ParsedURL & url, bool requireTree)
 {
-    for (auto & inputScheme : *inputSchemes) {
+    for (auto & [_, inputScheme] : *inputSchemes) {
         auto res = inputScheme->inputFromURL(url, requireTree);
         if (res) {
             experimentalFeatureSettings.require(inputScheme->experimentalFeature());
@@ -48,20 +54,44 @@ Input Input::fromURL(const ParsedURL & url, bool requireTree)
 
 Input Input::fromAttrs(Attrs && attrs)
 {
-    for (auto & inputScheme : *inputSchemes) {
+    auto schemeName = ({
-        auto res = inputScheme->inputFromAttrs(attrs);
+        auto schemeNameOpt = maybeGetStrAttr(attrs, "type");
-        if (res) {
+        if (!schemeNameOpt)
-            experimentalFeatureSettings.require(inputScheme->experimentalFeature());
+            throw Error("'type' attribute to specify input scheme is required but not provided");
-            res->scheme = inputScheme;
+        *std::move(schemeNameOpt);
-            fixupInput(*res);
+    });
-            return std::move(*res);
-        }
-    }
 
-    Input input;
+    auto raw = [&]() {
-    input.attrs = attrs;
+        // Return an input without a scheme; most operations will fail,
-    fixupInput(input);
+        // but not all of them. Doing this is to support those other
-    return input;
+        // operations which are supposed to be robust on
+        // unknown/uninterpretable inputs.
+        Input input;
+        input.attrs = attrs;
+        fixupInput(input);
+        return input;
+    };
+
+    std::shared_ptr<InputScheme> inputScheme = ({
+        auto i = inputSchemes->find(schemeName);
+        i == inputSchemes->end() ? nullptr : i->second;
+    });
+
+    if (!inputScheme) return raw();
+
+    experimentalFeatureSettings.require(inputScheme->experimentalFeature());
+
+    auto allowedAttrs = inputScheme->allowedAttrs();
+
+    for (auto & [name, _] : attrs)
+        if (name != "type" && allowedAttrs.count(name) == 0)
+            throw Error("input attribute '%s' not supported by scheme '%s'", name, schemeName);
+
+    auto res = inputScheme->inputFromAttrs(attrs);
+    if (!res) return raw();
+    res->scheme = inputScheme;
+    fixupInput(*res);
+    return std::move(*res);
 }
 
 ParsedURL Input::toURL() const
@@ -307,7 +337,7 @@ void InputScheme::clone(const Input & input, const Path & destDir) const
     throw Error("do not know how to clone input '%s'", input.to_string());
 }
 
-std::optional<ExperimentalFeature> InputScheme::experimentalFeature()
+std::optional<ExperimentalFeature> InputScheme::experimentalFeature() const
 {
     return {};
 }

src/libfetchers/fetchers.hh

@@ -126,6 +126,24 @@ struct InputScheme
 
     virtual std::optional<Input> inputFromAttrs(const Attrs & attrs) const = 0;
 
+    /**
+     * What is the name of the scheme?
+     *
+     * The `type` attribute is used to select which input scheme is
+     * used, and then the other fields are forwarded to that input
+     * scheme.
+     */
+    virtual std::string_view schemeName() const = 0;
+
+    /**
+     * Allowed attributes in an attribute set that is converted to an
+     * input.
+     *
+     * `type` is not included from this set, because the `type` field is
+      parsed first to choose which scheme; `type` is always required.
+     */
+    virtual StringSet allowedAttrs() const = 0;
+
     virtual ParsedURL toURL(const Input & input) const;
 
     virtual Input applyOverrides(
@@ -144,7 +162,7 @@ struct InputScheme
     /**
      * Is this `InputScheme` part of an experimental feature?
      */
-    virtual std::optional<ExperimentalFeature> experimentalFeature();
+    virtual std::optional<ExperimentalFeature> experimentalFeature() const;
 
     virtual bool isDirect(const Input & input) const
     { return true; }

src/libfetchers/git.cc

@@ -285,14 +285,32 @@ struct GitInputScheme : InputScheme
         return inputFromAttrs(attrs);
     }
 
+
+    std::string_view schemeName() const override
+    {
+        return "git";
+    }
+
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "url",
+            "ref",
+            "rev",
+            "shallow",
+            "submodules",
+            "lastModified",
+            "revCount",
+            "narHash",
+            "allRefs",
+            "name",
+            "dirtyRev",
+            "dirtyShortRev",
+        };
+    }
+
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        if (maybeGetStrAttr(attrs, "type") != "git") return {};
-
-        for (auto & [name, value] : attrs)
-            if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs" && name != "name" && name != "dirtyRev" && name != "dirtyShortRev")
-                throw Error("unsupported Git input attribute '%s'", name);
-
         maybeGetBoolAttr(attrs, "shallow");
         maybeGetBoolAttr(attrs, "submodules");
         maybeGetBoolAttr(attrs, "allRefs");

src/libfetchers/github.cc

@@ -27,13 +27,11 @@ std::regex hostRegex(hostRegexS, std::regex::ECMAScript);
 
 struct GitArchiveInputScheme : InputScheme
 {
-    virtual std::string type() const = 0;
-
     virtual std::optional<std::pair<std::string, std::string>> accessHeaderFromToken(const std::string & token) const = 0;
 
     std::optional<Input> inputFromURL(const ParsedURL & url, bool requireTree) const override
     {
-        if (url.scheme != type()) return {};
+        if (url.scheme != schemeName()) return {};
 
         auto path = tokenizeString<std::vector<std::string>>(url.path, "/");
 
@@ -91,7 +89,7 @@ struct GitArchiveInputScheme : InputScheme
             throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url.url, *ref, rev->gitRev());
 
         Input input;
-        input.attrs.insert_or_assign("type", type());
+        input.attrs.insert_or_assign("type", std::string { schemeName() });
         input.attrs.insert_or_assign("owner", path[0]);
         input.attrs.insert_or_assign("repo", path[1]);
         if (rev) input.attrs.insert_or_assign("rev", rev->gitRev());
@@ -101,14 +99,21 @@ struct GitArchiveInputScheme : InputScheme
         return input;
     }
 
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "owner",
+            "repo",
+            "ref",
+            "rev",
+            "narHash",
+            "lastModified",
+            "host",
+        };
+    }
+
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        if (maybeGetStrAttr(attrs, "type") != type()) return {};
-
-        for (auto & [name, value] : attrs)
-            if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "host")
-                throw Error("unsupported input attribute '%s'", name);
-
         getStrAttr(attrs, "owner");
         getStrAttr(attrs, "repo");
 
@@ -128,7 +133,7 @@ struct GitArchiveInputScheme : InputScheme
         if (ref) path += "/" + *ref;
         if (rev) path += "/" + rev->to_string(HashFormat::Base16, false);
         return ParsedURL {
-            .scheme = type(),
+            .scheme = std::string { schemeName() },
             .path = path,
         };
     }
@@ -220,7 +225,7 @@ struct GitArchiveInputScheme : InputScheme
         return {result.storePath, input};
     }
 
-    std::optional<ExperimentalFeature> experimentalFeature() override
+    std::optional<ExperimentalFeature> experimentalFeature() const override
     {
         return Xp::Flakes;
     }
@@ -228,7 +233,7 @@ struct GitArchiveInputScheme : InputScheme
 
 struct GitHubInputScheme : GitArchiveInputScheme
 {
-    std::string type() const override { return "github"; }
+    std::string_view schemeName() const override { return "github"; }
 
     std::optional<std::pair<std::string, std::string>> accessHeaderFromToken(const std::string & token) const override
     {
@@ -309,7 +314,7 @@ struct GitHubInputScheme : GitArchiveInputScheme
 
 struct GitLabInputScheme : GitArchiveInputScheme
 {
-    std::string type() const override { return "gitlab"; }
+    std::string_view schemeName() const override { return "gitlab"; }
 
     std::optional<std::pair<std::string, std::string>> accessHeaderFromToken(const std::string & token) const override
     {
@@ -377,7 +382,7 @@ struct GitLabInputScheme : GitArchiveInputScheme
 
 struct SourceHutInputScheme : GitArchiveInputScheme
 {
-    std::string type() const override { return "sourcehut"; }
+    std::string_view schemeName() const override { return "sourcehut"; }
 
     std::optional<std::pair<std::string, std::string>> accessHeaderFromToken(const std::string & token) const override
     {

src/libfetchers/indirect.cc

@@ -49,14 +49,23 @@ struct IndirectInputScheme : InputScheme
         return input;
     }
 
+    std::string_view schemeName() const override
+    {
+        return "indirect";
+    }
+
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "id",
+            "ref",
+            "rev",
+            "narHash",
+        };
+    }
+
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        if (maybeGetStrAttr(attrs, "type") != "indirect") return {};
-
-        for (auto & [name, value] : attrs)
-            if (name != "type" && name != "id" && name != "ref" && name != "rev" && name != "narHash")
-                throw Error("unsupported indirect input attribute '%s'", name);
-
         auto id = getStrAttr(attrs, "id");
         if (!std::regex_match(id, flakeRegex))
             throw BadURL("'%s' is not a valid flake ID", id);
@@ -92,7 +101,7 @@ struct IndirectInputScheme : InputScheme
         throw Error("indirect input '%s' cannot be fetched directly", input.to_string());
     }
 
-    std::optional<ExperimentalFeature> experimentalFeature() override
+    std::optional<ExperimentalFeature> experimentalFeature() const override
     {
         return Xp::Flakes;
     }

src/libfetchers/mercurial.cc

@@ -69,14 +69,25 @@ struct MercurialInputScheme : InputScheme
         return inputFromAttrs(attrs);
     }
 
+    std::string_view schemeName() const override
+    {
+        return "hg";
+    }
+
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "url",
+            "ref",
+            "rev",
+            "revCount",
+            "narHash",
+            "name",
+        };
+    }
+
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        if (maybeGetStrAttr(attrs, "type") != "hg") return {};
-
-        for (auto & [name, value] : attrs)
-            if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "revCount" && name != "narHash" && name != "name")
-                throw Error("unsupported Mercurial input attribute '%s'", name);
-
         parseURL(getStrAttr(attrs, "url"));
 
         if (auto ref = maybeGetStrAttr(attrs, "ref")) {

src/libfetchers/path.cc

@@ -32,23 +32,30 @@ struct PathInputScheme : InputScheme
         return input;
     }
 
+    std::string_view schemeName() const override
+    {
+        return "path";
+    }
+
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "path",
+            /* Allow the user to pass in "fake" tree info
+               attributes. This is useful for making a pinned tree work
+               the same as the repository from which is exported (e.g.
+               path:/nix/store/...-source?lastModified=1585388205&rev=b0c285...).
+             */
+            "rev",
+            "revCount",
+            "lastModified",
+            "narHash",
+        };
+    }
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        if (maybeGetStrAttr(attrs, "type") != "path") return {};
-
         getStrAttr(attrs, "path");
 
-        for (auto & [name, value] : attrs)
-            /* Allow the user to pass in "fake" tree info
-               attributes. This is useful for making a pinned tree
-               work the same as the repository from which is exported
-               (e.g. path:/nix/store/...-source?lastModified=1585388205&rev=b0c285...). */
-            if (name == "type" || name == "rev" || name == "revCount" || name == "lastModified" || name == "narHash" || name == "path")
-                // checked in Input::fromAttrs
-                ;
-            else
-                throw Error("unsupported path input attribute '%s'", name);
-
         Input input;
         input.attrs = attrs;
         return input;
@@ -121,7 +128,7 @@ struct PathInputScheme : InputScheme
         return {std::move(*storePath), input};
     }
 
-    std::optional<ExperimentalFeature> experimentalFeature() override
+    std::optional<ExperimentalFeature> experimentalFeature() const override
     {
         return Xp::Flakes;
     }

src/libfetchers/tarball.cc

@@ -184,7 +184,6 @@ DownloadTarballResult downloadTarball(
 // An input scheme corresponding to a curl-downloadable resource.
 struct CurlInputScheme : InputScheme
 {
-    virtual const std::string inputType() const = 0;
     const std::set<std::string> transportUrlSchemes = {"file", "http", "https"};
 
     const bool hasTarballExtension(std::string_view path) const
@@ -222,22 +221,27 @@ struct CurlInputScheme : InputScheme
         url.query.erase("rev");
         url.query.erase("revCount");
 
-        input.attrs.insert_or_assign("type", inputType());
+        input.attrs.insert_or_assign("type", std::string { schemeName() });
         input.attrs.insert_or_assign("url", url.to_string());
         return input;
     }
 
+    StringSet allowedAttrs() const override
+    {
+        return {
+            "type",
+            "url",
+            "narHash",
+            "name",
+            "unpack",
+            "rev",
+            "revCount",
+            "lastModified",
+        };
+    }
+
     std::optional<Input> inputFromAttrs(const Attrs & attrs) const override
     {
-        auto type = maybeGetStrAttr(attrs, "type");
-        if (type != inputType()) return {};
-
-        // FIXME: some of these only apply to TarballInputScheme.
-        std::set<std::string> allowedNames = {"type", "url", "narHash", "name", "unpack", "rev", "revCount", "lastModified"};
-        for (auto & [name, value] : attrs)
-            if (!allowedNames.count(name))
-                throw Error("unsupported %s input attribute '%s'", *type, name);
-
         Input input;
         input.attrs = attrs;
 
@@ -258,14 +262,14 @@ struct CurlInputScheme : InputScheme
 
 struct FileInputScheme : CurlInputScheme
 {
-    const std::string inputType() const override { return "file"; }
+    std::string_view schemeName() const override { return "file"; }
 
     bool isValidURL(const ParsedURL & url, bool requireTree) const override
     {
         auto parsedUrlScheme = parseUrlScheme(url.scheme);
         return transportUrlSchemes.count(std::string(parsedUrlScheme.transport))
             && (parsedUrlScheme.application
-                ? parsedUrlScheme.application.value() == inputType()
+                ? parsedUrlScheme.application.value() == schemeName()
                 : (!requireTree && !hasTarballExtension(url.path)));
     }
 
@@ -278,7 +282,7 @@ struct FileInputScheme : CurlInputScheme
 
 struct TarballInputScheme : CurlInputScheme
 {
-    const std::string inputType() const override { return "tarball"; }
+    std::string_view schemeName() const override { return "tarball"; }
 
     bool isValidURL(const ParsedURL & url, bool requireTree) const override
     {
@@ -286,7 +290,7 @@ struct TarballInputScheme : CurlInputScheme
 
         return transportUrlSchemes.count(std::string(parsedUrlScheme.transport))
             && (parsedUrlScheme.application
-                ? parsedUrlScheme.application.value() == inputType()
+                ? parsedUrlScheme.application.value() == schemeName()
                 : (requireTree || hasTarballExtension(url.path)));
     }