#550843 ldapvi: does neither handle $EDITOR nor $PAGER values with arguments

Package:
ldapvi
Source:
ldapvi
Description:
perform an LDAP search and update results using a text editor
Submitter:
Date:
2013-08-03 02:45:07 UTC
Severity:
wishlist
#550843#5
Date:
2009-10-13 11:46:03 UTC
From:
To:
I have

  EDITOR="emacsclient --alternate-editor emacs"

which works with other software but confuses ldapvi:

~$ ldapvi
     26 entries read
error (misc.c line 180): No such file or directory
editor died
error (ldapvi.c line 83): No such file or directory

Also the error messages are uninformative, it would help to print the
file name that was not found, for example.

#550843#12
Date:
2009-10-13 19:31:37 UTC
From:
To:
severity 550843 wishlist
thanks

	Hi!

* Marcus Better <marcus@better.se> [2009-10-13 13:46:03 CEST]:

 The question rather would be: How would you specify a pathname/filename
that has spaces in it? If it would get switched to your approach it
would break for those that have spaces in their editor paths. Also
following that approach might raise security related issues with
injecting escape sequences, though I'm not totally sure wether that
would be an issue because ldapvi is run under the user's own permissions
and it would need to tweak the users' environment.

 Still, I'm not totally convinced that this is a change that really
should be done, at most it's a wishlist entry and not a bug.

 Thanks. :)
Rhonda

#550843#17
Date:
2009-10-14 07:45:30 UTC
From:
To:
Gerfried Fuchs wrote:

By escaping like the shell perhaps?

Are you seriously saying that hg, svn and git have security issues
because of this? Because they work fine with my setting.

In fact this EDITOR setting was taken from the Emacs manual, you can
find it here under the "-a" option:

http://www.gnu.org/software/emacs/manual/html_node/emacs/emacsclient-Options.html

Cheers,

Marcus

#550843#22
Date:
2009-10-14 09:02:33 UTC
From:
To:
Hi,

Quoting Marcus Better (marcus@better.se):

That is what svn documents in the svnbook, yes.

I wouldn't see it as a security issue, but it's certainly something that
needs to be documented well.  (If it's shell, then which shell is it?
Does it have to be the user's shell?)

A quick google survey turns up some IBM tools which explicitly say that
EDITOR must be a full path for them, so presumably they even call
execv(3) rather than execvp(3), much less system(3).

But if prevailing implementation practice among modern tools is to go
through the shell, then I'm sympathetic to that request for the sake of
compatibility.


Do you have a patch for this?


d.

#550843#27
Date:
2009-10-14 09:29:34 UTC
From:
To:
* Marcus Better <marcus@better.se> [2009-10-14 09:45:30 CEST]:

 That would break the current way of allowing that and would require
complex parsing of the string.

 Erm, about git (svn):
<http://osdir.com/ml/git/2009-02/msg02581.html>, it doesn't work - and
there are also some interesting responses to it.

 Anyway, yes, I'm seriously saying that it might be troublesome and
non-trivial to do it right.

 Emacs is its own OS so that doesn't really convince me. ;)  Besides, it
isn't really my call, and if you can come up with a clean patch for it I
guess David might consider accepting it.

 So long!
Rhonda

#550843#32
Date:
2010-12-14 20:44:53 UTC
From:
To:
tags 550843 +patch
thanks

Here’s a patch to fix this by launching the editor command via /bin/sh.  I
also sent this upstream:
http://lists.askja.de/pipermail/ldapvi/2010-December/000084.html

#550843#39
Date:
2010-12-14 22:23:37 UTC
From:
To:
Here’s a better patch that also fixes a crash on 64-bit systems (due to
execl(…, 0) instead of execl(…, (char *) NULL)).  Also sent upstream:

http://lists.askja.de/pipermail/ldapvi/2010-December/000085.html
http://lists.askja.de/pipermail/ldapvi/2010-December/000086.html

Anders

#550843#44
Date:
2012-01-20 18:26:35 UTC
From:
To:
retitle 550843 ldapvi: does neither handle $EDITOR nor $PAGER values with arguments
kthxbye

Hi,

I ran into this issue today, too, with $PAGER set to "less -s" like in
this thread on the upstream ML:
http://lists.askja.de/pipermail/ldapvi/2011-May/000092.html

Rhonda wrote on Wed, 14 Oct 2009 11:29:34 +0200:

But that seems only for git-svn and not git itself. With git it works
fine since ages (I use "less -XF" as pager in git, works fine even on
git 1.5.x on Debian Lenny), same for mutt with $EDITOR where I use
"emacsclient -a emacs" or even more complex constructs. See
http://bugs.debian.org/656657 for a more complete discussion of this
topic. (I actually wrote that bug report during research for details
for this mail. :-)

The only interesting ones are IMHO those suggesting to pass the whole
string as argument to "sh -c":

http://osdir.com/ml/git/2009-02/msg02582.html
http://osdir.com/ml/git/2009-02/msg02589.html

(The link on http://osdir.com/ml/git/2009-02/msg02678.html is broken
due to some msg-id-looks-like-email obfuscating techniques, so there
maybe more advise, but the link itself doesn't help.)

IMHO the current state ignores the rule of "least surprise" (and the
issued error messages make that even worse).

Commands with whitespace in the path are extremly seldom and setting
commands with parameters via environment variables and passing them to
"sh -c" is very common.

It is also defined for at least $PAGER in the POSIX specification of
mailx[1] and man[2].

  [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mailx.html
  [2] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/man.html

In both is declared:

  PAGER

    Determine a string representing an output filtering or pagination
    command for writing the output to the terminal. Any string
    acceptable as a command_string operand to the sh -c command shall
    be valid.

So at least for $PAGER some parts of POSIX expect the value of the
variable to be handled like shell code.

I though found no such declaration for $EDITOR or $VISUAL in that
document, though. Nevertheless I'd expect those variables to be
handled identically.

Even though David hasn't yet decided (or at least not yet responded)
on the proposed patch(es), I would include them in the Debian package
for the sake of the least surprise, working pager and editor options,
less confusing error messages and a non-crashing ldapvi in case you
try to use a pager with options twice in a row.

		Regards, Axel

#550843#51
Date:
2013-08-03 02:43:15 UTC
From:
To:
tags 550843 + fixed-upstream
thanks

This is fixed in these upstream commits:
33d2368 Cast the trailing NULL on execl() calls
048068e Handle editor commands with arguments
f1d42ba Handle pager commands with arguments

Since the author seems to be too busy to do a release, can these be
cherry-picked into the Debian package?

Thanks,
Anders