Index | Thread | Search

From:
Walter Alejandro Iglesias <wai@roquesor.com>
Subject:
mail(1) fixes, (third round, ping!)
To:
tech@openbsd.org
Date:
Sat, 25 May 2024 14:53:49 +0200

Download raw body.

Thread
  • Walter Alejandro Iglesias:

    mail(1) fixes, (third round, ping!)

About this:

On Fri, 02 Feb 2024 07:57:43 -0700 Theo de Raadt wrote:
> I would be happy to see improvements to mail(1), but past improvements
> have tried to do too much in one go.  The work must be in small
> incremental steps, each on judged on their own.  It is good that this is
> finally getting split up into pieces.  But I think the steps are still
> too large.

Conceptually I understand what Theo asks in that message.  But, this is
what has happened so far:

1. In my first proposal[1] (long thread) some developers insisted on
   keeping my first idea (the one that added the '-m' option) arguing
   that my later modifications were "going off the rails."  I continued
   because a better solution came to mind, which led me in a different
   direction and also to the solution of other issues.  If that first
   "safe small step" had been accepted and committed, my best solution
   would've gone to the trash, or at least would've involved undoing what
   was done.

   (1) https://marc.info/?t=169512630100001&r=2&w=2

2. I fail to see (an example or clue will be welcome) how to go further
   in splitting my diffs *without breaking the functionality they were
   created for*.  Of course I can remove the handle from the hammer, but
   it won't be a hammer anymore.

I'd like to add that I've been "eating my own shit" for a while and, so
far, I experienced only one issue due to an oversight in my third diff,
which I corrected time ago.

This time I generated the diffs in a way you'll be able to apply them
consecutively (and see its real size):

  https://en.roquesor.com/Downloads/mail_patches.tar.gz

The tar file contains a little reference of what each diff does.  Below
I paste the first diff, which generates a Message-Id and adds
In-Reply-To.  At first glance you'll notice that only adds variables
here and there and a sixteen lines only function at the end which I cut
and pasted from smtpd(8).  Not a big deal.


Index: cmd3.c
===================================================================
RCS file: /cvs/src/usr.bin/mail/cmd3.c,v
retrieving revision 1.30
diff -u -p -r1.30 cmd3.c
--- cmd3.c	8 Mar 2023 04:43:11 -0000	1.30
+++ cmd3.c	20 Oct 2023 17:44:58 -0000
@@ -238,6 +238,7 @@ _respond(int *msgvec)
 		head.h_cc = np;
 	} else
 		head.h_cc = NULL;
+	head.h_msgid = hfield("message-id", mp);
 	head.h_bcc = NULL;
 	head.h_smopts = NULL;
 	mail1(&head, 1);
@@ -617,6 +618,7 @@ _Respond(int *msgvec)
 	if ((head.h_subject = hfield("subject", mp)) == NULL)
 		head.h_subject = hfield("subj", mp);
 	head.h_subject = reedit(head.h_subject);
+	head.h_msgid = hfield("message-id", mp);
 	head.h_from = NULL;
 	head.h_cc = NULL;
 	head.h_bcc = NULL;
