Uncatchable attachment error.

[geoidesic]

The error:

node:internal/streams/writable:314
      throw new ERR_INVALID_ARG_TYPE(
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of Object
    at new NodeError (node:internal/errors:387:5)
    at _write (node:internal/streams/writable:314:13)
    at PassThrough.Writable.end (node:internal/streams/writable:611:17)
    at Immediate.<anonymous> (/Users/me/email-server/node_modules/nodemailer/lib/mime-node/index.js:977:46)
    at processImmediate (node:internal/timers:466:21) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Methods from my EmailController class:

  getNodeMailer() {
    return require("nodemailer")
  }
  getTransporter() {
    if(!this.transporter) {
      const config = this.getConfig();
      //- create reusable transporter object using the default SMTP transport
      this.transporter = this.getNodeMailer().createTransport({
        host: config.smtp.host,
        port: config.smtp.port,
        secure: config.smtp.secure, // true for 465, false for other ports
        tls: config.smtp.tls,
        auth: {
          user: config.smtp.sendFrom,
          pass: config.smtp.password,
        },
        // debug: true,
        // logger: true
      });
    }
    return this.transporter;
  }
  async sendMail(msg) {
    
    try {
      const transporter = this.getTransporter();
      await transporter.sendMail(msg);
    } catch (error) {
      this.processSendError(msg, error)
    }
}

sendMail will catch normal errors (e.g. misconfigured transporter) but if an attachment causes an error (e.g. an attachment with no content) then it does not get caught. By "no content" I mean e.g. content: null; for which typeof would report object.

Conversation from #NodeMailer Discord Server channel:

geoidesic — Today at 7:41 PM
Well it's an attachment with no content. That's causing the error.
andrisreinman — Today at 7:42 PM
What is the type of that value? Is it an empty string or a buffer? Or something else?
geoidesic — Today at 7:42 PM
null type
andrisreinman — Today at 7:43 PM
Like that: content: null?
geoidesic — Today at 7:43 PM
Yup
If I do typeof it outputs object
andrisreinman — Today at 7:45 PM
Could you create a pull request with a failing test where you try to send an email with such an attachment and it fails? Once there is a test case, I could look into it and try to fix it.
Until then, try to send attachments that are either strings, buffers or streams. Nothing else is allowed. Nodemailer should reject such an email without trying to send it.
If it does bot reject that email, it’s a bug in input validation

I'm creating this ticket as a placeholder against which the PR can be created.

[github-actions]

This issue is stale because it has been open for 30 days with no activity.

[github-actions]

This issue was closed because it has been inactive for 14 days since being marked as stale.

[bharathpanchak]

Any fix given for this issue?

[andris9]

I was not able to verify the initial claim. Setting attachment content to null for a Nodemailer attachment did not trigger an uncaught error.

[geoidesic]

@andris9 How is that possible? The failing test was submitted and merged? That's all that was requested in order to progress this, it was done, no follow-up since then till yours an hour ago.

[andris9]

This test did something else, so I removed it. Adding an attachment to Nodemailer with content:null did nothing. Providing that attachment to Nodemailer's sendMail resulted in a normal empty attachment without throwing as was the initial claim.

----_NmP-cde822598c339221-Part_1
Content-Type: text/plain; name=test.txt
X-Test-1: 12345
X-Test-2: =?UTF-8?B?w5XDhMOWw5w=?=
X-Test-3: foo
X-Test-3: bar
Content-Disposition: attachment; filename=test.txt


----_NmP-cde822598c339221-Part_1--

[geoidesic]

Could you please try add an attachment that is 0kb in size? That's what caused the original issue for me.

[andris9]

Tested with content: Buffer.alloc(0) - no error thrown. So I would need a test that runs the following and causes an uncaught error:

await transporter.sendMail(failingPayload)

The initially provided test did not do it.

[geoidesic]

I don't know how to do that. What I can tell you is that if you use NodeMailer and you try to attach a 0kb attachment, then you get the error I posted. If you can't work with that, then I guess people will just have to live with this deficiency.

[andris9]

I tried all kinds of payloads to verify the initial claim but never managed to get an uncaught error. Without having a proper test case that I can use to verify this bug, I can only assume that the issue is in your own code, not in Nodemailer.

[geoidesic]

I don't understand what was wrong with the test I submitted. As far as I can see it does exactly what you asked for. Perhaps you could explain what's wrong with it in terms more specific than it does something else.

[andris9]

The provided test case was something else. It set a null payload and expected that Nodemailer throws on this. There is no reason to throw, so the test case was not valid. What I need is a test case, where you call the sendMail method with a specially crafted payload that triggers an uncaught error.

[geoidesic]

Why is there no reason to throw?

[andris9]

Empty attachment content is totally valid. It should only throw if the payload causes uncaught errors, and to prevent this, the payload should be handled in a way where the uncaught error is caught and returned. This test did not cause any uncaught errors and so there was no error to return.

This does not mean that the bug does not exist. It very well might exist, but to handle it, I need a test case with transporter.sendMail(failingPayload) that throws an uncaught error. Without such a test case, I can only state that there is no bug.

[geoidesic]

Ok well I'm no longer on that contract so I can't help with that. All I know is that some users were accidentally generating 0kb file attachments and that when sending those the error I posted happened. As far as I could tell, the file was interrogated to produce the input to NodeMailer that I showed and when I ran that code it generated an uncatchable error which caused the entire app to break.

To fix it I added in code before the NodeMailer call so that it would not attempt to process files with null content. It seems like at least one other person above, @bharathpanchak has experienced this, so perhaps they can help further.

[andris9]

Sure, in any case, as soon as I get a reproducible test case that causes an uncaught error, I can fix the handling for it to catch it and return it as a regular error. Right now, there is nothing to fix as I don't have such a payload.

[bharathpanchak]

@andris9 Here are my inputs.

let params = {
  from: { name: 'random', address: 'ramdom@random.com' },
  to: [ 'ramdom@random.com', 'ramdom@random.com' ],
  subject: 'random',
  html: [ '', '' ],
  attachments: [
    {
      filename: 'random.txt',
      content: {},
      contentType: 'text/plain',
    }
  ]
}

return transporter.sendMail(params);

My case is when I used type of content in attachment as object/array or type of html body as object/array(other than your specified formats string or an instance of Buffer or Uint8Array) Nodemailer is throwing an uncatchable error which is crashing my server.

[bharathpanchak]

If you have a question why my inputs are like array or object, in my project I will take the inputs from users, they can give any format they want I can't restrict that but I want to show an error that your input format is wrong. I can add a filtering code before sending mail if there is no way you can handle this.

[andris9]

Ok, now I see it. You are providing something that is a completely wrong data type. Attachments can be empty, strings, or Buffers.

This is not attachment specific, and can only be fixed by adding proper input validation for Nodemailer for all input values, as you could probably trigger something similar if you set a Date object as a header key or use any other strange values instead of expected ones. I have no plans for generic input validation for now, so you need to fix your input types in your code. Make sure you only use values that match the documented types.