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

Period at the beginning of line of encoded content #67

Open
level-xx opened this issue Feb 20, 2023 · 5 comments
Open

Period at the beginning of line of encoded content #67

level-xx opened this issue Feb 20, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@level-xx
Copy link
Contributor

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.

@level-xx level-xx added bug Something isn't working triage labels Feb 20, 2023
@mathe42
Copy link
Member

mathe42 commented Feb 20, 2023

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

@mathe42 mathe42 removed the triage label Feb 20, 2023
@level-xx
Copy link
Contributor Author

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
Copy link
Member

mathe42 commented Feb 21, 2023

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
Copy link
Contributor Author

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
Copy link
Member

mathe42 commented Feb 21, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants