On 2024/08/01 10:41:40 +0200, Walter Alejandro Iglesias <wai@roquesor.com> wrote:
> I fixed some mistakes and simplified a bit the code:
> 
>   https://en.roquesor.com/Downloads/mail_patches.tar.gz
> 
> I understand why my proposal doesn't attract too much attention, I can
> count only one person here using mail(1) as a MUA.  Anyway, I'd like to
> know if I'm doing something wrong.
> 
> At least I'd appreciate some guidance about the general procedure.
> Regarding the size, for example, I understand conceptually the
> convenience of splitting and committing a diff in small chunks to gain
> control and avoid introducing regressions, but isn't it necessary to
> test the full functional patches first, to evaluate what they do?

No, it is not.  For example, Date and User-agent are completely
orthogonal to MIME support or UTF8.  So, while I'm happy to see this
interest in improving mail(1), let's try to get it down to small pieces
that can be reviewed and everyone agree on.  Especially when some
"controversial" or, rather, delicate topics like UTF-8 are involved.
The risk is to derail the thread on some details and so loosing all the
other stuff.

So, let's discuss the first (?) patch, the one to add the Date and
User-Agent header.  We'll do separate threads for the others when I'll
get to review them.

Personally I don't feel a strong need for User-Agent, but given that
even mblaze(1) sets it by default I think it's fine.  I'm a bit unsure
about the format, your diff has "OpenBSD mail(1)" and the (...) are
usually comments in mail headers, so I've changed it to just "OpenBSD
mail".

Then, we can't use strftime() since it depends on the locale.  (On
OpenBSD this technically doesn't matter, but let's do the right thing.)
So, I've stolen a function from OpenSMTPD.  (which opens the problem of
the copyright and ISC vs BSD 3 clausole)

Anyway, here's the diff.  Except for the copyright "problem" from which
I'd like to hear other opinions, I think this is fine and is ok op@ if
someone wants to go ahead with this.


Cheers,

Omar Polo


P.S. minor style(9) nits included as well.

diff /home/op/w/mail
commit - 80617155f9fa32a62f9dbcbea7b90ebbdb39ec14
path + /home/op/w/mail
blob - 95508398058ef09067b62f596685a602cabbe85e
file + extern.h
--- extern.h
+++ extern.h
@@ -100,6 +100,7 @@ int	 collabort(void);
 void	 commands(void);
 int	 copycmd(void *);
 int	 count(struct name *);
+char	*date(void);
 int	 deletecmd(void *);
 int	 delm(int *);
 int	 deltype(void *);
blob - 9582675f9b851583f8487aae8ff1b82e70bf01d4
file + send.c
--- send.c
+++ send.c
@@ -519,12 +519,16 @@ puthead(struct header *hp, FILE *fo, int w)
 
 	gotcha = 0;
 	from = hp->h_from ? hp->h_from : value("from");
+	if (date() != NULL && fo != stdout)
+		fprintf(fo, "Date: %s\n", date()), gotcha++;
 	if (from != NULL)
 		fprintf(fo, "From: %s\n", from), gotcha++;
 	if (hp->h_to != NULL && w & GTO)
 		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\n"), gotcha++;
 	if (hp->h_cc != NULL && w & GCC)
 		fmt("Cc:", hp->h_cc, fo, w&GCOMMA), gotcha++;
 	if (hp->h_bcc != NULL && w & GBCC)
blob - 68fa944bcdc214a3e7ec65d28e886d522c9c30cb
file + util.c
--- util.c
+++ util.c
@@ -30,8 +30,12 @@
  * SUCH DAMAGE.
  */
 
-#include "rcv.h"
+#include <sys/time.h>
+
 #include <fcntl.h>
+#include <time.h>
+
+#include "rcv.h"
 #include "extern.h"
 
 /*
@@ -641,3 +645,43 @@ clearnew(void)
 		}
 	}
 }
+
+/*
+ * From usr.sbin/smtpd/to.c:/^time_to_text
+ */
+char *
+date(void)
+{
+	struct tm *lt;
+	static char buf[40];
+	const char *day[] = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
+	const char *month[] = {"Jan","Feb","Mar","Apr","May","Jun",
+	    "Jul","Aug","Sep","Oct","Nov","Dec"};
+	const char *tz;
+	time_t now;
+	long offset;
+	int r;
+
+	now = time(NULL);
+	lt = localtime(&now);
+	if (lt == NULL || now == 0)
+		err(1, "date: localtime");
+
+	offset = lt->tm_gmtoff;
+	tz = lt->tm_zone;
+
+	/* We do not use strftime because it is subject to locale substitution*/
+	r = snprintf(buf, sizeof(buf),
+	    "%s, %d %s %d %02d:%02d:%02d %c%02d%02d (%s)",
+	    day[lt->tm_wday], lt->tm_mday, month[lt->tm_mon],
+	    lt->tm_year + 1900,
+	    lt->tm_hour, lt->tm_min, lt->tm_sec,
+	    offset >= 0 ? '+' : '-',
+	    abs((int)offset / 3600),
+	    abs((int)offset % 3600) / 60,
+	    tz);
+	if (r < 0 || (size_t)r >= sizeof(buf))
+		errx(1, "date: bsnprintf");
+
+	return buf;
+}