Skip to content

Split body of mails not respecting RFC2822#49

Open
bapt wants to merge 1 commit into
corecode:masterfrom
bapt:splitline
Open

Split body of mails not respecting RFC2822#49
bapt wants to merge 1 commit into
corecode:masterfrom
bapt:splitline

Conversation

@bapt

@bapt bapt commented Oct 20, 2016

Copy link
Copy Markdown
Collaborator

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

@corecode

Copy link
Copy Markdown
Owner

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...

@bapt

bapt commented Oct 20, 2016

Copy link
Copy Markdown
Collaborator Author

So you want me to remove the discovery of the space and always split long lines?

@bapt

bapt commented Oct 20, 2016

Copy link
Copy Markdown
Collaborator Author

done

Comment thread mail.c
had_last_line = 1;
}
if (!had_headers) {
if (linelen > 1000)

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Collaborator Author

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

Comment thread mail.c Outdated
}

if (strcmp(line, "\n") == 0 && !had_headers) {
if (line[0] == '\n' && !had_headers) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Comment thread mail.c Outdated
if (!nocopy) {
if (fwrite(line, strlen(line), 1, queue->mailf) != 1)
return (-1);
if (newline[0] != '\0') {

Copy link
Copy Markdown
Owner

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
@bapt

bapt commented Oct 31, 2016

Copy link
Copy Markdown
Collaborator Author

I think I addressed all the comments

@emaste

emaste commented Feb 22, 2017

Copy link
Copy Markdown
Collaborator

Ping?

@kev009

kev009 commented Aug 26, 2017

Copy link
Copy Markdown

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.

@jailbird777

Copy link
Copy Markdown
Contributor

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.

@mslehto

mslehto commented Sep 29, 2022

Copy link
Copy Markdown

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.

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.

6 participants