git: 59597032c948 - main - mail: Don't trap signals we shouldn't.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Sun, 27 Apr 2025 06:29:54 UTC
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=59597032c948586257f123f57bbcfbad02a2cd1b

commit 59597032c948586257f123f57bbcfbad02a2cd1b
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-04-27 06:29:20 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-04-27 06:29:32 +0000

    mail: Don't trap signals we shouldn't.
    
    When in interactive mode, trap SIGINT, SIGHUP, and tty-related signals.
    Otherwise, leave signals untouched as required by POSIX.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D50011
---
 etc/mtree/BSD.tests.dist              |   2 +
 usr.bin/mail/Makefile                 |   5 ++
 usr.bin/mail/collect.c                |  34 ++++----
 usr.bin/mail/tests/Makefile           |   4 +
 usr.bin/mail/tests/mail_sigint_test.c | 149 ++++++++++++++++++++++++++++++++++
 5 files changed, 180 insertions(+), 14 deletions(-)

diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist
index 20744e7b944a..673452434080 100644
--- a/etc/mtree/BSD.tests.dist
+++ b/etc/mtree/BSD.tests.dist
@@ -1165,6 +1165,8 @@
         ..
         m4
         ..
+        mail
+        ..
         mkimg
         ..
         mktemp
diff --git a/usr.bin/mail/Makefile b/usr.bin/mail/Makefile
index c20060d88ebf..0a27b5c5a637 100644
--- a/usr.bin/mail/Makefile
+++ b/usr.bin/mail/Makefile
@@ -1,3 +1,5 @@
+.include <src.opts.mk>
+
 CONFS=	misc/mail.rc
 PROG=	mail
 SRCS=	version.c cmd1.c cmd2.c cmd3.c cmdtab.c collect.c edit.c fio.c \
@@ -17,4 +19,7 @@ etc-mailrc:
 	cd ${.CURDIR}/misc; ${INSTALL} -o root -g wheel \
 	    -m 644 ${EFILES} ${DESTDIR}/etc
 
+HAS_TESTS=
+SUBDIR.${MK_TESTS}=	tests
+
 .include <bsd.prog.mk>
diff --git a/usr.bin/mail/collect.c b/usr.bin/mail/collect.c
index 0c310b15b234..2423bcf7b262 100644
--- a/usr.bin/mail/collect.c
+++ b/usr.bin/mail/collect.c
@@ -79,16 +79,20 @@ collect(struct header *hp, int printheaders)
 	 * until we're in the main loop.
 	 */
 	(void)sigemptyset(&nset);
-	(void)sigaddset(&nset, SIGINT);
-	(void)sigaddset(&nset, SIGHUP);
+	if (value("interactive") != NULL) {
+		(void)sigaddset(&nset, SIGINT);
+		(void)sigaddset(&nset, SIGHUP);
+	}
 	(void)sigprocmask(SIG_BLOCK, &nset, NULL);
-	if ((saveint = signal(SIGINT, SIG_IGN)) != SIG_IGN)
-		(void)signal(SIGINT, collint);
-	if ((savehup = signal(SIGHUP, SIG_IGN)) != SIG_IGN)
-		(void)signal(SIGHUP, collhup);
-	savetstp = signal(SIGTSTP, collstop);
-	savettou = signal(SIGTTOU, collstop);
-	savettin = signal(SIGTTIN, collstop);
+	if (value("interactive") != NULL) {
+		if ((saveint = signal(SIGINT, SIG_IGN)) != SIG_IGN)
+			(void)signal(SIGINT, collint);
+		if ((savehup = signal(SIGHUP, SIG_IGN)) != SIG_IGN)
+			(void)signal(SIGHUP, collhup);
+		savetstp = signal(SIGTSTP, collstop);
+		savettou = signal(SIGTTOU, collstop);
+		savettin = signal(SIGTTIN, collstop);
+	}
 	if (setjmp(collabort) || setjmp(colljmp)) {
 		(void)rm(tempname);
 		goto err;
@@ -473,11 +477,13 @@ out:
 		rewind(collf);
 	noreset--;
 	(void)sigprocmask(SIG_BLOCK, &nset, NULL);
-	(void)signal(SIGINT, saveint);
-	(void)signal(SIGHUP, savehup);
-	(void)signal(SIGTSTP, savetstp);
-	(void)signal(SIGTTOU, savettou);
-	(void)signal(SIGTTIN, savettin);
+	if (value("interactive") != NULL) {
+		(void)signal(SIGINT, saveint);
+		(void)signal(SIGHUP, savehup);
+		(void)signal(SIGTSTP, savetstp);
+		(void)signal(SIGTTOU, savettou);
+		(void)signal(SIGTTIN, savettin);
+	}
 	(void)sigprocmask(SIG_UNBLOCK, &nset, NULL);
 	return (collf);
 }
