-
Notifications
You must be signed in to change notification settings - Fork 51
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
Split body of mails not respecting RFC2822 #49
base: master
Are you sure you want to change the base?
Conversation
If we split some lines, I think we should split all lines. For simplicity's sake, I'd split them strictly at byte 990 or so... |
So you want me to remove the discovery of the space and always split long lines? |
done |
if (!had_headers) { | ||
if (linelen > 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we also break the header lines? that would be complicated though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I followed the discussion on the ticket and decided to make a first step splitting the body
@@ -425,36 +442,44 @@ readmail(struct queue *queue, int nodot, int recp_from_header) | |||
} | |||
} | |||
|
|||
if (strcmp(line, "\n") == 0 && !had_headers) { | |||
if (line[0] == '\n' && !had_headers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change?
} | ||
if (!nodot && linelen == 2 && line[0] == '.') | ||
break; | ||
if (!nocopy) { | ||
if (fwrite(line, strlen(line), 1, queue->mailf) != 1) | ||
return (-1); | ||
if (newline[0] != '\0') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this conditional - it is very implicit.
For mails where the body does not respect RFC2822, split at the arbitrary length of 990 This should address corecode#18
I think I addressed all the comments |
Ping? |
We ran into unexpected issues that this would fix when trying to switch to dma. For instance, cron jobs would fail if there was a long line on stdout/stderr as it gets piped to sendmail. |
Just wanted to comment that while this PR seems to still apply cleanly, when we deployed it we instantly had complaints about some automated emails getting rejected with a corrupted queue error, so failing this check: https://github.com/corecode/dma/blob/master/net.c#L582 I sadly never got a copy of one in order to replicate the issue on-demand, so we just reverted the patch for now. If I find a way to replicate the issue, I'll dig some more into this. |
Hi @jailbird777 Regarding your comment on on 20 Feb 2020, can you try to replicate your issue with instructions on FreeBSD PR266629 ? And, if running FreeBSD, you can also try corresponding patches. |
For mails where the body does not respect RFC2822, try to split by words
finding the last space before 1000's character
If no spaces are found then consider the mail to be malformed anyway
This should address #18