[neomutt-devel] [PATCH 2/2] envelope: manage subject/real_subj together, ensuring real_subj is valid

наб nabijaczleweli at nabijaczleweli.xyz
Thu Dec 21 23:47:05 CET 2023


As it stands, subject is assigned at will throughout every interested
piece of code, and real_subj is parsed with $reply_regex and set in
three unaffiliated places.

This means that, for example, IMAP mailboxes simply do not have
$reply_regex processing. Unless you change it, then they refresh
to have it. Until you reopen and then it's lost.

Mark both fields as const and introduce a mutt_env_set_subject()
setter that sets subject and parses real_subj. This interior
mutability is bypassed only in hcache serialisation, by necessity.
rfc2047 (un)parsing needs to abuse it slightly to match the calling
convention.

This ensures that real_subj always matches subject, and so $reply_regex
works as documented instead of not at all.

Fixes: https://mailman.neomutt.org/pipermail/neomutt-devel-neomutt.org/2023-December/000897.html
---
 email/envelope.c     | 35 ++++++++++++++++++++++++++++++-----
 email/envelope.h     |  5 +++--
 email/parse.c        | 19 +------------------
 email/rfc2047.c      | 14 ++++++++++++--
 envelope/functions.c |  2 +-
 hcache/serialize.c   |  6 +++---
 index/index.c        | 13 +------------
 main.c               |  4 ++--
 ncrypt/crypt.c       |  2 +-
 pager/message.c      | 14 +-------------
 postpone/postpone.c  |  2 +-
 send/send.c          | 14 ++++++++------
 12 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/email/envelope.c b/email/envelope.c
index dcd42048..28110be9 100644
--- a/email/envelope.c
+++ b/email/envelope.c
@@ -33,6 +33,8 @@
 #include <string.h>
 #include "mutt/lib.h"
 #include "address/lib.h"
+#include "config/helpers.h"
+#include "core/neomutt.h"
 #include "envelope.h"
 #include "email.h"
 
@@ -58,6 +60,29 @@ struct Envelope *mutt_env_new(void)
   return env;
 }
 
+/**
+ * mutt_env_set_subject - Set both subject and real_subj to @subj
+ */
+void mutt_env_set_subject(struct Envelope *env, const char *subj)
+{
+  mutt_str_replace((char **)&env->subject, subj);
+  *(char **)&env->real_subj = NULL;
+
+  if (env->subject) {
+      regmatch_t match;
+      const struct Regex *c_reply_regex = cs_subset_regex(NeoMutt->sub, "reply_regex");
+      if (mutt_regex_capture(c_reply_regex, env->subject, 1, &match))
+      {
+        if (env->subject[match.rm_eo] != '\0')
+          *(char **)&env->real_subj = env->subject + match.rm_eo;
+      }
+      else
+      {
+        *(char **)&env->real_subj = env->subject;
+      }
+  }
+}
+
 #ifdef USE_AUTOCRYPT
 /**
  * mutt_autocrypthdr_new - Create a new AutocryptHeader
@@ -114,7 +139,7 @@ void mutt_env_free(struct Envelope **ptr)
   FREE(&env->list_post);
   FREE(&env->list_subscribe);
   FREE(&env->list_unsubscribe);
-  FREE(&env->subject);
+  FREE((char **)&env->subject);
   /* real_subj is just an offset to subject and shouldn't be freed */
   FREE(&env->disp_subj);
   FREE(&env->message_id);
@@ -226,11 +251,11 @@ void mutt_env_merge(struct Envelope *base, struct Envelope **extra)
   /* real_subj is subordinate to subject */
   if (!base->subject)
   {
-    base->subject = (*extra)->subject;
-    base->real_subj = (*extra)->real_subj;
+    *(char **)&base->subject = (*extra)->subject;
+    *(char **)&base->real_subj = (*extra)->real_subj;
     base->disp_subj = (*extra)->disp_subj;
-    (*extra)->subject = NULL;
-    (*extra)->real_subj = NULL;
+    *(char **)&(*extra)->subject = NULL;
+    *(char **)&(*extra)->real_subj = NULL;
     (*extra)->disp_subj = NULL;
   }
   /* spam and user headers should never be hashed, and the new envelope may
diff --git a/email/envelope.h b/email/envelope.h
index b124c10b..0202900f 100644
--- a/email/envelope.h
+++ b/email/envelope.h
@@ -67,8 +67,8 @@ struct Envelope
   char *list_post;                     ///< This stores a mailto URL, or nothing
   char *list_subscribe;                ///< This stores a mailto URL, or nothing
   char *list_unsubscribe;              ///< This stores a mailto URL, or nothing
-  char *subject;                       ///< Email's subject
-  char *real_subj;                     ///< Offset of the real subject
+  char *const subject;                 ///< Email's subject
+  char *const real_subj;               ///< Offset of the real subject
   char *disp_subj;                     ///< Display subject (modified copy of subject)
   char *message_id;                    ///< Message ID
   char *supersedes;                    ///< Supersedes header
@@ -117,6 +117,7 @@ bool             mutt_env_cmp_strict (const struct Envelope *e1, const struct En
 void             mutt_env_free       (struct Envelope **ptr);
 void             mutt_env_merge      (struct Envelope *base, struct Envelope **extra);
 struct Envelope *mutt_env_new        (void);
+void             mutt_env_set_subject(struct Envelope *env, const char *subj);
 bool             mutt_env_notify_send(struct Email *e, enum NotifyEnvelope type);
 int              mutt_env_to_intl    (struct Envelope *env, const char **tag, char **err);
 void             mutt_env_to_local   (struct Envelope *env);
diff --git a/email/parse.c b/email/parse.c
index 9310416a..24ae398c 100644
--- a/email/parse.c
+++ b/email/parse.c
@@ -936,7 +936,7 @@ int mutt_rfc822_parse_line(struct Envelope *env, struct Email *e,
       if ((name_len == 7) && eqi6(name + 1, "ubject"))
       {
         if (!env->subject)
-          env->subject = mutt_str_dup(body);
+          mutt_env_set_subject(env, body);
         matched = true;
       }
       else if ((name_len == 6) && eqi5(name + 1, "ender"))
@@ -1277,23 +1277,6 @@ struct Envelope *mutt_rfc822_read_header(FILE *fp, struct Email *e, bool user_hd
 
     rfc2047_decode_envelope(env);
 
-    if (env->subject)
-    {
-      regmatch_t pmatch[1];
-
-      const struct Regex *c_reply_regex = cs_subset_regex(NeoMutt->sub, "reply_regex");
-      if (mutt_regex_capture(c_reply_regex, env->subject, 1, pmatch))
-      {
-        env->real_subj = env->subject + pmatch[0].rm_eo;
-        if (env->real_subj[0] == '\0')
-          env->real_subj = NULL;
-      }
-      else
-      {
-        env->real_subj = env->subject;
-      }
-    }
-
     if (e->received < 0)
     {
       mutt_debug(LL_DEBUG1, "resetting invalid received time to 0\n");
diff --git a/email/rfc2047.c b/email/rfc2047.c
index d3141049..5980f019 100644
--- a/email/rfc2047.c
+++ b/email/rfc2047.c
@@ -838,7 +838,12 @@ void rfc2047_decode_envelope(struct Envelope *env)
   rfc2047_decode_addrlist(&env->return_path);
   rfc2047_decode_addrlist(&env->sender);
   rfc2047_decode(&env->x_label);
-  rfc2047_decode(&env->subject);
+
+  char * subj = env->subject;
+  *(char **)&env->subject = NULL;
+  rfc2047_decode(&subj);
+  mutt_env_set_subject(env, subj);
+  FREE(&subj);
 }
 
 /**
@@ -858,5 +863,10 @@ void rfc2047_encode_envelope(struct Envelope *env)
   rfc2047_encode_addrlist(&env->sender, "Sender");
   const struct Slist *const c_send_charset = cs_subset_slist(NeoMutt->sub, "send_charset");
   rfc2047_encode(&env->x_label, NULL, sizeof("X-Label:"), c_send_charset);
-  rfc2047_encode(&env->subject, NULL, sizeof("Subject:"), c_send_charset);
+
+  char * subj = env->subject;
+  *(char **)&env->subject = NULL;
+  rfc2047_encode(&subj, NULL, sizeof("Subject:"), c_send_charset);
+  mutt_env_set_subject(env, subj);
+  FREE(&subj);
 }
diff --git a/envelope/functions.c b/envelope/functions.c
index 0fda976d..c2e87660 100644
--- a/envelope/functions.c
+++ b/envelope/functions.c
@@ -287,7 +287,7 @@ static int op_envelope_edit_subject(struct EnvelopeWindowData *wdata, int op)
   if (mutt_str_equal(wdata->email->env->subject, buf_string(buf)))
     goto done; // no change
 
-  mutt_str_replace(&wdata->email->env->subject, buf_string(buf));
+  mutt_env_set_subject(wdata->email->env, buf_string(buf));
   mutt_env_notify_send(wdata->email, NT_ENVELOPE_SUBJECT);
   rc = FR_SUCCESS;
 
diff --git a/hcache/serialize.c b/hcache/serialize.c
index b5acfd06..92e2ed22 100644
--- a/hcache/serialize.c
+++ b/hcache/serialize.c
@@ -651,14 +651,14 @@ void serial_restore_envelope(struct Envelope *env, const unsigned char *d, int *
   if (c_auto_subscribe)
     mutt_auto_subscribe(env->list_post);
 
-  serial_restore_char(&env->subject, d, off, convert);
+  serial_restore_char((char **)&env->subject, d, off, convert);
   serial_restore_int((unsigned int *) (&real_subj_off), d, off);
 
   size_t len = mutt_str_len(env->subject);
   if ((real_subj_off < 0) || (real_subj_off >= len))
-    env->real_subj = NULL;
+    *(char **)&env->real_subj = NULL;
   else
-    env->real_subj = env->subject + real_subj_off;
+    *(char **)&env->real_subj = env->subject + real_subj_off;
 
   serial_restore_char(&env->message_id, d, off, false);
   serial_restore_char(&env->supersedes, d, off, false);
diff --git a/index/index.c b/index/index.c
index 978db75a..b426860f 100644
--- a/index/index.c
+++ b/index/index.c
@@ -198,9 +198,6 @@ static int config_reply_regex(struct MailboxView *mv)
 
   struct Mailbox *m = mv->mailbox;
 
-  regmatch_t pmatch[1];
-
-  const struct Regex *c_reply_regex = cs_subset_regex(NeoMutt->sub, "reply_regex");
   for (int i = 0; i < m->msg_count; i++)
   {
     struct Email *e = m->emails[i];
@@ -210,15 +207,7 @@ static int config_reply_regex(struct MailboxView *mv)
     if (!env || !env->subject)
       continue;
 
-    if (mutt_regex_capture(c_reply_regex, env->subject, 1, pmatch))
-    {
-      env->real_subj = env->subject + pmatch[0].rm_eo;
-      if (env->real_subj[0] == '\0')
-        env->real_subj = NULL;
-      continue;
-    }
-
-    env->real_subj = env->subject;
+    mutt_env_set_subject(env, env->subject);
   }
 
   OptResortInit = true; /* trigger a redraw of the index */
