[PATCH v2] apply: normalize path in --directory argument

["Joaquim Rocha via GitGitGadget" <gitgitgadget@gmail.com>]

From: Joaquim Rocha <joaquim@amutable.com>

When passing a relative path like --directory=./some/sub, the leading
"./" caused apply to prepend it literally to patch filenames, resulting
in an error (invalid path).
There may be more cases like this where users pass some/./path to the
directory which can easily be normalized to an acceptable path, so
these changes try to normalize the path before using it.

Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
---
    apply: strip ./ prefix from --directory argument
    
    Changes since v1:
    
     * Normalized the path as Patrick recommended
     * Return error is we have ../ at the beginning of the --directory
       argument

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2198%2Fjoaquimrocha%2Fapply-directory-dot-slash-prefix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2198/joaquimrocha/apply-directory-dot-slash-prefix-v2
Pull-Request: https://github.com/git/git/pull/2198

Range-diff vs v1:

 1:  82e24c3471 ! 1:  8ac66a876d apply: strip ./ prefix from --directory argument
     @@ Metadata
      Author: Joaquim Rocha <joaquim@amutable.com>
      
       ## Commit message ##
     -    apply: strip ./ prefix from --directory argument
     +    apply: normalize path in --directory argument
      
          When passing a relative path like --directory=./some/sub, the leading
          "./" caused apply to prepend it literally to patch filenames, resulting
          in an error (invalid path).
     -
     -    Since using "./" is almost memory muscle for many, strip the "./"
     -    prefix so it behaves the same as --directory=some/sub.
     +    There may be more cases like this where users pass some/./path to the
     +    directory which can easily be normalized to an acceptable path, so
     +    these changes try to normalize the path before using it.
      
          Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
      
       ## apply.c ##
      @@ apply.c: static int apply_option_parse_directory(const struct option *opt,
     - 	BUG_ON_OPT_NEG(unset);
       
       	strbuf_reset(&state->root);
     + 	strbuf_addstr(&state->root, arg);
      +
     -+	if (starts_with(arg, "./"))
     -+		arg += 2;
     ++	if (strbuf_normalize_path(&state->root) < 0)
     ++		return error(_("unable to normalize directory: '%s'"), arg);
      +
     - 	strbuf_addstr(&state->root, arg);
       	strbuf_complete(&state->root, '/');
       	return 0;
     + }
      
       ## t/t4128-apply-root.sh ##
      @@ t/t4128-apply-root.sh: test_expect_success 'apply --directory -p (2) ' '
     @@ t/t4128-apply-root.sh: test_expect_success 'apply --directory -p (2) ' '
      +	test_cmp expect actual &&
      +	test_cmp expect some/sub/dir/file
      +'
     ++
     ++test_expect_success 'apply --directory (double slash)' '
     ++	git reset --hard initial &&
     ++	git apply --directory=some//sub -p3 --index patch &&
     ++	echo Bello >expect &&
     ++	git show :some/sub/dir/file >actual &&
     ++	test_cmp expect actual &&
     ++	test_cmp expect some/sub/dir/file
     ++'
     ++
     ++test_expect_success 'apply --directory (./ in the middle)' '
     ++	git reset --hard initial &&
     ++	git apply --directory=some/./sub -p3 --index patch &&
     ++	echo Bello >expect &&
     ++	git show :some/sub/dir/file >actual &&
     ++	test_cmp expect actual &&
     ++	test_cmp expect some/sub/dir/file
     ++'
     ++
     ++test_expect_success 'apply --directory (../ in the middle)' '
     ++	git reset --hard initial &&
     ++	git apply --directory=some/../some/sub -p3 --index patch &&
     ++	echo Bello >expect &&
     ++	git show :some/sub/dir/file >actual &&
     ++	test_cmp expect actual &&
     ++	test_cmp expect some/sub/dir/file
     ++'
     ++
     ++test_expect_success 'apply --directory rejects leading ../' '
     ++	test_must_fail git apply --directory=../foo -p3 patch 2>err &&
     ++	test_grep "unable to normalize directory" err
     ++'
      +
       cat > patch << EOF
       diff --git a/newfile b/newfile


 apply.c               |  4 ++++
 t/t4128-apply-root.sh | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/apply.c b/apply.c
index 3de4aa4d2e..7f0ce5918b 100644
--- a/apply.c
+++ b/apply.c
@@ -5002,6 +5002,10 @@ static int apply_option_parse_directory(const struct option *opt,
 
 	strbuf_reset(&state->root);
 	strbuf_addstr(&state->root, arg);
+
+	if (strbuf_normalize_path(&state->root) < 0)
+		return error(_("unable to normalize directory: '%s'"), arg);
+
 	strbuf_complete(&state->root, '/');
 	return 0;
 }
diff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh
index f6db5a79dd..5eba15fa66 100755
--- a/t/t4128-apply-root.sh
+++ b/t/t4128-apply-root.sh
@@ -43,6 +43,47 @@ test_expect_success 'apply --directory -p (2) ' '
 
 '
 
+test_expect_success 'apply --directory (./ prefix)' '
+	git reset --hard initial &&
+	git apply --directory=./some/sub -p3 --index patch &&
+	echo Bello >expect &&
+	git show :some/sub/dir/file >actual &&
+	test_cmp expect actual &&
+	test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (double slash)' '
+	git reset --hard initial &&
+	git apply --directory=some//sub -p3 --index patch &&
+	echo Bello >expect &&
+	git show :some/sub/dir/file >actual &&
+	test_cmp expect actual &&
+	test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (./ in the middle)' '
+	git reset --hard initial &&
+	git apply --directory=some/./sub -p3 --index patch &&
+	echo Bello >expect &&
+	git show :some/sub/dir/file >actual &&
+	test_cmp expect actual &&
+	test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (../ in the middle)' '
+	git reset --hard initial &&
+	git apply --directory=some/../some/sub -p3 --index patch &&
+	echo Bello >expect &&
+	git show :some/sub/dir/file >actual &&
+	test_cmp expect actual &&
+	test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory rejects leading ../' '
+	test_must_fail git apply --directory=../foo -p3 patch 2>err &&
+	test_grep "unable to normalize directory" err
+'
+
 cat > patch << EOF
 diff --git a/newfile b/newfile
 new file mode 100644

base-commit: 6fcee4785280a08e7f271bd015a4dc33753e2886
-- 
gitgitgadget