Fossil Forum

Email truncation
Login

Email truncation

Email truncation

(1) By Warren Young (wyoung) on 2018-08-13 18:14:23 [link] [source]

At least twice now, Fossil has delivered truncated email notification messages to me.

Rather than create noisy test posts here to see it, this command line test will show it:

  $ fossil alerts test-message you@example.com --body README.md --subject Test

You just need a README.md file of sufficient size.

I've seen it from both fossil-scm.org and from my own public repositories, so it isn't specific to one email server or one Fossil email sending mechanism. (I use pipe-to-sendmail on my public Fossil servers.)

I've looked at the code, and I don't see any obvious fixed-size buffers to explain it. My best guess is that fread() might be giving a short read down in the blob-from-file code, but I haven't verified that yet. Certainly POSIX read(2) can do this, but maybe fread() on most platforms is prevented from returning early except in case of EOF or file I/O error?

(2) By sean (jungleboogie) on 2018-08-13 19:07:26 in reply to 1 [link] [source]

I think drh recently discussed this with another poster. It was because the subsequent email was edited. Is that the case this time?

(3) By Warren Young (wyoung) on 2018-08-13 19:16:36 in reply to 2 [link] [source]

How would that explain the README.md test?

But no, I didn't edit the post in question. If it helps, the email I got was truncated at "to get them unconfused, too". Everything from the period on is missing.

On looking at it, I see that a short fread() can't explain it, because that email is also missing the signature section, which means it's being truncated further down the line.

Are you signed up for immediate email alerts? Did you get the whole body text for that other post's notification?

(4) By ravbc on 2018-08-13 22:42:05 in reply to 3 [link] [source]

I've got truncated notification, and I think it's cut in the same place (on the doublequote, not on dot, after the sentence you've mentioned - in fact I don't have any doublequote marks in that notification).

(5) By Dingyuan Wang (gumblex) on 2018-08-14 02:39:01 in reply to 3 [link] [source]

Truncated on the same location.

Truncation test:

fusion, reply to them explaining the issue to get them unconfused, too. Document the behavior

(6) By Dingyuan Wang (gumblex) on 2018-08-14 03:07:09 in reply to 1 [link] [source]

As seen in https://fossil-scm.org/forum/forumpost/dd647287ef , By looking at the truncated email, I think this issue may have something to do with the smtp spec:

SMTP indicates the end of the mail data by sending a line containing only a "." (period or full stop).

That email's source code says:

1. Train our way out of it. That is, every time someone has such a con=
fusion, reply to them explaining the issue to get them unconfused, too=

[there would be a single . on the next line]

This may be avoided by adding this to the quoting code.

(7) By Warren Young (wyoung) on 2018-08-14 04:00:07 in reply to 6 [link] [source]

I believe you've got it! I've checked in a fix. Please check it for sanity. I think it's correct, and a bit of playing around here with fossil test-smtp-senddata seems to confirm it, but it involves pointer arithmetic, so it could use an extra bit of scrutiny.

(8) By Dingyuan Wang (gumblex) on 2018-08-14 05:05:34 in reply to 7 [link] [source]

But I think that's not the case. The original smtp.c is good enough to be not truncated. The fix should be in email.c append_quoted function, provided that fossil-scm.org uses an external smtp client, and other sending methods may also have this problem.

.

test again.

(9) By Warren Young (wyoung) on 2018-08-14 05:44:38 in reply to 8 [link] [source]

The original smtp.c is good enough to be not truncated.

My checkin to smtp.c is about more than just truncation. See section 4.5.2 in the RFC. The prior code would damage any line beginning with a dot and followed by any other character. The leading dot needs to be duplicated in that case, too.

The fix should be in email.c append_quoted function

Ugh... It looks like you're right, which means we've got a leaky abstraction here. All details specific to SMTP should be in smtp.c. email.c shouldn't need to know anything about RFC 821 or its successors; it should be concerned only with the RFC 822 level.

My fix doesn't affect either my own Fossil projects or the Fossil forum because they use methods 1 and 3 from alerts.md, respectively, which means they never use the function I just fixed. That's only used by method 4, SMTP relay.