diff --git a/main.c b/main.c
index cad43894..af28f425 100644
--- a/main.c
+++ b/main.c
@@ -1000,7 +1000,7 @@ main
 
     if (subject)
     {
-      mutt_str_replace(&e->env->subject, subject);
+      mutt_env_set_subject(e->env, subject);
     }
 
     if (draft_file)
@@ -1142,7 +1142,7 @@ main
         mutt_addrlist_copy(&e->env->cc, &opts_env->cc, false);
         mutt_addrlist_copy(&e->env->bcc, &opts_env->bcc, false);
         if (opts_env->subject)
-          mutt_str_replace(&e->env->subject, opts_env->subject);
+          mutt_env_set_subject(e->env, opts_env->subject);
 
         mutt_env_free(&opts_env);
         email_free(&e_tmp);
diff --git a/ncrypt/crypt.c b/ncrypt/crypt.c
index 9756f528..c46ff284 100644
--- a/ncrypt/crypt.c
+++ b/ncrypt/crypt.c
@@ -273,7 +273,7 @@ int mutt_protect(struct Email *e, char *keylist, bool postpone)
   if (c_crypt_protected_headers_write)
   {
     struct Envelope *protected_headers = mutt_env_new();
-    mutt_str_replace(&protected_headers->subject, e->env->subject);
+    mutt_env_set_subject(protected_headers, e->env->subject);
     /* Note: if other headers get added, such as to, cc, then a call to
      * mutt_env_to_intl() will need to be added here too. */
     mutt_prepare_envelope(protected_headers, 0, NeoMutt->sub);
diff --git a/pager/message.c b/pager/message.c
index 90d37978..3c84e3e8 100644
--- a/pager/message.c
+++ b/pager/message.c
@@ -67,7 +67,6 @@ static const char *ExtPagerProgress = N_("all");
 static void process_protected_headers(struct Mailbox *m, struct Email *e)
 {
   struct Envelope *prot_headers = NULL;
-  regmatch_t pmatch[1];
 
   const bool c_crypt_protected_headers_read = cs_subset_bool(NeoMutt->sub, "crypt_protected_headers_read");
 #ifdef USE_AUTOCRYPT
@@ -121,19 +120,8 @@ static void process_protected_headers(struct Mailbox *m, struct Email *e)
     if (m->subj_hash && e->env->real_subj)
       mutt_hash_delete(m->subj_hash, e->env->real_subj, e);
 
-    mutt_str_replace(&e->env->subject, prot_headers->subject);
+    mutt_env_set_subject(e->env, prot_headers->subject);
     FREE(&e->env->disp_subj);
-    const struct Regex *c_reply_regex = cs_subset_regex(NeoMutt->sub, "reply_regex");
-    if (mutt_regex_capture(c_reply_regex, e->env->subject, 1, pmatch))
-    {
-      e->env->real_subj = e->env->subject + pmatch[0].rm_eo;
-      if (e->env->real_subj[0] == '\0')
-        e->env->real_subj = NULL;
-    }
-    else
-    {
-      e->env->real_subj = e->env->subject;
-    }
 
     if (m->subj_hash)
       mutt_hash_insert(m->subj_hash, e->env->real_subj, e);
diff --git a/postpone/postpone.c b/postpone/postpone.c
index 788cafc6..c49d5a2e 100644
--- a/postpone/postpone.c
+++ b/postpone/postpone.c
@@ -597,7 +597,7 @@ int mutt_prepare_template(FILE *fp, struct Mailbox *m, struct Email *e_new,
   if (c_crypt_protected_headers_read && protected_headers && protected_headers->subject &&
       !mutt_str_equal(e_new->env->subject, protected_headers->subject))
   {
-    mutt_str_replace(&e_new->env->subject, protected_headers->subject);
+    mutt_env_set_subject(e_new->env, protected_headers->subject);
   }
   mutt_env_free(&protected_headers);
 
diff --git a/send/send.c b/send/send.c
index 53a1a5b9..dc50a0c8 100644
--- a/send/send.c
+++ b/send/send.c
@@ -336,7 +336,7 @@ static int edit_envelope(struct Envelope *en, SendFlags flags, struct ConfigSubs
     mutt_message(_("No subject, aborting"));
     goto done;
   }
-  mutt_str_replace(&en->subject, buf_string(buf));
+  mutt_env_set_subject(en, buf_string(buf));
   rc = 0;
 
 done:
@@ -1045,7 +1045,7 @@ void mutt_make_forward_subject(struct Envelope *env, struct Email *e, struct Con
   /* set the default subject for the message. */
   mutt_make_string(buf, sizeof(buf), 0, NONULL(c_forward_format), NULL, -1, e,
                    MUTT_FORMAT_NO_FLAGS, NULL);
-  mutt_str_replace(&env->subject, buf);
+  mutt_env_set_subject(env, buf);
 }
 
 /**
@@ -1064,13 +1064,15 @@ void mutt_make_misc_reply_headers(struct Envelope *env, struct Envelope *env_cur
    * been taken from a List-Post header.  Is that correct?  */
   if (env_cur->real_subj)
   {
-    FREE(&env->subject);
-    mutt_str_asprintf(&(env->subject), "Re: %s", env_cur->real_subj);
+    char *subj;
+    mutt_str_asprintf(&subj, "Re: %s", env_cur->real_subj);
+    mutt_env_set_subject(env, subj);
+    free(subj);
   }
   else if (!env->subject)
   {
     const char *const c_empty_subject = cs_subset_string(sub, "empty_subject");
-    env->subject = mutt_str_dup(c_empty_subject);
+    mutt_env_set_subject(env, c_empty_subject);
   }
 }
 
@@ -2958,7 +2960,7 @@ static bool send_simple_email(struct Mailbox *m, struct EmailArray *ea,
   mutt_parse_mailto(e->env, NULL, mailto);
   if (!e->env->subject)
   {
-    e->env->subject = mutt_str_dup(subj);
+    mutt_env_set_subject(e->env, subj);
   }
   if (TAILQ_EMPTY(&e->env->to) && !mutt_addrlist_parse(&e->env->to, NULL))
   {
-- 
2.39.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://mailman.neomutt.org/pipermail/neomutt-devel-neomutt.org/attachments/20231221/9b8b54fa/attachment.sig>


More information about the neomutt-devel mailing list