{"thread":{"id":"62639","subject":"[filter-repo PATCH] Don't crash on multi-line config values","startedAt":"2024-12-14T18:13:22Z","lastAt":"2025-01-28T18:30:43Z","messageCount":3,"participants":["Toke Høiland-Jørgensen","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"509118","messageId":"20241214181306.296673-1-toke@toke.dk","threadId":"62639","inReplyTo":null,"subject":"[filter-repo PATCH] Don't crash on multi-line config values","fromName":"Toke Høiland-Jørgensen","fromEmail":"toke@toke.dk","sentAt":"2024-12-14T18:13:06Z","receivedAt":"2024-12-14T18:13:22Z","isPatch":true,"sender":{"key":"toke@toke.dk","avatar":null},"body":"The parsing of the output of `git config --list` fails if any of the\nconfig values contain newlines. Fix this by using the --null parameter\nto `git config`, which is designed for this purpose.\n\nAdd a simple test that causes the crash pre-patch.\n\nSigned-off-by: Toke Høiland-Jørgensen \n---\n git-filter-repo | 6 +++---\n t/t9390-filter-repo-basics.sh | 11 +++++++++++\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-filter-repo b/git-filter-repo\nindex a40bce548d2c..3b75eadd81d7 100755\n--- a/git-filter-repo\n+++ b/git-filter-repo\n@@ -1683,14 +1683,14 @@ class GitUtils(object):\n def get_config_settings(repo_working_dir):\n output = ''\n try:\n- output = subproc.check_output('git config --list'.split(),\n+ output = subproc.check_output('git config --list --null'.split(),\n cwd=repo_working_dir)\n except subprocess.CalledProcessError as e: # pragma: no cover\n raise SystemExit('fatal: {}'.format(e))\n \n # FIXME: Ignores multi-valued keys, just let them overwrite for now\n- return dict(line.split(b'=', maxsplit=1)\n- for line in output.strip().split(b\"\\n\"))\n+ return dict(item.split(b'\\n', maxsplit=1)\n+ for item in output.strip().split(b\"\\0\") if item)\n \n @staticmethod\n def get_blob_sizes(quiet = False):\ndiff --git a/t/t9390-filter-repo-basics.sh b/t/t9390-filter-repo-basics.sh\nindex c129799fb6a5..1dc2dca789ab 100755\n--- a/t/t9390-filter-repo-basics.sh\n+++ b/t/t9390-filter-repo-basics.sh\n@@ -895,4 +895,15 @@ test_expect_success 'origin refs without origin remote does not die' '\n \t)\n '\n \n+test_expect_success 'multi-line config value' '\n+\ttest_create_repo multiline_config &&\n+\t(\n+\t\tcd multiline_config &&\n+\n+\t\tgit config set test.test \"test\n+test\" &&\n+\t\tgit filter-repo --force\n+\t)\n+'\n+\n test_done\n-- \n2.47.1\n\n"},{"id":"511361","messageId":"87sep2olti.fsf@toke.dk","threadId":"62639","inReplyTo":"20241214181306.296673-1-toke@toke.dk","subject":"Re: [filter-repo PATCH] Don't crash on multi-line config values","fromName":"Toke Høiland-Jørgensen","fromEmail":"toke@toke.dk","sentAt":"2025-01-28T18:06:01Z","receivedAt":"2025-01-28T18:06:09Z","isPatch":true,"sender":{"key":"toke@toke.dk","avatar":null},"body":"Toke Høiland-Jørgensen writes:\n\n> The parsing of the output of `git config --list` fails if any of the\n> config values contain newlines. Fix this by using the --null parameter\n> to `git config`, which is designed for this purpose.\n>\n> Add a simple test that causes the crash pre-patch.\n\nHi Elijah\n\nAny chance you could merge this? Seems a couple of others found this bug\nas well, there are two open PRs on github with alternative fixes for the\nsame issue...\n\n-Toke\n"},{"id":"511364","messageId":"CABPp-BHZ99ovFUfBUpvGRvhoudTbz4vZgCdVr=iUcE171CAYuQ@mail.gmail.com","threadId":"62639","inReplyTo":"20241214181306.296673-1-toke@toke.dk","subject":"Re: [filter-repo PATCH] Don't crash on multi-line config values","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-01-28T18:30:31Z","receivedAt":"2025-01-28T18:30:43Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Dec 14, 2024 at 10:13 AM Toke Høiland-Jørgensen wrote:\n>\n> The parsing of the output of `git config --list` fails if any of the\n> config values contain newlines. Fix this by using the --null parameter\n> to `git config`, which is designed for this purpose.\n\nNice; that's a cleaner way to do it than the other proposals.\n\n> Add a simple test that causes the crash pre-patch.\n\nMuch appreciated.\n\n> Signed-off-by: Toke Høiland-Jørgensen \n> ---\n> git-filter-repo | 6 +++---\n> t/t9390-filter-repo-basics.sh | 11 +++++++++++\n> 2 files changed, 14 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-filter-repo b/git-filter-repo\n> index a40bce548d2c..3b75eadd81d7 100755\n> --- a/git-filter-repo\n> +++ b/git-filter-repo\n> @@ -1683,14 +1683,14 @@ class GitUtils(object):\n> def get_config_settings(repo_working_dir):\n> output = ''\n> try:\n> - output = subproc.check_output('git config --list'.split(),\n> + output = subproc.check_output('git config --list --null'.split(),\n> cwd=repo_working_dir)\n> except subprocess.CalledProcessError as e: # pragma: no cover\n> raise SystemExit('fatal: {}'.format(e))\n>\n> # FIXME: Ignores multi-valued keys, just let them overwrite for now\n> - return dict(line.split(b'=', maxsplit=1)\n> - for line in output.strip().split(b\"\\n\"))\n> + return dict(item.split(b'\\n', maxsplit=1)\n> + for item in output.strip().split(b\"\\0\") if item)\n>\n> @staticmethod\n> def get_blob_sizes(quiet = False):\n> diff --git a/t/t9390-filter-repo-basics.sh b/t/t9390-filter-repo-basics.sh\n> index c129799fb6a5..1dc2dca789ab 100755\n> --- a/t/t9390-filter-repo-basics.sh\n> +++ b/t/t9390-filter-repo-basics.sh\n> @@ -895,4 +895,15 @@ test_expect_success 'origin refs without origin remote does not die' '\n> )\n> '\n>\n> +test_expect_success 'multi-line config value' '\n> + test_create_repo multiline_config &&\n> + (\n> + cd multiline_config &&\n> +\n> + git config set test.test \"test\n> +test\" &&\n> + git filter-repo --force\n> + )\n> +'\n> +\n> test_done\n> --\n> 2.47.1\n\nLooks good to me; applied.\n"}]}