* [filter-repo PATCH] Don't crash on multi-line config values
@ 2024-12-14 18:13 Toke Høiland-Jørgensen
2025-01-28 18:06 ` Toke Høiland-Jørgensen
2025-01-28 18:30 ` Elijah Newren
0 siblings, 2 replies; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2024-12-14 18:13 UTC (permalink / raw)
To: Elijah Newren; +Cc: git, Toke Høiland-Jørgensen
The parsing of the output of `git config --list` fails if any of the
config values contain newlines. Fix this by using the --null parameter
to `git config`, which is designed for this purpose.
Add a simple test that causes the crash pre-patch.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke•dk>
---
git-filter-repo | 6 +++---
t/t9390-filter-repo-basics.sh | 11 +++++++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/git-filter-repo b/git-filter-repo
index a40bce548d2c..3b75eadd81d7 100755
--- a/git-filter-repo
+++ b/git-filter-repo
@@ -1683,14 +1683,14 @@ class GitUtils(object):
def get_config_settings(repo_working_dir):
output = ''
try:
- output = subproc.check_output('git config --list'.split(),
+ output = subproc.check_output('git config --list --null'.split(),
cwd=repo_working_dir)
except subprocess.CalledProcessError as e: # pragma: no cover
raise SystemExit('fatal: {}'.format(e))
# FIXME: Ignores multi-valued keys, just let them overwrite for now
- return dict(line.split(b'=', maxsplit=1)
- for line in output.strip().split(b"\n"))
+ return dict(item.split(b'\n', maxsplit=1)
+ for item in output.strip().split(b"\0") if item)
@staticmethod
def get_blob_sizes(quiet = False):
diff --git a/t/t9390-filter-repo-basics.sh b/t/t9390-filter-repo-basics.sh
index c129799fb6a5..1dc2dca789ab 100755
--- a/t/t9390-filter-repo-basics.sh
+++ b/t/t9390-filter-repo-basics.sh
@@ -895,4 +895,15 @@ test_expect_success 'origin refs without origin remote does not die' '
)
'
+test_expect_success 'multi-line config value' '
+ test_create_repo multiline_config &&
+ (
+ cd multiline_config &&
+
+ git config set test.test "test
+test" &&
+ git filter-repo --force
+ )
+'
+
test_done
--
2.47.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [filter-repo PATCH] Don't crash on multi-line config values
2024-12-14 18:13 [filter-repo PATCH] Don't crash on multi-line config values Toke Høiland-Jørgensen
@ 2025-01-28 18:06 ` Toke Høiland-Jørgensen
2025-01-28 18:30 ` Elijah Newren
1 sibling, 0 replies; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2025-01-28 18:06 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Toke Høiland-Jørgensen <toke@toke•dk> writes:
> The parsing of the output of `git config --list` fails if any of the
> config values contain newlines. Fix this by using the --null parameter
> to `git config`, which is designed for this purpose.
>
> Add a simple test that causes the crash pre-patch.
Hi Elijah
Any chance you could merge this? Seems a couple of others found this bug
as well, there are two open PRs on github with alternative fixes for the
same issue...
-Toke
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [filter-repo PATCH] Don't crash on multi-line config values
2024-12-14 18:13 [filter-repo PATCH] Don't crash on multi-line config values Toke Høiland-Jørgensen
2025-01-28 18:06 ` Toke Høiland-Jørgensen
@ 2025-01-28 18:30 ` Elijah Newren
1 sibling, 0 replies; 3+ messages in thread
From: Elijah Newren @ 2025-01-28 18:30 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: git
On Sat, Dec 14, 2024 at 10:13 AM Toke Høiland-Jørgensen <toke@toke•dk> wrote:
>
> The parsing of the output of `git config --list` fails if any of the
> config values contain newlines. Fix this by using the --null parameter
> to `git config`, which is designed for this purpose.
Nice; that's a cleaner way to do it than the other proposals.
> Add a simple test that causes the crash pre-patch.
Much appreciated.
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke•dk>
> ---
> git-filter-repo | 6 +++---
> t/t9390-filter-repo-basics.sh | 11 +++++++++++
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/git-filter-repo b/git-filter-repo
> index a40bce548d2c..3b75eadd81d7 100755
> --- a/git-filter-repo
> +++ b/git-filter-repo
> @@ -1683,14 +1683,14 @@ class GitUtils(object):
> def get_config_settings(repo_working_dir):
> output = ''
> try:
> - output = subproc.check_output('git config --list'.split(),
> + output = subproc.check_output('git config --list --null'.split(),
> cwd=repo_working_dir)
> except subprocess.CalledProcessError as e: # pragma: no cover
> raise SystemExit('fatal: {}'.format(e))
>
> # FIXME: Ignores multi-valued keys, just let them overwrite for now
> - return dict(line.split(b'=', maxsplit=1)
> - for line in output.strip().split(b"\n"))
> + return dict(item.split(b'\n', maxsplit=1)
> + for item in output.strip().split(b"\0") if item)
>
> @staticmethod
> def get_blob_sizes(quiet = False):
> diff --git a/t/t9390-filter-repo-basics.sh b/t/t9390-filter-repo-basics.sh
> index c129799fb6a5..1dc2dca789ab 100755
> --- a/t/t9390-filter-repo-basics.sh
> +++ b/t/t9390-filter-repo-basics.sh
> @@ -895,4 +895,15 @@ test_expect_success 'origin refs without origin remote does not die' '
> )
> '
>
> +test_expect_success 'multi-line config value' '
> + test_create_repo multiline_config &&
> + (
> + cd multiline_config &&
> +
> + git config set test.test "test
> +test" &&
> + git filter-repo --force
> + )
> +'
> +
> test_done
> --
> 2.47.1
Looks good to me; applied.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-28 18:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-14 18:13 [filter-repo PATCH] Don't crash on multi-line config values Toke Høiland-Jørgensen
2025-01-28 18:06 ` Toke Høiland-Jørgensen
2025-01-28 18:30 ` Elijah Newren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox