libfetchers: make attribute / URL query handling consistent

The original idea was to fix lix#174, but for a user friendly solution,
I figured that we'd need more consistency:

* Invalid query params will cause an error, just like invalid
  attributes. This has the following two consequences:

  * The `?dir=`-param from flakes will be removed before the URL to be
    fetched is passed to libfetchers.

  * The tarball fetcher doesn't allow URLs with custom query params
    anymore. I think this was questionable anyways given that an
    arbitrary set of query params was silently removed from the URL you
    wanted to fetch. The correct way is to use an attribute-set
    with a key `url` that contains the tarball URL to fetch.

  * Same for the git & mercurial fetchers: in that case it doesn't even
    matter though: both fetchers added unused query params to the URL
    that's passed from the input scheme to the fetcher (`url2` in the code).
    It turns out that this was never used since the query parameters were
    erased again in `getActualUrl`.

* Validation happens for both attributes and URLs. Previously, a lot of
  fetchers validated e.g. refs/revs only when specified in a URL and
  the validity of attribute names only in `inputFromAttrs`.

  Now, all the validation is done in `inputFromAttrs` and `inputFromURL`
  constructs attributes that will be passed to `inputFromAttrs`.

* Accept all attributes as URL query parameters. That also includes
  lesser used ones such as `narHash`.

  And "output" attributes like `lastModified`: these could be declared
  already when declaring inputs as attribute rather than URL. Now the
  behavior is at least consistent.

  Personally, I think we should differentiate in the future between
  "fetched input" (basically the attr-set that ends up in the lock-file)
  and "unfetched input" earlier: both inputFrom{Attrs,URL} entrypoints
  are probably OK for unfetched inputs, but for locked/fetched inputs
  a custom entrypoint should be used. Then, the current entrypoints
  wouldn't have to allow these attributes anymore.

Change-Id: I1be1992249f7af8287cfc37891ab505ddaa2e8cd

3 files changed, 93 insertions, 1 deletions

diff --git a/tests/functional/fetchers.sh b/tests/functional/fetchers.sh
new file mode 100644
index 000000000..0f888dc33
--- /dev/null
+++ b/tests/functional/fetchers.sh
@@ -0,0 +1,91 @@
+source common.sh
+
+requireGit
+
+clearStore
+
+testFetchTreeError() {
+    rawFetchTreeArg="${1?fetchTree arg missing}"
+    messageSubstring="${2?messageSubstring missing}"
+
+    output="$(nix eval --impure --raw --expr "(builtins.fetchTree $rawFetchTreeArg).outPath" 2>&1)" && status=0 || status=$?
+    grepQuiet "$messageSubstring" <<<"$output"
+    test "$status" -ne 0
+}
+
+# github/gitlab/sourcehut fetcher input validation
+for provider in github gitlab sourcehut; do
+    # ref/rev validation
+    testFetchTreeError \
+        "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; ref = \",\"; }" \
+        "URL '$provider:foo/bar' contains an invalid branch/tag name"
+
+    testFetchTreeError \
+        "\"$provider://host/foo/bar/,\"" \
+        "URL '$provider:foo/bar', ',' is not a commit hash or a branch/tag name"
+
+    testFetchTreeError \
+        "\"$provider://host/foo/bar/f16d8f43dd0998cdb315a2cccf2e4d10027e7ca4?rev=abc\"" \
+        "URL '$provider://host/foo/bar/f16d8f43dd0998cdb315a2cccf2e4d10027e7ca4?rev=abc' already contains a ref or rev"
+
+    testFetchTreeError \
+        "\"$provider://host/foo/bar/ref?ref=ref2\"" \
+        "URL '$provider://host/foo/bar/ref?ref=ref2' already contains a ref or rev"
+
+    # host validation
+    testFetchTreeError \
+        "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; host = \"git_hub.com\"; }" \
+        "URL '$provider:foo/bar' contains an invalid instance host"
+
+    testFetchTreeError \
+        "\"$provider://host/foo/bar/ref?host=git_hub.com\"" \
+        "URL '$provider:foo/bar' contains an invalid instance host"
+
+    # invalid attributes
+    testFetchTreeError \
+        "{ type = \"$provider\"; owner = \"foo\"; repo = \"bar\"; wrong = true; }" \
+        "unsupported input attribute 'wrong'"
+
+    testFetchTreeError \
+        "\"$provider://host/foo/bar/ref?wrong=1\"" \
+        "unsupported input attribute 'wrong'"
+done
+
+# unsupported attributes w/ tarball fetcher
+testFetchTreeError \
+    "\"https://host/foo?wrong=1\"" \
+    "unsupported tarball input attribute 'wrong'. If you wanted to fetch a tarball with a query parameter, please use '{ type = \"tarball\"; url = \"...\"; }"
+
+# test for unsupported attributes / validation in git fetcher
+testFetchTreeError \
+    "\"git+https://github.com/owner/repo?invalid=1\"" \
+    "unsupported Git input attribute 'invalid'"
+
+testFetchTreeError \
+    "\"git+https://github.com/owner/repo?url=foo\"" \
+    "URL 'git+https://github.com/owner/repo?url=foo' must not override url via query param!"
+
+testFetchTreeError \
+    "\"git+https://github.com/owner/repo?ref=foo.lock\"" \
+    "invalid Git branch/tag name 'foo.lock'"
+
+testFetchTreeError \
+    "{ type = \"git\"; url =\"https://github.com/owner/repo\"; ref = \"foo.lock\"; }" \
+    "invalid Git branch/tag name 'foo.lock'"
+
+# same for mercurial
+testFetchTreeError \
+    "\"hg+https://forge.tld/owner/repo?invalid=1\"" \
+    "unsupported Mercurial input attribute 'invalid'"
+
+testFetchTreeError \
+    "{ type = \"hg\"; url = \"https://forge.tld/owner/repo\"; invalid = 1; }" \
+    "unsupported Mercurial input attribute 'invalid'"
+
+testFetchTreeError \
+    "\"hg+https://forge.tld/owner/repo?ref=,\"" \
+    "invalid Mercurial branch/tag name ','"
+
+testFetchTreeError \
+    "{ type = \"hg\"; url = \"https://forge.tld/owner/repo\"; ref = \",\"; }" \
+    "invalid Mercurial branch/tag name ','"
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index a13dee001..d99f9bbd3 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -93,6 +93,7 @@ functional_tests_scripts = [
   'fetchGitRefs.sh',
   'gc-runtime.sh',
   'tarball.sh',
+  'fetchers.sh',
   'fetchGit.sh',
   'fetchurl.sh',
   'fetchPath.sh',
diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix
index ca7627bd1..5deba4a12 100644
--- a/tests/nixos/tarball-flakes.nix
+++ b/tests/nixos/tarball-flakes.nix
@@ -69,7 +69,7 @@ in
 
     # Check that we got redirected to the immutable URL.
     locked_url = info["locked"]["url"]
-    assert locked_url == "http://localhost/stable/${nixpkgs.rev}.tar.gz", f"{locked_url=} != http://localhost/stable/${nixpkgs.rev}.tar.gz"
+    assert locked_url == "http://localhost/stable/${nixpkgs.rev}.tar.gz?rev=${nixpkgs.rev}&revCount=1234", f"{locked_url=} != http://localhost/stable/${nixpkgs.rev}.tar.gz"
 
     # Check that we got the rev and revCount attributes.
     revision = info["revision"]