Period at the beginning of line of encoded content

level-xx commented 1 year ago

First of all: Many thanks for this great work, it helped me a lot!

Describe the bug

After encoding and line breaking it may occur that a line starts with a period. This may lead to problems with the SMTP server which may take the period as the »end of message« and treat the rest of the message as commands, resulting in a mess, of course :)

Content lines starting with a period are non-compliant with RFC5321 which says in section 4.5.2 that if the first character of the line is a period, one additional period has to be inserted at the beginning of the line.

To Reproduce

A message with a line consisting of 75 dots at the beginning will do the trick -- at least on my SMTP server.

Logs

Deno will yield a bad resource error when the SMTP server closes the connection.

error: Uncaught BadResource: Bad resource ID
    at async Object.write (internal:ext/web/06_streams.js:902:11)

Proposed solution

Check for lines starting with a period in denomailer/config/mail/encoding.ts and prepend a period if necessary. At line 46 add something like

    if (old.startsWith('.')) {
      old = "." + old;
    }

This is how I fixed it, although I am not sure if this breaks something else.

I can provide a PR if so desired.

mathe42 commented 1 year ago

Oh your right. Better solution would be at https://github.com/EC-Nordbund/denomailer/blob/main/config/mail/encoding.ts#L13 to add a replaceAll "." => "=2E". Could you test that and provide a PR. im currently quite busy...

level-xx commented 1 year ago

I'll do that, np. Just a couple of thoughts:

I think line 13 is a no-op. The value is never assigned. However, since code 61 is excluded in line 26, the equals signs are encoded downstream, lines 32-36. If it's OK, I remove line 13 in the PR.
Replacing the dots with a data = data.replaceAll(".", "=2E"); near line 13 doesn't work, as the equals signs will be encoded downstream, yielding "=3d2E". Adding && code !== 46 to line 26 works. It is a bit of an overkill, though, as it is not strictly necessary to encode all the periods. They cause problems only on line starts.

If it's OK this way, I'll submit the PR. There is a little issue with the line breaking in lines 45-67 as well, as »equals encoded« chars may be broken up. I'll try to look into that soon, too, and then give the period thingy another thought.

mathe42 commented 1 year ago

Yes your right...

In your PR could you fix also the "="?

I would encode all . as this makes the code more readable etc.

level-xx commented 1 year ago

OK, will do that. By »fix also the "="« remove line 13? For the 45-67 thing I need more time, but it's not critical, as I believe it won't kill the connection.

mathe42 commented 1 year ago

Yes remove the = line..

And I missed that for 45-67 I think I tested the splitting with the = encoding and even adding a line break is fine (as they are removed before parsing the rest)

EC-Nordbund / denomailer