Re: [PATCH] Add filename filters when saving attachments.

Alejandro Colomar via Mutt-dev <[email protected]> Sat, 9 May 2026 14:02:54 +0200
Newsgroups	gmane.mail.mutt.devel
Message-ID	<af8guf6an9ZhO6vM@devuan>
Hi Kevin,

On 2026-05-09T18:59:22+0800, Kevin J. McCarthy wrote:
> In send mode (the compose menu), simply take the basename, since it's
> attached by the user already.
> 
> In receive mode, try to keep the whole filename, even if it orginally
> contained path characters.  Filter "/" and non-printables into an
> underscore "_".
> ---
> 
> This is an early v2 patch, sent out to solicit feedback.
> 
> The attachment saving code is a bit of a maze, so there isn't a need to
> understand it all.
> 
> Suffice to say I've swapped out the various places in it that directly
> reference the attachment name to use a version that has been filtered by
> safe_attachment_filename() just below.
> 
> Right now, in receive mode, this filters "/" and unprintables into "_".
> I could use a stricter sanitizer function, but I think that would be too
> strict and irritate users.
> 
> I could also simply filter only "/" -> "_", which might be all we need
> to do.  Comments?

I think I'd do the simplest change.  And if we want to do more, maybe
split that into its own commit, with a different justification in the
commit message.

> 
>  muttlib.c    | 11 ++++++++---
>  recvattach.c | 51 ++++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/muttlib.c b/muttlib.c
> index 6eabdec2..858a4bcf 100644
> --- a/muttlib.c
> +++ b/muttlib.c
> @@ -1277,8 +1277,13 @@ void mutt_expand_fmt(BUFFER *dest, const char *fmt, const char *src)
>    }
>  }
>  
> -/* return 0 on success, -1 on abort, 1 on error */
> -int mutt_check_overwrite(const char *attname, const char *path,
> +/* return 0 on success, -1 on abort, 1 on error
> + *
> + * safe_attname should have been filtered through safe_attachment_filename()
> + * in recvattach.c to either get the basename in sendmode,
> + * or to filter '/' and unsafe characters in recvmode.
> + * */
> +int mutt_check_overwrite(const char *safe_attname, const char *path,
>                           BUFFER *fname, int *append, char **directory)
>  {
>    int rc = 0;
> @@ -1321,7 +1326,7 @@ int mutt_check_overwrite(const char *attname, const char *path,
>        return (rc == MUTT_NO) ? 1 : -1;
>  
>      tmp = mutt_buffer_pool_get();
> -    mutt_buffer_strcpy(tmp, mutt_basename(NONULL(attname)));
> +    mutt_buffer_strcpy(tmp, safe_attname);
>      if ((mutt_buffer_get_field(_("File under directory: "), tmp,
>                                 MUTT_FILE | MUTT_CLEAR) != 0) ||
>          !mutt_buffer_len(tmp))
> diff --git a/recvattach.c b/recvattach.c
> index 3b172e4c..f64a0008 100644
> --- a/recvattach.c
> +++ b/recvattach.c
> @@ -518,23 +518,49 @@ static int save_attachment_helper(FILE *fp, BODY *m, const char *path,
>    return rc;
>  }
>  
> +static void safe_attachment_filename(BUFFER *safe_filename, const char *filename,
> +                                     int sendmode)
> +{
> +  if (sendmode)
> +    mutt_buffer_strcpy(safe_filename, mutt_basename(filename));
> +  else
> +  {
> +    /* Alternatively, we could use
> +     *   mutt_buffer_sanitize_filename(safe_filename, filename, MUTT_SANITIZE_ALLOW_8BIT);
> +     * Comments?
> +     */
> +    mutt_buffer_clear(safe_filename);
> +
> +    for (; *filename; filename++)
> +    {
> +      if (*filename == '/' || !IsPrint((unsigned char) *filename))

AFAICS, IsPrint() already does the necessary cast.

	alx@devuan:~/src/mutt/mutt/master$ grepc -B1 -A3 -n IsPrint .
	./protos.h-452-#ifdef LOCALES_HACK
	./protos.h:453:#define IsPrint(c) (isprint((unsigned char)(c)) ||      \
			    ((unsigned char)(c) >= 0xa0))
	./protos.h-455-#define IsWPrint(wc) (iswprint(wc) || wc >= 0xa0)
	./protos.h-456-#else
	./protos.h:457:#define IsPrint(c) (isprint((unsigned char)(c)) ||      \
			    (option(OPTLOCALES) ? 0 :           \
			     ((unsigned char)(c) >= 0xa0)))
	./protos.h-460-#define IsWPrint(wc) (iswprint(wc) ||                           \
	./protos.h-461-                      (option(OPTLOCALES) ? 0 : (wc >= 0xa0)))
	./protos.h-462-#endif

> +        mutt_buffer_addch(safe_filename, '_');
> +      else
> +        mutt_buffer_addch(safe_filename, *filename);
> +    }
> +  }
> +}


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>
signature.asc (application/pgp-signature, 833 B)