On 06/06, Emily Xie wrote:
> An empty string as a pathspec element matches all paths. A buggy
> script, however, could accidentally assign an empty string to a
> variable that then gets passed to a Git command invocation, e.g.:
>
> path=... compute a path to be removed in $path ...
> git rm -r "$path"
>
> which would unintentionally remove all paths in the current
> directory.
>
> The fix for this issue comprises of two steps. Step 1, which warns
> that empty strings as pathspecs will become invalid, has already
> been implemented in commit d426430 ("pathspec: warn on empty strings
> as pathspec", 2016-06-22).
>
> This patch is step 2. It removes the warning and throws an error
> instead.
>
> Signed-off-by: Emily Xie <emilyxxie@xxxxxxxxx>
> Reported-by: David Turner <novalis@xxxxxxxxxxx>
> ---
> pathspec.c | 10 ++++------
> t/t3600-rm.sh | 5 ++---
> t/t3700-add.sh | 5 ++---
> 3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index 50f76ff..65e18b1 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -605,7 +605,7 @@ void parse_pathspec(struct pathspec *pathspec,
> {
> struct pathspec_item *item;
> const char *entry = argv ? *argv : NULL;
> - int i, n, prefixlen, warn_empty_string, nr_exclude = 0;
> + int i, n, prefixlen, nr_exclude = 0;
>
> memset(pathspec, 0, sizeof(*pathspec));
>
> @@ -638,12 +638,10 @@ void parse_pathspec(struct pathspec *pathspec,
> }
>
> n = 0;
> - warn_empty_string = 1;
> while (argv[n]) {
> - if (*argv[n] == '\0' && warn_empty_string) {
> - warning(_("empty strings as pathspecs will be made invalid in upcoming releases. "
> - "please use . instead if you meant to match all paths"));
> - warn_empty_string = 0;
> + if (*argv[n] == '\0') {
> + die("empty string is not a valid pathspec. "
> + "please use . instead if you meant to match all paths");
The spacing (indentation) of the second line here is really weird.
> }
> n++;
> }
> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
> index 5f9913b..c787eac 100755
> --- a/t/t3600-rm.sh
> +++ b/t/t3600-rm.sh
> @@ -858,9 +858,8 @@ test_expect_success 'rm files with two different errors' '
> test_i18ncmp expect actual
> '
>
> -test_expect_success 'rm empty string should invoke warning' '
> - git rm -rf "" 2>output &&
> - test_i18ngrep "warning: empty strings" output
> +test_expect_success 'rm empty string should fail' '
> + test_must_fail git rm -rf ""
> '
>
> test_done
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index f3a4b4a..40a0d2b 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -331,9 +331,8 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
> test_i18ncmp expect.err actual.err
> '
>
> -test_expect_success 'git add empty string should invoke warning' '
> - git add "" 2>output &&
> - test_i18ngrep "warning: empty strings" output
> +test_expect_success 'git add empty string should fail' '
> + test_must_fail git add ""
> '
>
> test_expect_success 'git add --chmod=[+-]x stages correctly' '
> --
> 2.8.4
>
--
Brandon Williams