Index: collect.c
===================================================================
RCS file: /cvs/src/usr.bin/mail/collect.c,v
retrieving revision 1.34
diff -u -p -r1.34 collect.c
--- collect.c	17 Jan 2014 18:42:30 -0000	1.34
+++ collect.c	20 Oct 2023 17:44:58 -0000
@@ -87,7 +87,7 @@ collect(struct header *hp, int printhead
 	 * refrain from printing a newline after
 	 * the headers (since some people mind).
 	 */
-	t = GTO|GSUBJECT|GCC|GNL;
+	t = GTO|GSUBJECT|GMID|GCC|GNL;
 	getsub = 0;
 	if (hp->h_subject == NULL && value("interactive") != NULL &&
 	    (value("ask") != NULL || value("asksub") != NULL))
@@ -208,7 +208,7 @@ cont:
 			/*
 			 * Grab a bunch of headers.
 			 */
-			grabh(hp, GTO|GSUBJECT|GCC|GBCC);
+			grabh(hp, GTO|GSUBJECT|GMID|GCC|GBCC);
 			goto cont;
 		case 't':
 			/*
@@ -328,7 +328,7 @@ cont:
 			 */
 			rewind(collf);
 			puts("-------\nMessage contains:");
-			puthead(hp, stdout, GTO|GSUBJECT|GCC|GBCC|GNL);
+			puthead(hp, stdout, GTO|GSUBJECT|GMID|GCC|GBCC|GNL);
 			while ((t = getc(collf)) != EOF)
 				(void)putchar(t);
 			goto cont;
Index: def.h
===================================================================
RCS file: /cvs/src/usr.bin/mail/def.h,v
retrieving revision 1.17
diff -u -p -r1.17 def.h
--- def.h	28 Jan 2022 06:18:41 -0000	1.17
+++ def.h	20 Oct 2023 17:44:58 -0000
@@ -53,6 +53,7 @@
 #include <termios.h>
 #include <unistd.h>
 #include <limits.h>
+#include <inttypes.h>
 #include <vis.h>
 #include "pathnames.h"
 
@@ -156,14 +157,15 @@ struct headline {
 
 #define	GTO	1		/* Grab To: line */
 #define	GSUBJECT 2		/* Likewise, Subject: line */
-#define	GCC	4		/* And the Cc: line */
-#define	GBCC	8		/* And also the Bcc: line */
-#define	GMASK	(GTO|GSUBJECT|GCC|GBCC)
+#define	GMID	4		/* Message-ID: line */
+#define	GCC	8		/* And the Cc: line */
+#define	GBCC	16		/* And also the Bcc: line */
+#define	GMASK	(GTO|GSUBJECT|GMID|GCC|GBCC)
 				/* Mask of places from whence */
 
-#define	GNL	16		/* Print blank line after */
-#define	GDEL	32		/* Entity removed from list */
-#define	GCOMMA	64		/* detract puts in commas */
+#define	GNL	32		/* Print blank line after */
+#define	GDEL	64		/* Entity removed from list */
+#define	GCOMMA	128		/* detract puts in commas */
 
 /*
  * Structure used to pass about the current
@@ -173,6 +175,7 @@ struct header {
 	struct name *h_to;		/* Dynamic "To:" string */
 	char *h_from;			/* User-specified "From:" string */
 	char *h_subject;		/* Subject string */
+	char *h_msgid;			/* Message-ID string */
 	struct name *h_cc;		/* Carbon copies string */
 	struct name *h_bcc;		/* Blind carbon copies */
 	struct name *h_smopts;		/* Sendmail options */
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.bin/mail/extern.h,v
retrieving revision 1.29
diff -u -p -r1.29 extern.h
--- extern.h	16 Sep 2018 02:38:57 -0000	1.29
+++ extern.h	20 Oct 2023 17:44:58 -0000
@@ -129,6 +129,7 @@ int	 forward(char *, FILE *, char *, int
 void	 free_child(pid_t);
 int	 from(void *);
 off_t	 fsize(FILE *);
+uint64_t generate_uid(void);
 int	 getfold(char *, int);
 int	 gethfield(FILE *, char *, int, char **);
 int	 gethfromtty(struct header *, int);
@@ -163,7 +164,7 @@ void	 load(char *);
 struct var *
 	 lookup(char *);
 int	 mail(struct name *, struct name *, struct name *, struct name *,
-	       char *, char *);
+	       char *, char *, char *);
 void	 mail1(struct header *, int);
 void	 makemessage(FILE *, int);
 void	 mark(int);
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/mail/main.c,v
retrieving revision 1.35
diff -u -p -r1.35 main.c
--- main.c	26 Jan 2021 18:21:47 -0000	1.35
+++ main.c	20 Oct 2023 17:44:58 -0000
@@ -103,6 +103,7 @@ main(int argc, char **argv)
 	struct name *to, *cc, *bcc, *smopts;
 	char *fromaddr;
 	char *subject;
+	char *msgid;
 	char *ef;
 	char nosrc = 0;
 	char *rc;
@@ -136,6 +137,7 @@ main(int argc, char **argv)
 	smopts = NULL;
 	fromaddr = NULL;
 	subject = NULL;
+	msgid = NULL;
 	while ((i = getopt(argc, argv, "EINb:c:dfinr:s:u:v")) != -1) {
 		switch (i) {
 		case 'u':
@@ -269,7 +271,7 @@ main(int argc, char **argv)
 		rc = "~/.mailrc";
 	load(expand(rc));
 	if (!rcvmode) {
-		mail(to, cc, bcc, smopts, fromaddr, subject);
+		mail(to, cc, bcc, smopts, fromaddr, subject, msgid);
 		/*
 		 * why wait?
 		 */
Index: send.c
===================================================================
RCS file: /cvs/src/usr.bin/mail/send.c,v
retrieving revision 1.26
diff -u -p -r1.26 send.c
--- send.c	8 Mar 2023 04:43:11 -0000	1.26
+++ send.c	20 Oct 2023 17:44:58 -0000
@@ -279,13 +279,14 @@ statusput(struct message *mp, FILE *obuf
  */
 int
 mail(struct name *to, struct name *cc, struct name *bcc, struct name *smopts,
-     char *fromaddr, char *subject)
+     char *fromaddr, char *subject, char *msgid)
 {
 	struct header head;
 
 	head.h_to = to;
 	head.h_from = fromaddr;
 	head.h_subject = subject;
+	head.h_msgid = msgid;
 	head.h_cc = cc;
 	head.h_bcc = bcc;
 	head.h_smopts = smopts;
@@ -306,6 +307,7 @@ sendmail(void *v)
 	head.h_to = extract(str, GTO);
 	head.h_from = NULL;
 	head.h_subject = NULL;
+	head.h_msgid = NULL;
 	head.h_cc = NULL;
 	head.h_bcc = NULL;
 	head.h_smopts = NULL;
@@ -482,7 +484,7 @@ infix(struct header *hp, FILE *fi)
 		return(fi);
 	}
 	(void)rm(tempname);
-	(void)puthead(hp, nfo, GTO|GSUBJECT|GCC|GBCC|GNL|GCOMMA);
+	(void)puthead(hp, nfo, GTO|GSUBJECT|GMID|GCC|GBCC|GNL|GCOMMA);
 	c = getc(fi);
 	while (c != EOF) {
 		(void)putc(c, nfo);
@@ -516,6 +518,8 @@ puthead(struct header *hp, FILE *fo, int
 {
 	int gotcha;
 	char *from;
+	char *inrepto;
+	char hostname[256];
 
 	gotcha = 0;
 	from = hp->h_from ? hp->h_from : value("from");
@@ -525,6 +529,17 @@ puthead(struct header *hp, FILE *fo, int
 		fmt("To:", hp->h_to, fo, w&GCOMMA), gotcha++;
 	if (hp->h_subject != NULL && w & GSUBJECT)
 		fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
+	if (fo != stdout) {
+		fprintf(fo, "User-Agent: OpenBSD mail(1)\n"),
+		    gotcha++;
+		if (gethostname(hostname, sizeof(hostname)) != -1)
+			fprintf(fo, "Message-ID: <%016"PRIx64"@%s>\n",
+			    generate_uid(), hostname),
+			    gotcha++;
+		if (hp->h_msgid != NULL && w & GMID)
+			fprintf(fo, "In-Reply-To: %s\n", hp->h_msgid),
+			    gotcha++;
+	}
 	if (hp->h_cc != NULL && w & GCC)
 		fmt("Cc:", hp->h_cc, fo, w&GCOMMA), gotcha++;
 	if (hp->h_bcc != NULL && w & GBCC)
Index: util.c
===================================================================
RCS file: /cvs/src/usr.bin/mail/util.c,v
retrieving revision 1.2
diff -u -p -r1.2 util.c
--- util.c	26 Dec 2022 19:16:01 -0000	1.2
+++ util.c	20 Oct 2023 17:44:58 -0000
@@ -641,3 +641,20 @@ clearnew(void)
 		}
 	}
 }
+
+uint64_t
+generate_uid(void)
+{
+	static uint32_t id;
+	static uint8_t	inited;
+	uint64_t	uid;
+
+	if (!inited) {
+		id = arc4random();
+		inited = 1;
+	}
+	while ((uid = ((uint64_t)(id++) << 32 | arc4random())) == 0)
+		;
+
+	return (uid);
+}


-- 
Walter