‘eval "$@"’ created an extra layer of shell interpretation, which was
probably not expected by a user who passed multiple arguments to git
submodule foreach:
$ git grep "'"
[searches for single quotes]
$ git submodule foreach git grep "'"
Entering '[submodule]'
/usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted
string
Stopping at '[submodule]'; script returned non-zero status.
To fix this, if the user passed more than one argument, just execute
"$@" directly instead of passing it to eval.
Signed-off-by: Anders Kaseorg
---
On Fri, 27 Sep 2013, Johan Herland wrote:
> 1. Please add the use case you mention above as a new test case, so
> that we can easily catch future regressions.
Test added.
> 2. If we are unlucky there might be existing users that work around the
> existing behavior by adding an extra level of quoting (i.e. doing the
> equivalent of git submodule foreach git grep "\'" in your example
> above). Will their workaround break as a result of your change? Is that
> acceptable?
Anyone adding an extra level of quoting ought to realize that they should
be passing a single argument to submodule foreach, so that the reason for
the extra quoting is clear:
git submodule foreach "git grep \'"
will not break. If someone is actually doing
git submodule foreach git grep "\'"
then this will change in behavior. I think this change is important.
(One could even imagine someone feeding untrusted input to
git submodule foreach git grep "$variable"
which, without my patch, results in a nonobvious shell code injection
vulnerability.)
I considered an alternative fix where the first argument is always
shell-evaulated and any others are not (i.e. cmd=$1 && shift && eval
"$cmd \"\$@\""), which is potentially more useful in case the command
needs to use $path. But that may be too confusing, and this way has some
precedent (e.g. perl’s system()).
Anders
git-submodule.sh | 7 ++-
t/t7407-submodule-foreach.sh | 9 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index c17bef1..3381864 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -545,7 +545,12 @@ cmd_foreach()
sm_path=$(relative_path "$sm_path") &&
# we make $path available to scripts ...
path=$sm_path &&
- eval "$@" &&
+ if [ $# -eq 1 ]
+ then
+ eval "$1"
+ else
+ "$@"
+ fi &&
if test -n "$recursive"
then
cmd_foreach "--recursive" "$@"
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index be93f10..6b2fd39 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -329,4 +329,13 @@ test_expect_success 'command passed to foreach --recursive
retains notion of std
test_cmp expected actual
'
+test_expect_success 'multi-argument command passed to foreach is not
shell-evaluated twice' '
+ (
+ cd super &&
+ git submodule foreach "echo \\\"quoted\\\"" > ../expected &&
+ git submodule foreach echo \"quoted\" > ../actual
+ ) &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
