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>
Chris Packham committed
Oct 2, 2011 at 13:44 UTC
480f062cecdd4f662b1d2ed5d497b6f458974b22
2 files changed
+71
-5
git-web--browse.sh
+5
-5
@@ -156,7 +156,7 @@ firefox|iceweasel|seamonkey|iceape)
156
;;
157
google-chrome|chrome|chromium|chromium-browser)
158
# No need to specify newTab. It's default in chromium
159
- eval "$browser_path" "$@" &
159
+ "$browser_path" "$@" &
160
;;
161
konqueror)
162
case "$(basename "$browser_path")" in
@@ -164,10 +164,10 @@ konqueror)
164
# It's simpler to use kfmclient to open a new tab in konqueror.
165
browser_path="$(echo "$browser_path" | sed -e 's/konqueror$/kfmclient/')"
166
type "$browser_path" > /dev/null 2>&1 || die "No '$browser_path' found."
167
- eval "$browser_path" newTab "$@"
167
+ "$browser_path" newTab "$@" &
168
;;
169
kfmclient)
170
- eval "$browser_path" newTab "$@"
170
+ "$browser_path" newTab "$@" &
171
;;
172
*)
173
"$browser_path" "$@" &
@@ -175,7 +175,7 @@ konqueror)
175
esac
176
;;
177
w3m|elinks|links|lynx|open)
178
- eval "$browser_path" "$@"
178
+ "$browser_path" "$@"
179
;;
180
start)
181
exec "$browser_path" '"web-browse"' "$@"
@@ -185,7 +185,7 @@ opera|dillo)
185
;;
186
*)
187
if test -n "$browser_cmd"; then
188
- ( eval $browser_cmd "$@" )
188
+ ( eval "$browser_cmd \"\$@\"" )
189
fi
190
;;
191
esac
t/t9901-git-web--browse.sh
new
+66
@@ -0,0 +1,66 @@
1
+#!/bin/sh
2
+#
3
+
4
+test_description='git web--browse basic tests
5
+
6
+This test checks that git web--browse can handle various valid URLs.'
7
+
8
+. ./test-lib.sh
9
+
10
+test_expect_success \
11
+ 'URL with an ampersand in it' '
12
+ echo http://example.com/foo\&bar >expect &&
13
+ git config browser.custom.cmd echo &&
14
+ git web--browse --browser=custom \
15
+ http://example.com/foo\&bar >actual &&
16
+ test_cmp expect actual
17
+'
18
+
19
+test_expect_success \
20
+ 'URL with a semi-colon in it' '
21
+ echo http://example.com/foo\;bar >expect &&
22
+ git config browser.custom.cmd echo &&
23
+ git web--browse --browser=custom \
24
+ http://example.com/foo\;bar >actual &&
25
+ test_cmp expect actual
26
+'
27
+
28
+test_expect_success \
29
+ 'URL with a hash in it' '
30
+ echo http://example.com/foo#bar >expect &&
31
+ git config browser.custom.cmd echo &&
32
+ git web--browse --browser=custom \
33
+ http://example.com/foo#bar >actual &&
34
+ test_cmp expect actual
35
+'
36
+
37
+test_expect_success \
38
+ 'browser paths are properly quoted' '
39
+ echo fake: http://example.com/foo >expect &&
40
+ cat >"fake browser" <<-\EOF &&
41
+ #!/bin/sh
42
+ echo fake: "$@"
43
+ EOF
44
+ chmod +x "fake browser" &&
45
+ git config browser.w3m.path "`pwd`/fake browser" &&
46
+ git web--browse --browser=w3m \
47
+ http://example.com/foo >actual &&
48
+ test_cmp expect actual
49
+'
50
+
51
+test_expect_success \
52
+ 'browser command allows arbitrary shell code' '
53
+ echo "arg: http://example.com/foo" >expect &&
54
+ git config browser.custom.cmd "
55
+ f() {
56
+ for i in \"\$@\"; do
57
+ echo arg: \$i
58
+ done
59
+ }
60
+ f" &&
61
+ git web--browse --browser=custom \
62
+ http://example.com/foo >actual &&
63
+ test_cmp expect actual
64
+'
65
+
66
+test_done