Re: [filter-repo PATCH] Don't crash on multi-line config values

From: Elijah Newren <newren@gmail.com>
Date: Jan 28, 2025, 18:30 UTC
Message-ID: <CABPp-BHZ99ovFUfBUpvGRvhoudTbz4vZgCdVr=iUcE171CAYuQ@mail.gmail.com>
In-Reply-To: <20241214181306.296673-1-toke@toke.dk>

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.

Show 50 quoted lines

> 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.