I don't want to put this logic in email.c. Not only does it violate a layer boundary, it would mean the fix I just committed would cause re-doubled leading dots in the case of method 4.

I think email.c should pass the nearly-complete Blob down into smtp.c to be SMTP-quoted in the cases that require it, then continue with the current logic.

Does that work for you?

test again.

Your inline tests here in this forum won't show any differences until new code is deployed on fossil-scm.org. I can't do that. I only have checkin rights on the repo, not any admin ability on the server.

(10) By Warren Young (wyoung) on 2018-08-14 06:01:49 in reply to 9 [link] [source]

I was right to resist the layer boundary violation. It turns out that there's a much simpler and more correct fix: add -i to the sendmail command for methods 1 and 3.

I've just tested it on one of my public Fossil servers, and it does work, so I checked the change into Fossil itself.

The smtp.c fix remains: the leading dot does not mean exactly the same thing in SMTP and to sendmail(1). They need to be handled differently.

(11) By Dingyuan Wang (gumblex) on 2018-08-14 06:13:30 in reply to 9 [link] [source]

Your inline tests here in this forum won't show any differences until new code is deployed on fossil-scm.org. I can't do that. I only have checkin rights on the repo, not any admin ability on the server.

I think the fossil code is auto compiled and deployed periodically.

For the layer boundary violation, that's drh who should fix the sendmail setup.

(12) By sean (jungleboogie) on 2018-08-14 06:19:37 in reply to 11 [link] [source]

> I think the fossil code is auto compiled and deployed periodically.

I've never seen the code auto compiled on the hosted fossil-scm.org website. drh does that and updates the binary. I believe he's the only one with access to do so. We'll know when the repo has been updated once the hash at the bottom of all the fossil pages matches the hash of Warren's check-in.

(13) By anonymous on 2018-08-18 15:18:56 in reply to 9 [link] [source]

This problem doesn't exist if email submission is done via stdin instead of SMTP.  Does the new forum feature allow an administrator to simply fork() the preferred MTA and feet it appropriate arguments and the message on stdin to avoid SMTP handling?

(14.1) By Warren Young (wyoung) on 2018-08-18 20:04:05 edited from 14.0 in reply to 13 [link] [source]

This problem doesn't exist if email submission is done via stdin instead of SMTP.

Incorrect. Both the SMTP protocol and the most common implementations of /usr/sbin/sendmail treat a lone dot on a line as "end of message". Say man sendmail on your system. The linked man page explains this right up at the top.

In SMTP, it's baked into the protocol, so I had to change Fossil's SMTP client implementation to account for it.

That protocol rule is actually not the same as the rule implemented by sendmail, because it affects all leading dots on a line, not just a lone dot. Thus, it had to happen down at the smtp.c level, not up at the level common to both the SMTP and sendmail cases.

sendmail only cares about lone dots on a line, but it ignores them if you pass -i. Thus my other fix, which adds -i to all sendmail -t commands hardcoded into Fossil.

We could still get into trouble if there is a sendmail implementation that doesn't understand -i, but the implementations in Sendmail, Postfix, and Exim all understand it. That probably covers 99%+ of all non-Windows boxes where Fossil is installed. There are outliers like OpenSMTPD, but its sendmail implementation doesn't use the dot protocol at all, and so doesn't need -i.

In such cases, you just change the Fossil default, dropping the -i flag. Many in this situation will also need to edit the command anyway, so it's no extra burden.

simply fork() the preferred MTA

The current implementation offers five different methods for sending mail. The first — pipe — is based on popen(), which does as you suggest, forking off the child and feeding it input via stdin. And for this, we must give the -i flag to avoid this truncation problem with the most common sendmail implementations.

(15) By Warren Young (wyoung) on 2018-09-13 15:28:06 in reply to 14.1 [source]

drh, are you using my patched tools/email-sender.tcl script on fossil-scm.org? I've seen two truncated email notifications since doing all of the above.

The most recent was for my Optimizing Fossil web responses post.

I'm hoping you just haven't gotten around to it yet, since the alternative is that sendmail -i isn't the fix after all, or at least, it isn't a sufficient fix.