EC-Nordbund / denomailer

A SMTP-Client implementation for deno (to send mails!)
https://deno.land/x/denomailer
MIT License
50 stars 16 forks source link

Crash when attempting connection #9

Closed Schotsl closed 2 years ago

Schotsl commented 2 years ago

Describe the bug When I try to connect to my mail server I get this error:

error: Uncaught (in promise) Error: 250: -srv8.flevohost.nl Hello mail.corvusconsultancy.nl [84.241.192.141]
      throw new Error(msg || cmd.code + ": " + cmd.args);
            ^
    at SmtpClient.assertCode (https://deno.land/x/denomailer@0.12.0/smtp.ts:391:13)
    at SmtpClient.#connect (https://deno.land/x/denomailer@0.12.0/smtp.ts:376:12)
    at async SmtpClient.connectTLS (https://deno.land/x/denomailer@0.12.0/smtp.ts:63:5)
    at async file:///usr/local/var/www/Uberdeno/Uberdeno/services/smtpClient.ts:21:1

To Reproduce I've valid my credentials with the MacOS mail client and everything works fine, If anyone needs credentials to test you can contact me at sjorsvanholst@gmail.com

import { SmtpClient } from "https://deno.land/x/denomailer@0.12.0/mod.ts";

const smtpClient = new SmtpClient();
await smtpClient.connectTLS({
  hostname: Deno.env.get("IMAP_HOSTNAME")!,
  username: Deno.env.get("IMAP_USERNAME")!,
  password: Deno.env.get("IMAP_PASSWORD")!,
  port: +Deno.env.get("IMAP_PORT")!,
});

Expected behavior I expected denomailer to connect, and not crash ;)

Logs

deno 1.19.3
220-srv8.flevohost.nl ESMTP Exim 4.94.2 #2 Thu, 07 Apr 2022 16:35:17 +0200 
┌───────┬─────────────────────────────┐
│ (idx) │ Values                      │
├───────┼─────────────────────────────┤
│     0 │ "EHLO"                      │
│     1 │ "mail.corvusconsultancy.nl" │
└───────┴─────────────────────────────┘
220-We do not authorize the use of this system to transport unsolicited, 
220 and/or bulk e-mail.
┌───────┬─────────┐
│ (idx) │ Values  │
├───────┼─────────┤
│     0 │ "AUTH"  │
│     1 │ "LOGIN" │
└───────┴─────────┘
250-srv8.flevohost.nl Hello mail.corvusconsultancy.nl [84.241.192.141]

Additional context I've cloned the repo and the crash happens at these lines:

204: this.assertCode(await this.readCmd(), CommandCode.BEGIN_DATA);
378: this.assertCode(thing, 334);
382: this.assertCode(await this.readCmd(), 334);
385: this.assertCode(await this.readCmd(), CommandCode.AUTHO_SUCCESS);

If I replace every code (so CommandCode.BEGIN_DATA, 334 OR CommandCode.AUTHO_SUCCESS) with 250 (orCommandCODE.OK) it works just fine (I've checked, they are actually send).

So I think my mail server provider is just lazy and returns OK for every success? This issue could be solved by check for 334 OR 250 etc.. I didn't make a pull request since I know almost nothing about mail protocols and I might be wrong?

mathe42 commented 2 years ago

I think this is not as SMTP is specified. But I think it is ok to be "nice" and don't error.

https://github.com/EC-Nordbund/denomailer/blob/master/smtp.ts#L375-L382

Do we need to change Lines 376, 379 and 382 or only some of them?

mathe42 commented 2 years ago

Would be nice if you could create a PR and replace WHERE NEEDED! the this.assert... line with:

const { code, args } = (await this.readCmd())!

// Some SMTP-Server responde with a 250. We will accept that.
if(code !== 334 && code !== 250) {
  throw new Error(code + ': ' + args)
}
Schotsl commented 2 years ago

Ahaha, I ended up switching to Mailgun! Altough I've still created a PR to fix my previous issue and tested it (#10), I think it should be good for a merge? :)

mathe42 commented 2 years ago

I closed this for now but add a comment to the readme. I'm not sure if I want to allow non speced behavior if we don't realy need it...

If someone needs that change I will merge it but not for now...