Skip to content

fix for allmailfrom#84

Open
adippl wants to merge 1 commit intobruceg:masterfrom
adippl:allmailfrom_fix
Open

fix for allmailfrom#84
adippl wants to merge 1 commit intobruceg:masterfrom
adippl:allmailfrom_fix

Conversation

@adippl
Copy link

@adippl adippl commented May 26, 2022

This commit moves allmailfrom functionality from queue.cc to inject.cc
Using allmailfrom now gives nullmailer ability to overwrite existing
From: fields in mail headers.
This solves issue with allmailfrom not being effective on messages
which already have From: set before they enter sendmail.

This commit moves allmailfrom functionality from queue.cc to inject.cc
Using allmailfrom now gives nullmailer ability to overwrite existing
From: fields in mail headers.
This solves issue with allmailfrom not being effective on messages
which already have From: set before they enter sendmail.
@adippl adippl changed the title fix for allmailform fix for allmailfrom May 26, 2022
@mtfurlan
Copy link

This doesn't pass tests, because there is a test for allmailfrom with nullmailer-queue modifying the envelope sender.

The documentation is also incorrect.

This will make it pass tests, and I think the documentation is now correct.

diff --git a/doc/nullmailer-inject.1 b/doc/nullmailer-inject.1
index f000e9e..67b55ff 100644
--- a/doc/nullmailer-inject.1
+++ b/doc/nullmailer-inject.1
@@ -218,6 +218,10 @@ to queue the formatted message.
 When reading the following files, a single line is read and stripped
 of all leading and trailing whitespace characters.
 .TP
+.B allmailfrom
+If this file is not empty, its contents will override the envelope
+sender and From header on all messages.
+.TP
 .B defaultdomain
 The content of this file is appended to any host name that does not
 contain a period (except
diff --git a/doc/nullmailer-queue.8 b/doc/nullmailer-queue.8
index 97aa92f..f25f05b 100644
--- a/doc/nullmailer-queue.8
+++ b/doc/nullmailer-queue.8
@@ -29,10 +29,6 @@ This is provided to allow local daemons to be able to send email to
 "somebody@localhost" and have it go somewhere sensible instead of
 being bounced by your relay host. To send to multiple addresses, put
 them all on one line separated by a comma.
-.TP
-.B allmailfrom
-If this file is not empty, its contents will override the envelope
-sender on all messages.
 .SH OTHER FILES
 .TP
 .B /var/spool/nullmailer/queue
diff --git a/doc/nullmailer.7 b/doc/nullmailer.7
index 6ccb524..31b3b78 100644
--- a/doc/nullmailer.7
+++ b/doc/nullmailer.7
@@ -22,7 +22,7 @@ The following table lists all the control files used by
 .ta 5c
 control file	used by
 .I adminaddr	\fBnullmailer-dsn\fR, \fBnullmailer-queue
-.I allmailfrom	\fBnullmailer-queue
+.I allmailfrom	\fBnullmailer-inject
 .I defaultdomain	\fBnullmailer-dsn\fR, \fBnullmailer-inject
 .I defaulthost	\fBnullmailer-dsn\fR, \fBnullmailer-inject
 .I doublebounceto	\fBnullmailer-dsn
diff --git a/test/tests/inject/from b/test/tests/inject/from
index d27ee03..acdc108 100644
--- a/test/tests/inject/from
+++ b/test/tests/inject/from
@@ -39,6 +39,11 @@ echo "Checking that inject ignores config/defaultdomain for localhost"
 echo localhost >$SYSCONFDIR/defaulthost
 inj | grep -q '@localhost>$'
 
+echo "Checking that inject respects config/allmailfrom"
+echo admin@domain.tld >$SYSCONFDIR/allmailfrom
+inj | grep -q '<admin@domain.tld>'
+rm $SYSCONFDIR/allmailfrom
+
 testvar HOSTNAME test1.org '@test1.org>$'
 
 testvar MAILHOST test2.org '@test2.org>$'
diff --git a/test/tests/queue/rewrite b/test/tests/queue/rewrite
index 780aa3b..1ff3e35 100644
--- a/test/tests/queue/rewrite
+++ b/test/tests/queue/rewrite
@@ -68,15 +68,3 @@ header
 
 data
 EOF
-
-echo "Checking that queue rewrites sender with allmailfrom."
-echo sender@remote3 >$SYSCONFDIR/allmailfrom
-que-sender '^sender@remote3$' <<EOF
-bruceg@qcc.sk.ca
-user@localhost
-
-header
-
-data
-EOF
-rm -f $SYSCONFDIR/allmailfrom

@juduroe
Copy link

juduroe commented Aug 8, 2025

Hi all, this solves a frequently addressed problem with allmailfrom (see #72 ). Is there anything left preventing an integration of the suggestions?

@jniggemann
Copy link

Can we get this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants