Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for the rewritefrom functionality #20

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

keithbare2
Copy link

Closes #12

Gmail has been rejecting some of the messages sent to ezmlm-idx mailing lists I help run. Eventually I determined it was due to the outgoing messages having two different Cc headers: one from the original poster and then a second with the poster's address, the latter having been added by the rewritefrom logic (we use replytolist). I saw that issue #12 mentions the same problem happening with Reply-To headers.

We had not noticed the issue with encoded words that is perhaps the main complaint in issue #12. (We tend to all be Americans with 7-bit ASCII names.)

In any case, the duplicated headers problem is causing us a lot of trouble. Essentially we lose messages whenever somebody tries to use reply-all to messages sent on our lists. So I've implemented a fix. And since it was in the same GitHub issue, I also took a swing at maintaining encoded words in the author name of the rewritten From header.

The comments for author_name() and author_addr() did not accurately describe
what the code is actually doing.
Copy rfc2047 encoded words in the original From line to the rewritten From
line without decoding them.

The encoded words also need to remain unquoted in the rewritten From line.
To accomplish this, do not quote the entire result of the AUTHOR_VIA_LIST
template substitution. Instead, if the original From line's author name text
was from a quoted string, comment or bare address, quote the author name
text before substituting. Otherwise, the original From line's author name
text (which will not contain a quoted string) is substituted verbatim.
Running ezmlm-test does not require a working qmail installation.

Do not explicitly warn about the dot.qmail-__tstlist* files, because
they're created in __TSTDIR.
When processing a message's headers:

 - Make note if Reply-To was seen.
 - Save off and do not copy the Cc header to the output immediately.
 - Save off and do not copy the From header to the output immediately.

At the end of the message headers, call rewrite_from().

When flagrewritefrom is set, either by the list's configuration or
because a DMARC reject policy applies to the message sender's address:

 - The From header is rewritten as before.
 - If replytolist is not enabled, a Reply-To header is only added
   if the original message did not have one. Rationale is that the
   original Reply-To header indicated where the post author is
   interested in receiving responses, not the From header.
 - When replytolist is enabled and the original message had a Cc header,
   the From address is appended to the existing Cc header.
 - When replytolist is enabled and the original message did not have a
   Cc header, generate a new one using the From address.

After rewrite_from() performs any of the applicable manipulations
described above, the resulting From and Cc headers are copied to the
output.

Update tests/551-ezmlm-send-rewritefrom:

 - Confirm Reply-To and Cc headers are not duplicated.
 - Validate behavior when replytolist is configured.
@keithbare2 keithbare2 force-pushed the rewrite_from_fixes branch from 70f572d to 0a719d3 Compare March 5, 2023 03:01
@keithbare2
Copy link
Author

The primary purpose of the update was to resolve an issue in my original changes that arose for MIME messages with a multipart Content-Type. In that case, flagbadpart would get set on the header -> body transition, and inhibit output of the line containing the rewritten From and new Reply-To/Cc headers. To resolve, the actual line content is output when flagbadpart is set, rather than assuming only a "\n" is required.

@raboof
Copy link

raboof commented Feb 6, 2024

This looks great, thanks so much for working on this! We're also running into the duplicate Reply-To/Cc issue, and at first glance c179dfd looks like a reasonable solution.

Perhaps it should be its own PR, to make it more self-contained and likely to get merged?

bin/ezmlm-send.c Outdated Show resolved Hide resolved
When rewritefrom needs to append the original From address to an existing Cc
header the from address is placed on the next line. Whitespace is required
at the beginning of the line so the added line is treated as an extension of
the previous header. Ensure there is whitespace at the beginning of the
line, in case there was not any after the colon in the From header.
@keithbare2
Copy link
Author

This looks great, thanks so much for working on this! We're also running into the duplicate Reply-To/Cc issue

You're welcome. Starting in March 2023, I've been running several cmucc mailing lists with the changes I posted here and haven't noticed any problems.

In preparation for Google's sender requirements going into effect, I wanted to switch everything to use rewritefrom unconditionally. I found myself rebuilding ezmlm-idx and was reminded that I faced a few problems building and installing code based on the master branch here. I collected my fixes for those problems and posted them in #22. I figure that might help if you or anybody else wants to build and run with changes from this PR.

[A]t first glance c179dfd looks like a reasonable solution.

Perhaps it should be its own PR, to make it more self-contained and likely to get merged?

The only problem with using that commit on its own is that the tests I wrote have a dependency on the way 3bf20d9 changed the quoting of the From header's display name. Should be straightforward to resolve though.

I haven't heard anything from the owner or anybody else that could merge a PR. I certainly could create a new PR with just the duplicate header fix if they ask.

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.

rewritefrom does not encode headers correctly
2 participants