diff options
| author | Chris Packham <judge.packham@gmail.com> | 2011-10-02 13:44:17 +1300 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2011-10-03 10:47:07 -0700 |
| commit | 480f062cecdd4f662b1d2ed5d497b6f458974b22 (patch) | |
| tree | 20aa2d56e3dd3be14b32be8d64238bd05ed18d21 | |
| parent | f696543dad6c7ba27b0c4fab167a5687263a9ba0 (diff) | |
| download | git-cp/git-web-browse-browsers.tar.gz | |
git-web--browse: avoid the use of evalcp/git-web-browse-browsers
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>
| -rwxr-xr-x | git-web--browse.sh | 10 | ||||
| -rwxr-xr-x | t/t9901-git-web--browse.sh | 66 |
2 files changed, 71 insertions, 5 deletions
diff --git a/git-web--browse.sh b/git-web--browse.sh index e9de241dd0..1e827264b4 100755 --- a/git-web--browse.sh +++ b/git-web--browse.sh @@ -156,7 +156,7 @@ firefox|iceweasel|seamonkey|iceape) ;; 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 @@ konqueror) # 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 @@ konqueror) esac ;; w3m|elinks|links|lynx|open) - eval "$browser_path" "$@" + "$browser_path" "$@" ;; start) exec "$browser_path" '"web-browse"' "$@" @@ -185,7 +185,7 @@ opera|dillo) ;; *) 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 0000000000..7906e5d032 --- /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 |