Merge pull request #7776 from yorickvP/fix-path-escapes-7707

Properly escape local paths into URLs in fetchTree

5 files changed, 70 insertions, 7 deletions

diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc
index 2e924c302..c9faf3ffb 100644
--- a/src/libexpr/primops/fetchTree.cc
+++ b/src/libexpr/primops/fetchTree.cc
@@ -4,6 +4,7 @@
 #include "fetchers.hh"
 #include "filetransfer.hh"
 #include "registry.hh"
+#include "url.hh"
 
 #include <ctime>
 #include <iomanip>
@@ -68,7 +69,16 @@ void emitTreeAttrs(
 std::string fixURI(std::string uri, EvalState & state, const std::string & defaultScheme = "file")
 {
     state.checkURI(uri);
-    return uri.find("://") != std::string::npos ? uri : defaultScheme + "://" + uri;
+    if (uri.find("://") == std::string::npos) {
+        const auto p = ParsedURL {
+            .scheme = defaultScheme,
+            .authority = "",
+            .path = uri
+        };
+        return p.to_string();
+    } else {
+        return uri;
+    }
 }
 
 std::string fixURIForGit(std::string uri, EvalState & state)
diff --git a/src/libutil/tests/url.cc b/src/libutil/tests/url.cc
index e0c438b4d..a908631e6 100644
--- a/src/libutil/tests/url.cc
+++ b/src/libutil/tests/url.cc
@@ -302,4 +302,37 @@ namespace nix {
         ASSERT_EQ(d, s);
     }
 
+
+    /* ----------------------------------------------------------------------------
+     * percentEncode
+     * --------------------------------------------------------------------------*/
+
+    TEST(percentEncode, encodesUrlEncodedString) {
+        std::string s = percentEncode("==@==");
+        std::string d = "%3D%3D%40%3D%3D";
+        ASSERT_EQ(d, s);
+    }
+
+    TEST(percentEncode, keepArgument) {
+        std::string a = percentEncode("abd / def");
+        std::string b = percentEncode("abd / def", "/");
+        ASSERT_EQ(a, "abd%20%2F%20def");
+        ASSERT_EQ(b, "abd%20/%20def");
+    }
+
+    TEST(percentEncode, inverseOfDecode) {
+        std::string original = "%3D%3D%40%3D%3D";
+        std::string once = percentEncode(original);
+        std::string back = percentDecode(once);
+
+        ASSERT_EQ(back, original);
+    }
+
+    TEST(percentEncode, trailingPercent) {
+        std::string s = percentEncode("==@==%");
+        std::string d = "%3D%3D%40%3D%3D%25";
+
+        ASSERT_EQ(d, s);
+    }
+
 }
diff --git a/src/libutil/url.cc b/src/libutil/url.cc
index 4e43455e1..9e44241ac 100644
--- a/src/libutil/url.cc
+++ b/src/libutil/url.cc
@@ -88,17 +88,22 @@ std::map<std::string, std::string> decodeQuery(const std::string & query)
     return result;
 }
 
-std::string percentEncode(std::string_view s)
+const static std::string allowedInQuery = ":@/?";
+const static std::string allowedInPath = ":@/";
+
+std::string percentEncode(std::string_view s, std::string_view keep)
 {
     std::string res;
     for (auto & c : s)
+        // unreserved + keep
         if ((c >= 'a' && c <= 'z')
             || (c >= 'A' && c <= 'Z')
             || (c >= '0' && c <= '9')
-            || strchr("-._~!$&'()*+,;=:@", c))
+            || strchr("-._~", c)
+            || keep.find(c) != std::string::npos)
             res += c;
         else
-            res += fmt("%%%02x", (unsigned int) c);
+            res += fmt("%%%02X", (unsigned int) c);
     return res;
 }
 
@@ -109,9 +114,9 @@ std::string encodeQuery(const std::map<std::string, std::string> & ss)
     for (auto & [name, value] : ss) {
         if (!first) res += '&';
         first = false;
-        res += percentEncode(name);
+        res += percentEncode(name, allowedInQuery);
         res += '=';
-        res += percentEncode(value);
+        res += percentEncode(value, allowedInQuery);
     }
     return res;
 }
@@ -122,7 +127,7 @@ std::string ParsedURL::to_string() const
         scheme
         + ":"
         + (authority ? "//" + *authority : "")
-        + path
+        + percentEncode(path, allowedInPath)
         + (query.empty() ? "" : "?" + encodeQuery(query))
         + (fragment.empty() ? "" : "#" + percentEncode(fragment));
 }
diff --git a/src/libutil/url.hh b/src/libutil/url.hh
index 2a9fb34c1..ddd673d65 100644
--- a/src/libutil/url.hh
+++ b/src/libutil/url.hh
@@ -22,6 +22,7 @@ struct ParsedURL
 MakeError(BadURL, Error);
 
 std::string percentDecode(std::string_view in);
+std::string percentEncode(std::string_view s, std::string_view keep="");
 
 std::map<std::string, std::string> decodeQuery(const std::string & query);
 
diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh
index da09c3f37..a7a8df186 100644
--- a/tests/fetchGit.sh
+++ b/tests/fetchGit.sh
@@ -237,3 +237,17 @@ rm -rf $repo/.git
 # should succeed for a repo without commits
 git init $repo
 path10=$(nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath")
+
+# should succeed for a path with a space
+# regression test for #7707
+repo="$TEST_ROOT/a b"
+git init "$repo"
+git -C "$repo" config user.email "foobar@example.com"
+git -C "$repo" config user.name "Foobar"
+
+echo utrecht > "$repo/hello"
+touch "$repo/.gitignore"
+git -C "$repo" add hello .gitignore
+git -C "$repo" commit -m 'Bla1'
+cd "$repo"
+path11=$(nix eval --impure --raw --expr "(builtins.fetchGit ./.).outPath")