From 0844856c8487e2bdf66a2272622f6bbf6b8b5fb8 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Tue, 7 Feb 2023 16:44:37 +0100 Subject: [PATCH 1/3] url: make percentEncode stricter, expose and unit test it --- src/libutil/tests/url.cc | 33 +++++++++++++++++++++++++++++++++ src/libutil/url.cc | 17 +++++++++++------ src/libutil/url.hh | 1 + 3 files changed, 45 insertions(+), 6 deletions(-) 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 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 & 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 decodeQuery(const std::string & query); From 2c0866fc3f4583c178617f764cd1fa423737a14e Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Tue, 7 Feb 2023 16:45:01 +0100 Subject: [PATCH 2/3] fetchTree: convert fs path to url via ParsedURL::to_string --- src/libexpr/primops/fetchTree.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) 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 #include @@ -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) From ae5082bbba5ddc7ac06e8b917c5a3dce267c66b7 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Mon, 27 Feb 2023 15:33:54 +0100 Subject: [PATCH 3/3] tests/fetchGit: add regression test for #7707 --- tests/fetchGit.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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")