From: Jeff King <peff@peff.net>
To: Chris Packham <judge.packham@gmail.com>
Cc: Christian Couder <chriscool@tuxfamily.org>,
Jeff Epler <jepler@unpythonic.net>,
git@vger.kernel.org
Subject: Re: git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk)
Date: Sat, 17 Sep 2011 23:29:34 -0400 [thread overview]
Message-ID: <20110918032933.GA17977@sigill.intra.peff.net> (raw)
In-Reply-To: <4E753C04.1070202@gmail.com>
On Sun, Sep 18, 2011 at 12:32:04PM +1200, Chris Packham wrote:
> Update: it's the call to eval that causes the problem
>
> eval kfmclient newTab https://internalhost/code\&stuff/bugs.php?id=foo
> [1] 14728
> bash: stuff/bugs.php?id=foo: No such file or directory
Hmm. The offending lines look like:
eval "$browser_path" "$@" &
Normally in git we treat user-configured commands as shell snippets,
meaning the user is responsible for any quoting. But in this script, we
seem to run:
type "$browser_path"
several times. Which implies that "$browser_path" must be the actual
executable. In which case, I would think that:
"$browser_path" "$@" &
would be the right thing. And indeed, that is what the firefox arm of
the case statement does. But chrome, konqueror, and others use eval.
Unrelated, but it also looks like $browser_path is used unquoted in the
firefox case (see inside the vers=$(...)).
-Peff
next prev parent reply other threads:[~2011-09-18 3:29 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-17 2:29 [RFC/PATCH] Configurable hyperlinking in gitk Jeff Epler
2011-09-17 9:26 ` Chris Packham
2011-09-17 10:01 ` Chris Packham
2011-09-17 13:45 ` Jeff Epler
2011-09-17 23:33 ` Chris Packham
2011-09-18 0:30 ` git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk) Chris Packham
2011-09-18 0:32 ` Chris Packham
2011-09-18 3:29 ` Jeff King [this message]
2011-09-18 10:20 ` [PATCH] git-web--browse: invoke kfmclient directly Chris Packham
2011-09-18 18:38 ` Jeff King
2011-09-19 9:26 ` [RFC/PATCHv2] git-web--browse: avoid the use of eval Chris Packham
2011-09-19 18:34 ` Jeff King
2011-09-20 9:04 ` Chris Packham
2011-09-20 18:49 ` Jeff King
2011-09-20 19:35 ` Junio C Hamano
2011-09-19 17:57 ` [PATCH] git-web--browse: invoke kfmclient directly Junio C Hamano
2011-09-19 18:20 ` Jeff King
2011-09-19 20:42 ` Junio C Hamano
2011-09-19 20:44 ` Jeff King
2011-09-19 21:22 ` Junio C Hamano
2011-09-19 21:46 ` Andreas Schwab
2011-09-19 22:23 ` Jeff King
2011-09-19 22:28 ` Junio C Hamano
2011-09-19 20:44 ` Andreas Schwab
2011-09-19 21:32 ` Jakub Narebski
2011-09-18 14:46 ` git web--browse error handling URL with & in it (Was Re: [RFC/PATCH] Configurable hyperlinking in gitk) Christian Couder
2011-09-19 15:05 ` [RFC/PATCH] Configurable hyperlinking in gitk Marc Branchaud
2011-09-18 18:50 ` Jakub Narebski
2011-09-22 1:31 ` Jeff Epler
2011-09-22 2:15 ` [PATCH v3] " Jeff Epler
2011-10-11 18:37 ` [RESEND PATCH " Jeff Epler
2011-10-11 22:13 ` Junio C Hamano
2011-10-12 9:07 ` Chris Packham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110918032933.GA17977@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=jepler@unpythonic.net \
--cc=judge.packham@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).