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
Description: PGP signature
