| Message ID | 20241214181306.296673-1-toke@toke.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [filter-repo] Don't crash on multi-line config values | expand |
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
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.
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
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(-)