Common Unix practice is to prefer VISUAL over EDITOR, particularly if
the editor of choice spawns a new window. Thus, it is also common to
see settings like EDITOR='emacs -nw', with the expectation that the
shell will parse this as an argument to 'emacs' and not try to invoke
a file containing a space.
If a user puts junk in EDITOR, they deserve what they get (much more
than virsh will misbehave); furthermore, sudo scrubs EDITOR by
default. So the blind use of metacharacters in EDITOR should not be
considered too much of a security issue.
* tools/virsh.c (editFile): Prefer VISUAL over EDITOR. Don't
reject shell metacharacters in EDITOR.
* tools/virsh.pod (edit, net-edit, ENVIRONMENT): Document VISUAL.
---
tools/virsh.c | 19 ++++++++-----------
tools/virsh.pod | 15 ++++++++++-----
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index aa85ee6..9f3b197 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7381,20 +7381,17 @@ editFile (vshControl *ctl, const char *filename)
char *command;
int command_ret;
- editor = getenv ("EDITOR");
+ editor = getenv ("VISUAL");
+ if (!editor) editor = getenv ("EDITOR");
if (!editor) editor = "vi"; /* could be cruel & default to ed(1) here */
- /* Check the editor doesn't contain shell meta-characters, and if
- * it does, refuse to run.
+ /* Check that filename doesn't contain shell meta-characters, and
+ * if it does, refuse to run. Follow the Unix conventions for
+ * EDITOR: the user can intentionally specify command options, so
+ * we don't protect any shell metacharacters there. Lots more
+ * than virsh will misbehave if EDITOR has bogus contents (which
+ * is why sudo scrubs it by default).
*/
- if (strspn (editor, ACCEPTED_CHARS) != strlen (editor)) {
- vshError(ctl,
- _("%s: $EDITOR environment variable contains shell meta or "
- "other unacceptable characters"),
- editor);
- return -1;
- }
- /* Same for the filename. */
if (strspn (filename, ACCEPTED_CHARS) != strlen (filename)) {
vshError(ctl,
_("%s: temporary filename contains shell meta or other "
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 302de18..3707916 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -332,8 +332,8 @@ This is equivalent to:
virsh define domain.xml
except that it does some error checking.
-The editor used can be supplied by the C<$EDITOR> environment
-variable, or if that is not defined defaults to C<vi>.
+The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
+variables, and defaults to C<vi>.
=item B<migrate> optional I<--live> I<--suspend> I<domain-id> I<desturi> I<migrateuri>
@@ -557,8 +557,8 @@ This is equivalent to:
virsh define network.xml
except that it does some error checking.
-The editor used can be supplied by the C<$EDITOR> environment
-variable, or if that is not defined defaults to C<vi>.
+The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
+variables, and defaults to C<vi>.
=item B<net-list> optional I<--inactive> or I<--all>
@@ -639,10 +639,15 @@ of C<virsh>
The hypervisor to connect to by default. Set this to a URI, in the same
format as accepted by the B<connect> option.
-=item EDITOR
+=item VISUAL
The editor to use by the B<edit> and B<net-edit> options.
+=item EDITOR
+
+The editor to use by the B<edit> and B<net-edit> options, if C<VISUAL>
+is not set.
+
=item LIBVIRT_DEBUG=LEVEL
Turn on verbose debugging of all libvirt API calls. Valid levels are
--
1.6.6.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list