Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

emailjs can't send messages >12kB, algorithms don't scale #40

Closed
jart opened this Issue · 7 comments

3 participants

@jart

emailjs can't send messages larger than 12kb because it allocates a fixed buffer of that size which cannot be changed.

If you try to increase the buffer size, the process will crash with a stack overflow error because 'output_base64()' recurses for every 76 bytes. Seriously? If there wasn't a 12kb limitation, that would be a huge security risk.

eleith, I really appreciate all the hard work you've done making emailjs, but I urge you to consider deprecating this project in favor of nodemailer which doesn't have these problems. I didn't know about nodemailer until now because your project was the first thing that showed up for me when I googled for a node email library. I feel very concerned for the people using this library in production who didn't test it as thoroughly as I have.

@eleith
Owner

currently, emailjs successfully sends large attachments (the tests include a tar.gz file and a pdf file) so i haven't run into this issue before and i am curious and more than happy to look into it and improve it.

could you outline a test i can replicate which causes this failure? i believe the line you are referring to is this: https://github.com/eleith/emailjs/blob/master/smtp/message.js#L291

according to that line, isn't this recursing for every 76616 bytes and not just 76?

lastly, could you further explain the security risk you referred to? i think that would help me better understand the issue you are reporting.

your suggestion to others that they try out nodemailer is valid and i will continue updating and improving emailjs, which works reasonably well for many use cases.

i like choice and i don't see bugs/mistakes in one project as a reason to deprecate in favor of another.

but despite our disagreement, i would appreciate your help in getting to the bottom of this one!

@jart

I realized after I reported this that it works ONLY when you specify an attachment by its filename on the disk. If you pass it any large string values, either in the msg.text field or as an attachment, it'll crash.

Here's the code I used:

fs = require "fs"
util = require "util"
emailjs = require "emailjs"
wap = fs.readFileSync("/home/jart/Desktop/war-and-peace.txt").toString()
# wap = wap.toString()[..12000]
server = emailjs.server.connect
  host: "mail.blah.com"
  user: "foo"
  password: "bar"
  ssl: true
msg =
  to: "jtunney@lobstertech.com"
  from: "jtunney@lobstertech.com"
  subject: "War and Peace"
  text: wap
server.send msg, (err, message) ->
  console.log "email() failed: #{util.inspect err}" if err

email() failed: { message: 'internal buffer got too large to handle!' }

And it seems the process DOES indeed crash with a stack overflow if you do this:

msg =
  to: "jtunney@lobstertech.com"
  from: "jtunney@lobstertech.com"
  subject: "War and Peace"
  text: "small text"
  attachment: [{
    alternative: true
    data: "<html><body><pre>#{wap}</pre></body></html>"
  }]

buffer.js:0
(function (exports, require, module, __filename, __dirname) { // Copyright Joy
^
RangeError: Maximum call stack size exceeded

This means some of your users are in big trouble. Let's say you've got a node.js webserver with a Contact Us page. I could put a really large message in that form and crash your webserver.

And the code I was talking about was here: https://github.com/eleith/emailjs/blob/master/smtp/message.js#L384

I don't think offering 'choices' is a valid argument for not wanting to hop on board with the nodemailer project. People don't want choices when it comes to a node.js mailer, they don't want to evaluate the merits of each project and make an informed decision, they want something that works and is easy to use so they can get back to writing their app.

They also want their app to work reliably and not have to go digging through logs to find out why messages fail to send intermittently. You have a responsibility to the developers using this project, and the people who use their projects, to make sure that what you're putting out is quality engineering. Sometimes the best way to shoulder that responsibility is by gracefully stepping down when someone else has invested the extra time and effort into building a better product. We should be cooperating whenever possible because needless competition hurts communities. It's sad that you're doing this because nodemailer could be a lot more awesome if you worked for them.

If you choose to keep this project alive, then I urge you to carve out a niche that clearly differentiates it from nodemailer.

@eleith
Owner

thanks for use case and sharing your opinions. i'll be sure to add this as a test case as well, to ensure this works properly from now on.

@jart

Regardless of whether or not you take my advice, I commend you for being so on top of this issue and writing tests to make sure this problem doesn't happen again. It's incredibly rare that I see open source developers respond and start working on a fix in a matter of hours.

@eleith
Owner

thanks again. i have tests for both large message and data strings.

and now...time for a lobster:

                 ,.---.
                ,,,,     /    _ `.
                 \\\\   /      \  )
                  |||| /\/``-.__\/
                  ::::/\/_
  {{`-.__.-'(`(^^(^^^(^ 9 `.========='
 {{{{{{ { ( ( (  (   (-----:=
  {{.-'~~'-.(,(,,(,,,(__6_.'=========.
                  ::::\/\
                  |||| \/\  ,-'/\
   jgs           ////   \ `` _/  )
                ''''     \  `   /
                          `---''
@eleith eleith closed this
@jart

D'awww <3

@andris9

As the author of Nodemailer, I wouldn't suggest Eleith to deprecate Emailjs. I think it's a good project (I have borrowed some ideas from it now and then :snowman:) and I believe that users should have a choice - Nodejitsu guys already deprecated their version of a SMTP client. Node.js is not Python where they usually have one standard module for one task and if it doesn't cut it, you're out of luck and have to implement your own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.