Fix the parsing of path flake refs with a query

`/path?foo=bar` was parsed as `/path`, omitting the query altogether
(`/path?foo=bar#` was parsed correctly though).

Rewrite the path flake ref parser to fix this

src/libexpr/flake/flakeref.cc

@@ -55,7 +55,7 @@ FlakeRef parseFlakeRef(
 {
     auto [flakeRef, fragment] = parseFlakeRefWithFragment(url, baseDir, allowMissing, isFlake);
     if (fragment != "")
-        throw Error("unexpected fragment '%s' in flake reference '%s'", fragment, url);
+        throw FlakeRefError("unexpected fragment '%s' in flake reference '%s'", fragment, url);
     return flakeRef;
 }
 
@@ -78,19 +78,25 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
     std::string path = url;
     std::string fragment = "";
     std::map<std::string, std::string> query;
-    auto pathEnd = url.find_first_of("#?");
-    auto fragmentStart = pathEnd;
-    if (pathEnd != std::string::npos && url[pathEnd] == '?') {
-        fragmentStart = url.find("#");
-    }
+    auto pathEnd = url.find_first_of("?#");
     if (pathEnd != std::string::npos) {
+        // There's something (either a query string or a fragment) in addition
+        // to the path
         path = url.substr(0, pathEnd);
-    }
-    if (fragmentStart != std::string::npos) {
-        fragment = percentDecode(url.substr(fragmentStart+1));
-    }
-    if (pathEnd != std::string::npos && fragmentStart != std::string::npos) {
-        query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1));
+        std::string non_path_part = url.substr(pathEnd + 1);
+        if (url[pathEnd] == '#') {
+            // Not query, just a fragment
+            fragment = percentDecode(non_path_part);
+        } else {
+            // We have a query, and maybe a fragment too
+            auto fragmentStart = non_path_part.find("#");
+            if (fragmentStart != std::string::npos) {
+                query = decodeQuery(non_path_part.substr(0, fragmentStart));
+                fragment = percentDecode(non_path_part.substr(fragmentStart+1));
+            } else {
+                query = decodeQuery(non_path_part);
+            }
+        }
     }
 
     if (baseDir) {
@@ -113,10 +119,10 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
                         found = true;
                         break;
                     } else if (pathExists(path + "/.git"))
-                        throw Error("path '%s' is not part of a flake (neither it nor its parent directories contain a 'flake.nix' file)", path);
+                        throw FlakeRefError("path '%s' is not part of a flake (neither it nor its parent directories contain a 'flake.nix' file)", path);
                     else {
                         if (lstat(path).st_dev != device)
-                            throw Error("unable to find a flake before encountering filesystem boundary at '%s'", path);
+                            throw FlakeRefError("unable to find a flake before encountering filesystem boundary at '%s'", path);
                     }
                     path = dirOf(path);
                 }
@@ -148,7 +154,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
 
                     if (subdir != "") {
                         if (parsedURL.query.count("dir"))
-                            throw Error("flake URL '%s' has an inconsistent 'dir' parameter", url);
+                            throw FlakeRefError("flake URL '%s' has an inconsistent 'dir' parameter", url);
                         parsedURL.query.insert_or_assign("dir", subdir);
                     }
 
@@ -171,11 +177,16 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
         path = canonPath(path + "/" + getOr(query, "dir", ""));
     }
 
-    fetchers::Attrs attrs;
-    attrs.insert_or_assign("type", "path");
-    attrs.insert_or_assign("path", path);
+    auto parsedURL = ParsedURL{
+        .url = url,
+        .base = url,
+        .scheme = "path",
+        .authority = "",
+        .path = path,
+        .query = query,
+    };
 
-    return std::make_pair(FlakeRef(fetchers::Input::fromAttrs(std::move(attrs)), ""), fragment);
+    return std::make_pair(FlakeRef(fetchers::Input::fromURL(parsedURL), getOr(parsedURL.query, "dir", "")), fragment);
 };
 
 

src/libexpr/flake/flakeref.hh

@@ -15,6 +15,8 @@ class Store;
 
 typedef std::string FlakeId;
 
