On Sat, May 09, 2026 at 06:59:22PM +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?
Maybe I missed some previous discussion, but when saving an attachment
we prompt the user for the name and display a suggestions, right?
Besides unprintable characters, which can obviously not be shown in the
prompt, why do we need to filter anything?
And what ever the answer to my question above is should probably be part
of 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
I think having a flag on a function to let it do two things is really
ugly. But if you think that this is the cleanest solution, go with it.
> + {
> + /* Alternatively, we could use
> + * mutt_buffer_sanitize_filename(safe_filename, filename,
> MUTT_SANITIZE_ALLOW_8BIT);
> + * Comments?
I that _8BIT enough to represent all the languages that people around
the world use? If not, then this is no alternative. If yes, we should
not reinvent the wheel here.
> + */
> + mutt_buffer_clear(safe_filename);
> +
> + for (; *filename; filename++)
> + {
> + if (*filename == '/' || !IsPrint((unsigned char) *filename))
> + mutt_buffer_addch(safe_filename, '_');
> + else
> + mutt_buffer_addch(safe_filename, *filename);
> + }
> + }
> +}
> +
> static int mutt_query_save_attachment(FILE *fp, BODY *body, HEADER *hdr,
> char **directory)
> {
> char *prompt;
> - BUFFER *buf = NULL, *tfile = NULL;
> + BUFFER *buf = NULL, *tfile = NULL, *safe_filename = NULL;
> int is_message;
> int append = 0;
> int rc = -1;
>
> buf = mutt_buffer_pool_get();
> tfile = mutt_buffer_pool_get();
> + safe_filename = mutt_buffer_pool_get();
> +
> + safe_attachment_filename(safe_filename, NONULL(body->filename), (fp ==
> NULL));
>
> if (body->filename)
> {
> if (directory && *directory)
> - mutt_buffer_concat_path(buf, *directory,
> mutt_basename(body->filename));
> + mutt_buffer_concat_path(buf, *directory, mutt_b2s(safe_filename));
> else
> - mutt_buffer_strcpy(buf, body->filename);
> + mutt_buffer_strcpy(buf, mutt_b2s(safe_filename));
> }
> else if (body->hdr &&
> body->encoding != ENCBASE64 &&
> @@ -579,7 +605,7 @@ static int mutt_query_save_attachment(FILE *fp, BODY
> *body, HEADER *hdr, char **
> }
> else
> {
> - if ((rc = mutt_check_overwrite(body->filename, mutt_b2s(buf),
> + if ((rc = mutt_check_overwrite(mutt_b2s(safe_filename), mutt_b2s(buf),
> tfile, &append, directory)) == -1)
> goto cleanup;
> else if (rc == 1)
> @@ -613,19 +639,24 @@ static int mutt_query_save_attachment(FILE *fp, BODY
> *body, HEADER *hdr, char **
> cleanup:
> mutt_buffer_pool_release(&buf);
> mutt_buffer_pool_release(&tfile);
> + mutt_buffer_pool_release(&safe_filename);
> return rc;
> }
>
> void mutt_save_attachment_list(ATTACH_CONTEXT *actx, FILE *fp, int tag, BODY
> *top, HEADER *hdr, MUTTMENU *menu)
> {
> - BUFFER *buf = NULL, *tfile = NULL, *orig_cwd = NULL;
> + BUFFER *buf = NULL, *tfile = NULL, *orig_cwd = NULL, *safe_filename = NULL;
> char *directory = NULL;
> int i, rc = 1, restore_cwd = 0;
> int last = menu ? menu->current : -1;
> FILE *fpout;
>
> - buf = mutt_buffer_pool_get();
> - tfile = mutt_buffer_pool_get();
> + if (!option(OPTATTACHSPLIT))
> + {
> + buf = mutt_buffer_pool_get();
> + tfile = mutt_buffer_pool_get();
> + safe_filename = mutt_buffer_pool_get();
> + }
>
> if (AttachSaveDir)
> {
> @@ -686,7 +717,8 @@ void mutt_save_attachment_list(ATTACH_CONTEXT *actx, FILE
> *fp, int tag, BODY *to
> {
> append = 0;
>
> - mutt_buffer_strcpy(buf, mutt_basename(NONULL(top->filename)));
> + safe_attachment_filename(safe_filename, NONULL(top->filename), (fp
> == NULL));
> + mutt_buffer_strcpy(buf, mutt_b2s(safe_filename));
> prepend_curdir(buf);
>
> if ((mutt_buffer_get_field(_("Save to file: "), buf,
> @@ -694,7 +726,7 @@ void mutt_save_attachment_list(ATTACH_CONTEXT *actx, FILE
> *fp, int tag, BODY *to
> !mutt_buffer_len(buf))
> goto cleanup;
> mutt_buffer_expand_path(buf);
> - if (mutt_check_overwrite(top->filename, mutt_b2s(buf), tfile,
> + if (mutt_check_overwrite(mutt_b2s(safe_filename), mutt_b2s(buf),
> tfile,
> &append, NULL))
> goto cleanup;
> }
> @@ -750,6 +782,7 @@ cleanup:
> mutt_buffer_pool_release(&buf);
> mutt_buffer_pool_release(&tfile);
> mutt_buffer_pool_release(&orig_cwd);
> + mutt_buffer_pool_release(&safe_filename);
> }
>
> static void
> --
> 2.54.0
>
