[RFC/PATCHv2] git-web--browse: avoid the use of eval
flat view
From: Chris Packham <hidden>
Date: 2016-06-15 22:52:04
Subsystem:
the rest · Maintainer:
Linus Torvalds
Using eval causes problems when the URL contains an appropriately escaped ampersand (\&). Dropping eval from the built-in browser invocation avoids the problem. Cc: peff@peff.net Cc: chriscool@tuxfamily.org Cc: jepler@unpythonic.net Signed-off-by: Chris Packham <redacted> --- Here's an updated patch which drops the uses of eval when invoking a supported browser. The default case still uses eval but adds some extra quoting which also fixes the problem. I've avoided touching the 'start' case because I don't have access to a windows system to test with. I've replaced my tests With the test suggested by Peff (should I be giving him credit in the copyright line or something?). I've grabbed t9901 but if there is a better set of miscellaneous minor tests that I should be using let me know. git-web--browse.sh | 10 +++++----- t/t9901-git-web--browse.sh | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) create mode 100755 t/t9901-git-web--browse.sh
diff --git a/git-web--browse.sh b/git-web--browse.sh
index e9de241..ee05f10 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 0000000..141ed17
--- /dev/null
+++ b/t/t9901-git-web--browse.sh@@ -0,0 +1,21 @@ +#!/bin/sh +# +# Copyright (c) 2011 Chris Packham +# + +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 \ + 'accepts a 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_done
--
1.7.7.rc1.4.g47f23.dirty