[PATCH v2] apply: normalize path in --directory argument
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
argumentPublished-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(+)
Show changes to 2 files +45 −0
apply.c, t/t4128-apply-root.sh
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