diff --git a/usr.bin/mail/tests/Makefile b/usr.bin/mail/tests/Makefile
new file mode 100644
index 000000000000..cb194b3da1f1
--- /dev/null
+++ b/usr.bin/mail/tests/Makefile
@@ -0,0 +1,4 @@
+PACKAGE=	tests
+ATF_TESTS_C+=	mail_sigint_test
+
+.include <bsd.test.mk>
diff --git a/usr.bin/mail/tests/mail_sigint_test.c b/usr.bin/mail/tests/mail_sigint_test.c
new file mode 100644
index 000000000000..76bf9b19b7f1
--- /dev/null
+++ b/usr.bin/mail/tests/mail_sigint_test.c
@@ -0,0 +1,149 @@
+/*-
+ * Copyright (c) 2025 Klara, Inc.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <sys/poll.h>
+#include <sys/wait.h>
+
+#include <fcntl.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <atf-c.h>
+
+#define MAILX	"mailx"
+#define BODY	"hello\n"
+#define BODYLEN	(sizeof(BODY) - 1)
+
+/*
+ * When interactive, mailx(1) should print a message on receipt of SIGINT,
+ * then exit cleanly on receipt of a second.
+ *
+ * When not interactive, mailx(1) should terminate on receipt of SIGINT.
+ */
+static void
+mailx_sigint(bool interactive)
+{
+	char obuf[1024] = "";
+	char ebuf[1024] = "";
+	struct pollfd fds[2];
+	int ipd[2], opd[2], epd[2], spd[2];
+	size_t olen = 0, elen = 0;
+	ssize_t rlen;
+	pid_t pid;
+	int kc, status;
+
+	/* input, output, error, sync pipes */
+	if (pipe(ipd) != 0 || pipe(opd) != 0 || pipe(epd) != 0 ||
+	    pipe2(spd, O_CLOEXEC) != 0)
+		atf_tc_fail("failed to pipe");
+	/* fork child */
+	if ((pid = fork()) < 0)
+		atf_tc_fail("failed to fork");
+	if (pid == 0) {
+		/* child */
+		dup2(ipd[0], STDIN_FILENO);
+		close(ipd[0]);
+		close(ipd[1]);
+		dup2(opd[1], STDOUT_FILENO);
+		close(opd[0]);
+		close(opd[1]);
+		dup2(epd[1], STDERR_FILENO);
+		close(epd[0]);
+		close(epd[1]);
+		/* force dead.letter to go to cwd */
+		setenv("HOME", ".", 1);
+		/* exec mailx */
+		execlp(MAILX,
+		    MAILX,
+		    interactive ? "-Is" : "-s",
+		    "test",
+		    "test@example.com",
+		    NULL);
+		_exit(2);
+	}
+	/* parent */
+	close(ipd[0]);
+	close(opd[1]);
+	close(epd[1]);
+	close(spd[1]);
+	/* block until child execs or exits */
+	(void)read(spd[0], &spd[1], sizeof(spd[1]));
+	/* send one line of input */
+	ATF_REQUIRE_INTEQ(BODYLEN, write(ipd[1], BODY, BODYLEN));
+	/* give it a chance to process */
+	poll(NULL, 0, 2000);
+	/* send first SIGINT */
+	ATF_CHECK_INTEQ(0, kill(pid, SIGINT));
+	kc = 1;
+	/* receive output until child terminates */
+	fds[0].fd = opd[0];
+	fds[0].events = POLLIN;
+	fds[1].fd = epd[0];
+	fds[1].events = POLLIN;
+	for (;;) {
+		if (poll(fds, 2, 1000) < 0)
+			atf_tc_fail("failed to poll");
+		if (fds[0].revents == POLLIN && olen < sizeof(obuf)) {
+			rlen = read(opd[0], obuf + olen, sizeof(obuf) - olen - 1);
+			if (rlen < 0)
+				atf_tc_fail("failed to read");
+			olen += rlen;
+		}
+		if (fds[1].revents == POLLIN && elen < sizeof(ebuf)) {
+			rlen = read(epd[0], ebuf + elen, sizeof(ebuf) - elen - 1);
+			if (rlen < 0)
+				atf_tc_fail("failed to read");
+			elen += rlen;
+		}
+		if (elen > 0 && kc == 1) {
+			kill(pid, SIGINT);
+			kc++;
+		}
+		if (waitpid(pid, &status, WNOHANG) == pid)
+			break;
+	}
+	close(ipd[1]);
+	close(opd[0]);
+	close(epd[0]);
+	close(spd[0]);
+	if (interactive) {
+		ATF_CHECK(WIFEXITED(status));
+		ATF_CHECK_INTEQ(0, WEXITSTATUS(status));
+		ATF_CHECK_INTEQ(2, kc);
+		ATF_CHECK_STREQ("", obuf);
+		ATF_CHECK_MATCH("Interrupt -- one more to kill letter", ebuf);
+		atf_utils_compare_file("dead.letter", BODY);
+	} else {
+		ATF_CHECK(WIFSIGNALED(status));
+		ATF_CHECK_INTEQ(SIGINT, WTERMSIG(status));
+		ATF_CHECK_INTEQ(1, kc);
+		ATF_CHECK_STREQ("", obuf);
+		ATF_CHECK_STREQ("", ebuf);
+		ATF_CHECK_INTEQ(-1, access("dead.letter", F_OK));
+	}
+}
+
+
+ATF_TC_WITHOUT_HEAD(mail_sigint_interactive);
+ATF_TC_BODY(mail_sigint_interactive, tc)
+{
+	mailx_sigint(true);
+}
+
+ATF_TC_WITHOUT_HEAD(mail_sigint_noninteractive);
+ATF_TC_BODY(mail_sigint_noninteractive, tc)
+{
+	mailx_sigint(false);
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+	ATF_TP_ADD_TC(tp, mail_sigint_interactive);
+	ATF_TP_ADD_TC(tp, mail_sigint_noninteractive);
+	return (atf_no_error());
+}