From 0844856c8487e2bdf66a2272622f6bbf6b8b5fb8 Mon Sep 17 00:00:00 2001
From: Yorick van Pelt <yorick@yorickvanpelt.nl>
Date: Tue, 7 Feb 2023 16:44:37 +0100
Subject: [PATCH] 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<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);