my_foo and environment not interpreted correctly with mailboxes and ignore and probably other commands

[michael.tatge@…]

set my_mbox_list="+foo +bar"
mailboxes $my_mbox_list

The result seems to interpreted as one argument which is not considered for tokenizing.
I was told the problem lies in mutt_extract_token().
>How-To-Repeat:
set my_mbox_list="+foo +bar" or enviremtn accordingly with existing mailboxes
mailboxes $my_mbox_list
enter mailboxes view
>Fix:
Unknown

[Rocco Rutte <pdmef@…>]

Hi,

* michael.tatge@web.de [06-07-16 15:11:46 +0200] wrote:

>I was told the problem lies in mutt_extract_token().

That was me on #mutt. The problem is: in parse_mailboxes() and all other 
consumers of mutt_extract_token(). When an environment or mutt variable 
is found, it's _complete_ value is returned as just one token.

This means that:

   set my_foo="+bar +foo"
   mailboxes $my_foo

will make mutt_extract_token return '+bar +foo' as the only token. It 
should instead push the results of the expansion back and continue. As 
Michael found out, this is done for backtick expansion already.

In the attached patch, I moved the "push result back" code out to a new 
function I also used to push the result of environment vars back.

Now this seems to work but needs much more testing and hacking to 
resolve infinite loops:

   $ export FOO='$FOO'
   $ ./mutt -e 'mailboxes $FOO'

   bye, Rocco
-- 
:wq!

--zYM0uCDKw75PZbzx
Content-Type: text/x-patch; charset=us-ascii
Content-Disposition: attachment; filename="patch-init0.diff"

diff --git a/buffy.c b/buffy.c
diff --git a/init.c b/init.c
index a0a5d3d..9b64231 100644
--- a/init.c
+++ b/init.c
@@ -129,6 +129,36 @@ int mutt_option_index (char *s)
   return (-1);
 }
 
+static void push_result (BUFFER *dest, BUFFER *tok, BUFFER *result, unsigned char qc)
+{
+  size_t resultlen;
+  char* ptr;
+
+  /* if we got output, make a new string consiting of the ouptput to be
+   * pushed back plus whatever else was left on the original line;
+   * BUT: If this is inside a quoted string, directly add output to the token */
+  if (result->data && qc)
+  {
+    mutt_buffer_addstr (dest, result->data);
+    FREE (&result->data);
+  }
+  else if (result->data)
+  {
+    resultlen = mutt_strlen (result->data);
+    tok->dsize = resultlen + mutt_strlen (tok->dptr) + 1;
+    ptr = safe_malloc (tok->dsize);
+    memcpy (ptr, result->data, resultlen);
+    strcpy (ptr + resultlen, tok->dptr);	/* __STRCPY_CHECKED__ */
+    if (tok->destroy)
+      FREE (&tok->data);
+    tok->data = ptr;
+    tok->dptr = ptr;
+    tok->destroy = 1; /* mark that the caller should destroy this data */
+    ptr = NULL;
+    FREE (&result->data);
+  }
+}
+
 int mutt_extract_token (BUFFER *dest, BUFFER *tok, int flags)
 {
   char		ch;
@@ -220,8 +250,7 @@ int mutt_extract_token (BUFFER *dest, BU
     {
       FILE	*fp;
       pid_t	pid;
-      char	*cmd, *ptr;
-      size_t	expnlen;
+      char	*cmd;
       BUFFER	expn;
       int	line = 0;
 
@@ -256,36 +285,18 @@ int mutt_extract_token (BUFFER *dest, BU
       fclose (fp);
       mutt_wait_filter (pid);
 
-      /* if we got output, make a new string consiting of the shell ouptput
-	 plus whatever else was left on the original line */
-      /* BUT: If this is inside a quoted string, directly add output to 
-       * the token */
-      if (expn.data && qc)
-      {
-	mutt_buffer_addstr (dest, expn.data);
-	FREE (&expn.data);
-      }
-      else if (expn.data)
-      {
-	expnlen = mutt_strlen (expn.data);
-	tok->dsize = expnlen + mutt_strlen (tok->dptr) + 1;
-	ptr = safe_malloc (tok->dsize);
-	memcpy (ptr, expn.data, expnlen);
-	strcpy (ptr + expnlen, tok->dptr);	/* __STRCPY_CHECKED__ */
-	if (tok->destroy)
-	  FREE (&tok->data);
-	tok->data = ptr;
-	tok->dptr = ptr;
-	tok->destroy = 1; /* mark that the caller should destroy this data */
-	ptr = NULL;
-	FREE (&expn.data);
-      }
+      /* result may contain more tokens we need to parse */
+      push_result (dest, tok, &expn, qc);
+
     }
     else if (ch == '$' && (!qc || qc == '"') && (*tok->dptr == '{' || isalpha ((unsigned char) *tok->dptr)))
     {
       const char *env = NULL;
       char *var = NULL;
       int idx;
+      BUFFER expn;
+
+      memset (&expn, 0, sizeof (BUFFER));
 
       if (*tok->dptr == '{')
       {
@@ -306,17 +317,21 @@ int mutt_extract_token (BUFFER *dest, BU
       if (var)
       {
         if ((env = getenv (var)) || (env = myvar_get (var)))
-          mutt_buffer_addstr (dest, env);
+          mutt_buffer_addstr (&expn, env);
         else if ((idx = mutt_option_index (var)) != -1)
         {
           /* expand settable mutt variables */
           char val[LONG_STRING];
 
           if (var_to_string (idx, val, sizeof (val)))
-            mutt_buffer_addstr (dest, val);
+	    mutt_buffer_addstr (&expn, val);
         }
         FREE (&var);
       }
+
+      /* result may contain more tokens we need to parse */
+      push_result (dest, tok, &expn, qc);
+
     }
     else
       mutt_buffer_addch (dest, ch);

--zYM0uCDKw75PZbzx--

[Michael Tatge <michael.tatge@…>]

* On Sun, Jul 16, 2006 I (michael.tatge@web.de) muttered:
> >Number:         2362
> >Synopsis:       my_foo and environment not interpreted correctly with
> >                mailboxes and ignore and probably other commands
> >Description:
> set my_mbox_list="+foo +bar"
> mailboxes $my_mbox_list
> >How-To-Repeat:
> set my_mbox_list="+foo +bar" or environment accordingly with existing mailboxes
> mailboxes $my_mbox_list
> enter mailboxes view

mailboxes `echo $ENVVAR` works as intended. I'd still prefer $my_foo to
be interpreted correctly.

Michael

[Michael Tatge <Michael.Tatge@…>]

* On Sun, Jul 16, 2006 Rocco Rutte wrote:
> In the attached patch, I moved the "push result back" code out to a new 
> function I also used to push the result of environment vars back.

That patch works fine for the problem at hand. It breaks if you don't
quote my_vars that contain spaces if you want them to be considered as
one token. That's consistent with mutt's other quoting needs though and
fine with me.

set my_spell="/usr/bin/aspell -c --mode=email --lang=en_GB"
ispell="$my_ispell"

> Now this seems to work but needs much more testing and hacking to 
> resolve infinite loops:
> 
>   $ export FOO='$FOO'
>   $ ./mutt -e 'mailboxes $FOO'

This is clearly a user error IMO. Should mutt try to detect it? I'm not
sure.

Thanks for the patch, seems to work well.

Michael
-- 
MOUNT TAPE U1439 ON B3, NO RING

PGP-Key-ID: 0xDC1A44DD
Jabber:     init[0]@amessage.de