From 1d7a57cfd9bf40658e2f35c13146d746a1011020 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 17 Aug 2023 13:40:57 -0700 Subject: [PATCH 1/2] libexpr/tests: test that parseFlakeRef doesn't percent-encode twice --- src/libexpr/tests/flakeref.cc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/libexpr/tests/flakeref.cc diff --git a/src/libexpr/tests/flakeref.cc b/src/libexpr/tests/flakeref.cc new file mode 100644 index 000000000..2b7809b93 --- /dev/null +++ b/src/libexpr/tests/flakeref.cc @@ -0,0 +1,22 @@ +#include + +#include "flake/flakeref.hh" + +namespace nix { + +/* ----------- tests for flake/flakeref.hh --------------------------------------------------*/ + + /* ---------------------------------------------------------------------------- + * to_string + * --------------------------------------------------------------------------*/ + + TEST(to_string, doesntReencodeUrl) { + auto s = "http://localhost:8181/test/+3d.tar.gz"; + auto flakeref = parseFlakeRef(s); + auto parsed = flakeref.to_string(); + auto expected = "http://localhost:8181/test/%2B3d.tar.gz"; + + ASSERT_EQ(parsed, expected); + } + +} From 73696ec71678433dac87863bab36b66701d4b6ed Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 17 Aug 2023 13:21:38 -0700 Subject: [PATCH 2/2] libutil: fix double-encoding of URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you have a URL that needs to be percent-encoded, such as `http://localhost:8181/test/+3d.tar.gz`, and try to lock that in a Nix flake such as the following: { inputs.test = { url = "http://localhost:8181/test/+3d.tar.gz"; flake = false; }; outputs = { test, ... }: { t = builtins.readFile test; }; } running `nix flake metadata` shows that the input URL has been incorrectly double-encoded (despite the flake.lock being correctly encoded only once): [...snip...] Inputs: └───test: http://localhost:8181/test/%252B3d.tar.gz?narHash=sha256-EFUdrtf6Rn0LWIJufrmg8q99aT3jGfLvd1//zaJEufY%3D (Notice the `%252B`? That's just `%2B` but percent-encoded again) With this patch, the double-encoding is gone; running `nix flake metadata` will show the proper URL: [...snip...] Inputs: └───test: http://localhost:8181/test/%2B3d.tar.gz?narHash=sha256-EFUdrtf6Rn0LWIJufrmg8q99aT3jGfLvd1//zaJEufY%3D --- As far as I can tell, this happens because Nix already percent-encodes the URL and stores this as the value of `inputs.asdf.url`. However, when Nix later tries to read this out of the eval state as a string (via `getStrAttr`), it has to run it through `parseURL` again to get the `ParsedURL` structure. Now, this itself isn't a problem -- the true problem arises when using `ParsedURL::to_string` later, which then _re-escapes the path_. It is at this point that what would have been `%2B` (`+`) becomes `%252B` (`%2B`). --- src/libutil/url.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 9e44241ac..a8f7d39fd 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -44,7 +44,7 @@ ParsedURL parseURL(const std::string & url) .base = base, .scheme = scheme, .authority = authority, - .path = path, + .path = percentDecode(path), .query = decodeQuery(query), .fragment = percentDecode(std::string(fragment)) };