From 1f024ecfcdc248214c5b4ad8f7b1c8a959fd522c Mon Sep 17 00:00:00 2001 From: Bryan Honof Date: Mon, 30 Sep 2024 14:42:47 +0200 Subject: [PATCH 1/2] fix: warn on malformed URI query parameter --- src/libflake/flake/flakeref.cc | 2 +- src/libutil/url.cc | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index a57fce9f3..01fe747f9 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -88,7 +88,7 @@ std::pair parsePathFlakeRefWithFragment( if (fragmentStart != std::string::npos) { fragment = percentDecode(url.substr(fragmentStart+1)); } - if (pathEnd != std::string::npos && fragmentStart != std::string::npos) { + if (pathEnd != std::string::npos && fragmentStart != std::string::npos && url[pathEnd] == '?') { query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1)); } diff --git a/src/libutil/url.cc b/src/libutil/url.cc index bcbe9ea4e..9ed49dcbe 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -79,10 +79,14 @@ std::map decodeQuery(const std::string & query) for (auto s : tokenizeString(query, "&")) { auto e = s.find('='); - if (e != std::string::npos) - result.emplace( - s.substr(0, e), - percentDecode(std::string_view(s).substr(e + 1))); + if (e == std::string::npos) { + warn("dubious URI query '%s' is missing equal sign '%s', ignoring", s, "="); + continue; + } + + result.emplace( + s.substr(0, e), + percentDecode(std::string_view(s).substr(e + 1))); } return result; From 5150a962f53cfd702315ed96fb5ce395af6e94a4 Mon Sep 17 00:00:00 2001 From: Bryan Honof Date: Mon, 30 Sep 2024 14:43:15 +0200 Subject: [PATCH 2/2] test(functional/flakes): add dubious-query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim --- tests/functional/flakes/dubious-query.sh | 31 ++++++++++++++++++++++++ tests/functional/flakes/local.mk | 3 ++- tests/functional/flakes/meson.build | 1 + 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/functional/flakes/dubious-query.sh diff --git a/tests/functional/flakes/dubious-query.sh b/tests/functional/flakes/dubious-query.sh new file mode 100644 index 000000000..b481b5f81 --- /dev/null +++ b/tests/functional/flakes/dubious-query.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash + +source ./common.sh + +requireGit + +repoDir="$TEST_ROOT/repo" +createGitRepo "$repoDir" +createSimpleGitFlake "$repoDir" + +# Check that a flakeref without a query is accepted correctly. +expectStderr 0 nix --offline build --dry-run "git+file://$repoDir#foo" + +# Check that a flakeref with a good query is accepted correctly. +expectStderr 0 nix --offline build --dry-run "git+file://$repoDir?foo=bar#foo" + +# Check that we get the dubious query warning, when passing in a query without an equal sign. +expectStderr 0 nix --offline build --dry-run "git+file://$repoDir?bar#foo" \ + | grepQuiet "warning: dubious URI query 'bar' is missing equal sign '=', ignoring" + +# Check that the anchor (#) is taken as a whole, not split, and throws an error. +expectStderr 1 nix --offline build --dry-run "git+file://$repoDir#foo?bar" \ + | grepQuiet "error: flake 'git+file://$repoDir' does not provide attribute 'packages.$system.foo?bar', 'legacyPackages.$system.foo?bar' or 'foo?bar'" + +# Check that a literal `?` in the query doesn't print dubious query warning. +expectStderr 0 nix --offline build --dry-run "git+file://$repoDir?#foo" \ + | grepInverse "warning: dubious URI query " + +# Check that a literal `?=` in the query doesn't print dubious query warning. +expectStderr 0 nix --offline build --dry-run "git+file://$repoDir?=#foo" \ + | grepInverse "warning: dubious URI query " diff --git a/tests/functional/flakes/local.mk b/tests/functional/flakes/local.mk index 71e50ad07..a37840240 100644 --- a/tests/functional/flakes/local.mk +++ b/tests/functional/flakes/local.mk @@ -19,6 +19,7 @@ flake-tests := \ $(d)/eval-cache.sh \ $(d)/search-root.sh \ $(d)/config.sh \ - $(d)/show.sh + $(d)/show.sh \ + $(d)/dubious-query.sh install-tests-groups += flake diff --git a/tests/functional/flakes/meson.build b/tests/functional/flakes/meson.build index 8c1afd6ff..4f455fcce 100644 --- a/tests/functional/flakes/meson.build +++ b/tests/functional/flakes/meson.build @@ -23,6 +23,7 @@ suites += { 'search-root.sh', 'config.sh', 'show.sh', + 'dubious-query.sh', ], 'workdir': meson.current_build_dir(), }