+MakeError(FlakeRefError, Error);
+
 /**
  * A flake reference specifies how to fetch a flake or raw source
  * (e.g. from a Git repository).  It is created from a URL-like syntax

tests/functional/flakes/inputs.sh

@@ -24,7 +24,7 @@ test_subdir_self_path() {
 }
 EOF
     (
-        nix build $baseDir?dir=b-low --no-link
+        nix build $baseDir/b-low --no-link
     )
 }
 test_subdir_self_path

tests/unit/libexpr/flake/flakeref.cc

@@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 
+#include "file-system.hh"
 #include "flake/flakeref.hh"
 
 namespace nix {
@@ -10,7 +11,7 @@ namespace nix {
      * to_string
      * --------------------------------------------------------------------------*/
 
-    TEST(to_string, doesntReencodeUrl) {
+    TEST(flakeRef, to_string_doesntReencodeUrl) {
         auto s = "http://localhost:8181/test/+3d.tar.gz";
         auto flakeref = parseFlakeRef(s);
         auto parsed = flakeref.to_string();
@@ -19,4 +20,69 @@ namespace nix {
         ASSERT_EQ(parsed, expected);
     }
 
+    TEST(flakeRef, simplePath) {
+        auto raw_ref = "/foo/bar";
+
+        auto flakeref = parseFlakeRef(raw_ref);
+        auto flakeref_attrs = flakeref.toAttrs();
+        ASSERT_EQ(flakeref.input.getType(), "path");
+        ASSERT_EQ(fetchers::getStrAttr(flakeref_attrs, "path"), "/foo/bar");
+        ASSERT_EQ(fetchers::maybeGetIntAttr(flakeref_attrs, "lastModified"), std::nullopt);
+    }
+
+    TEST(flakeRef, pathWithQuery) {
+        auto raw_ref = "/foo/bar?lastModified=5";
+
+        auto flakeref = parseFlakeRef(raw_ref);
+        auto flakeref_attrs = flakeref.toAttrs();
+        EXPECT_EQ(flakeref.input.getType(), "path");
+        EXPECT_EQ(fetchers::getStrAttr(flakeref_attrs, "path"), "/foo/bar");
+        EXPECT_EQ(fetchers::getIntAttr(flakeref_attrs, "lastModified"), 5);
+    }
+
+    TEST(flakeRef, pathWithQueryAndEmptyFragment) {
+        auto raw_ref = "/foo/bar?lastModified=5#";
+
+        auto flakeref = parseFlakeRef(raw_ref);
+        auto flakeref_attrs = flakeref.toAttrs();
+        EXPECT_EQ(flakeref.input.getType(), "path");
+        EXPECT_EQ(fetchers::getStrAttr(flakeref_attrs, "path"), "/foo/bar");
+        EXPECT_EQ(fetchers::getIntAttr(flakeref_attrs, "lastModified"), 5);
+    }
+
+    TEST(flakeRef, pathWithFragment) {
+        auto raw_ref = "/foo/bar?lastModified=5#foo";
+
+        ASSERT_THROW(
+            parseFlakeRef(raw_ref),
+            FlakeRefError
+        );
+    }
+
+    TEST(flakeRef, relativePath) {
+        Path tmpDir = createTempDir();
+        AutoDelete delTmpDir(tmpDir);
+
+        // Relative path flakerefs require a `flake.nix`
+        writeFile(tmpDir + "/flake.nix", "");
+        createDirs(tmpDir + "/foo");
+
+        std::vector<std::string> raw_refs = {
+            ".?lastModified=5",
+            "./foo?lastModified=5",
+            "./foo?lastModified=5#",
+            tmpDir + "?lastModified=5",
+            fmt("../%s/?lastModified=5", baseNameOf(tmpDir)),
+            "./foo/..?lastModified=5"
+        };
+
+        for (auto raw_ref : raw_refs) {
+            auto flakeref = parseFlakeRef(raw_ref, tmpDir);
+            auto flakeref_attrs = flakeref.toAttrs();
+            EXPECT_EQ(flakeref.input.getType(), "path");
+            EXPECT_EQ(fetchers::getStrAttr(flakeref_attrs, "path"), tmpDir);
+            EXPECT_EQ(fetchers::getIntAttr(flakeref_attrs, "lastModified"), 5);
+        }
+    }
+
 }

tests/unit/libexpr/primops.cc

@@ -3,6 +3,7 @@
 
 #include "eval-settings.hh"
 #include "memory-input-accessor.hh"
+#include "flake/flakeref.hh"
 
 #include "tests/libexpr.hh"