nodemailer / smtp-server

Create custom SMTP servers on the fly
Other
846 stars 146 forks source link

File attachments not taken into consideration for file size limit? #90

Closed niftylettuce closed 6 years ago

niftylettuce commented 6 years ago

This might be a core bug, as I wrote a basic test and a file created that's over the limit sent just fine.

It didn't throw an error, and it seems like we're not adding to the this.dataBytes for attachments.

this._dataStream.sizeExceeded = this.dataBytes > this._maxBytes;
andris9 commented 6 years ago

It’s a soft fail, you get an indicator that the file size was too big but nothing is rejected automatically

niftylettuce commented 6 years ago

It doesn't fail though at all, also stream if (stream.sizeExceeded) { is always false.

niftylettuce commented 6 years ago

In other words I don't get any indicators, no error messages, nothing

niftylettuce commented 6 years ago

Also, I tried it without even using attachments, it still doesn't seem to work. The byte reading seems to be inaccurate... hmm

andris9 commented 6 years ago

Could you try with this example script to see if it fails if message is larger than 25 bytes:

/* eslint-env es6 */

'use strict';

const SMTPServer = require('smtp-server').SMTPServer;

let server = new SMTPServer({
    logger: true,
    disabledCommands: ['STARTTLS', 'AUTH'],
    size: 25
});

server.onData = function(stream, session, callback) {
    let chunks = [];
    let chunklen = 0;

    stream.on('data', chunk => {
        chunks.push(chunk);
        chunklen += chunk.length;
    });

    stream.on('end', () => {
        let err;

        if (stream.sizeExceeded) {
            err = new Error('Maximum allowed message size 25B exceeded');
            err.responseCode = 552;
            return callback(err);
        }

        process.stdout.write(Buffer.concat(chunks, chunklen));

        callback();
    });
};

server.listen(5525);
niftylettuce commented 6 years ago

That's not a really complete example script :(

On Sun, Oct 1, 2017 at 4:41 AM, Andris Reinman notifications@github.com wrote:

Could you try with this example script to see if it fails if message is larger than 25 bytes:

/ eslint-env es6 / 'use strict'; const SMTPServer = require('smtp-server').SMTPServer; let server = new SMTPServer({ logger: true, disabledCommands: ['STARTTLS', 'AUTH'], size: 25 }); server.onData = function(stream, session, callback) { let chunks = []; let chunklen = 0;

stream.on('data', chunk => {
    chunks.push(chunk);
    chunklen += chunk.length;
});

stream.on('end', () => {
    let err;

    if (stream.sizeExceeded) {
        err = new Error('Maximum allowed message size 1kB exceeded');
        err.statusCode = 552;
        return callback(err);
    }

    process.stdout.write(Buffer.concat(chunks, chunklen));

    callback();
});

}; server.listen(5525);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nodemailer/smtp-server/issues/90#issuecomment-333362154, or mute the thread https://github.com/notifications/unsubscribe-auth/AAf7haJP0pO4YpaxgPPYUc16SFeb3ig9ks5sn1AtgaJpZM4Pp0pN .

-- @niftylettuce

andris9 commented 6 years ago

and btw you can only check the byte size after the stream has been processed. This value is not known beforehand as the client has not yet sent the message.

andris9 commented 6 years ago

run the script and try to send mail to port 5525. if the message is bigger than 25 bytes, the message should fail

andris9 commented 6 years ago
$ telnet localhost 5525
Trying ::1...
Connected to localhost.
Escape character is '^]'.
220 Macintosh.lan ESMTP
ehlo foo
250-Macintosh.lan Nice to meet you, [::1]
250-PIPELINING
250-8BITMIME
250-SMTPUTF8
250 SIZE 25
mail from:<test@example>
250 Accepted
rcpt to:<test@example>
250 Accepted
data
354 End data with <CR><LF>.<CR><LF>
subject: test

tere
tere
tere, vana kere
kuidas laheb
voi mid aiganes
.
552 Maximum allowed message size 25B exceeded
andris9 commented 6 years ago

Here's a longer version, first message is passed (shorter than 25 bytes), second one fails:

$ telnet localhost 5525
Trying ::1...
Connected to localhost.
Escape character is '^]'.
220 Macintosh.lan ESMTP
ehlo foo
250-Macintosh.lan Nice to meet you, [::1]
250-PIPELINING
250-8BITMIME
250-SMTPUTF8
250 SIZE 25
mail from:<test@example>
250 Accepted
rcpt to:<test@example>
250 Accepted
data
354 End data with <CR><LF>.<CR><LF>
subject: short

hello
.
250 OK: message queued
mail from:<test@example.com>
250 Accepted
rcpt to:<test@example.com>
250 Accepted
data
354 End data with <CR><LF>.<CR><LF>
subject: longer message

this is going to fail
.
552 Maximum allowed message size 25B exceeded
andris9 commented 6 years ago

So, stream.sizeExceeded is a soft indicator that you can check only after the message is processed and not immediately in the start of the onData handler (client has not yet sent anything at that point so there's nothing to check).

niftylettuce commented 6 years ago

will try to fix my code, I think I was missing something

niftylettuce commented 6 years ago

@andris9 I just tested it again with attachments and it does not work.

niftylettuce commented 6 years ago

Output:

databytes 35873301
this._maxBytes 26214400

The stream never emits an end or close event when piping along it seems, and that's the culprit I believe.

niftylettuce commented 6 years ago

I think I've figured it out, give me a bit and I'll document what I did.

andris9 commented 6 years ago

Can you create a minimal test case that I could try out. I don’t see how the attachments could affect anything as smtp-server treats the entire email as a blackbox binary data stream

niftylettuce commented 6 years ago

It's totally working fine, I just had some stuff backwards :)

On Sun, Oct 1, 2017 at 12:40 PM, Andris Reinman notifications@github.com wrote:

Can you create a minimal test case that I could try out. I don’t see how the attachments could affect anything as smtp-server treats the entire email as a blackbox binary data stream

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/nodemailer/smtp-server/issues/90#issuecomment-333389179, or mute the thread https://github.com/notifications/unsubscribe-auth/AAf7hS-Irirl6xOTP3c6EE9qIHu_QeFwks5sn8B_gaJpZM4Pp0pN .

-- @niftylettuce