git-web--browse: avoid the use of eval
Using eval causes problems when the URL contains an appropriately
escaped ampersand (\&). Dropping eval from the built-in browser
invocation avoids the problem.
Helped-by: Jeff King <peff@peff.net> (test case)
Signed-off-by: Chris Packham <judge.packham@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/git-web--browse.sh b/git-web--browse.sh
index e9de241..1e82726 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -156,7 +156,7 @@
;;
google-chrome|chrome|chromium|chromium-browser)
# No need to specify newTab. It's default in chromium
- eval "$browser_path" "$@" &
+ "$browser_path" "$@" &
;;
konqueror)
case "$(basename "$browser_path")" in
@@ -164,10 +164,10 @@
# It's simpler to use kfmclient to open a new tab in konqueror.
browser_path="$(echo "$browser_path" | sed -e 's/konqueror$/kfmclient/')"
type "$browser_path" > /dev/null 2>&1 || die "No '$browser_path' found."
- eval "$browser_path" newTab "$@"
+ "$browser_path" newTab "$@" &
;;
kfmclient)
- eval "$browser_path" newTab "$@"
+ "$browser_path" newTab "$@" &
;;
*)
"$browser_path" "$@" &
@@ -175,7 +175,7 @@
esac
;;
w3m|elinks|links|lynx|open)
- eval "$browser_path" "$@"
+ "$browser_path" "$@"
;;
start)
exec "$browser_path" '"web-browse"' "$@"
@@ -185,7 +185,7 @@
;;
*)
if test -n "$browser_cmd"; then
- ( eval $browser_cmd "$@" )
+ ( eval "$browser_cmd \"\$@\"" )
fi
;;
esac
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
new file mode 100755
index 0000000..7906e5d
--- /dev/null
+++ b/t/t9901-git-web--browse.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+#
+
+test_description='git web--browse basic tests
+
+This test checks that git web--browse can handle various valid URLs.'
+
+. ./test-lib.sh
+
+test_expect_success \
+ 'URL with an ampersand in it' '
+ echo http://example.com/foo\&bar >expect &&
+ git config browser.custom.cmd echo &&
+ git web--browse --browser=custom \
+ http://example.com/foo\&bar >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'URL with a semi-colon in it' '
+ echo http://example.com/foo\;bar >expect &&
+ git config browser.custom.cmd echo &&
+ git web--browse --browser=custom \
+ http://example.com/foo\;bar >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'URL with a hash in it' '
+ echo http://example.com/foo#bar >expect &&
+ git config browser.custom.cmd echo &&
+ git web--browse --browser=custom \
+ http://example.com/foo#bar >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'browser paths are properly quoted' '
+ echo fake: http://example.com/foo >expect &&
+ cat >"fake browser" <<-\EOF &&
+ #!/bin/sh
+ echo fake: "$@"
+ EOF
+ chmod +x "fake browser" &&
+ git config browser.w3m.path "`pwd`/fake browser" &&
+ git web--browse --browser=w3m \
+ http://example.com/foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'browser command allows arbitrary shell code' '
+ echo "arg: http://example.com/foo" >expect &&
+ git config browser.custom.cmd "
+ f() {
+ for i in \"\$@\"; do
+ echo arg: \$i
+ done
+ }
+ f" &&
+ git web--browse --browser=custom \
+ http://example.com/foo >actual &&
+ test_cmp expect actual
+'
+
+test_done
