Bug: `headerPos` set incorrectly due to varying byte length

[titanism]

Per https://github.com/nodemailer/wildduck/blob/master/lib/message-splitter.js the chunk might look like:

Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

H

But the raw email sent across actually was:

Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

Note that the i, gets chopped off, and the resulting body and headers joined is:

Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

i,

e.g. the H gets chopped off (first letter after header line break)

The trailing line break is the culprit. Curious if you think we should dummy-proof this @andris9 (e.g. for developers in case they accidentally send email with a trailing line break)?

I think it'd be nice to dummy-proof against this, perhaps using something similar to this code:

// <https://github.com/sindresorhus/strip-final-newline/blob/a1bfe78e3a3de2f73ed3a7600932d7cc952732b4/index.js#L18-26>
const LF = '\n';
const LF_BINARY = LF.codePointAt(0);
const CR = '\r';
const CR_BINARY = CR.codePointAt(0);

const stripFinalNewlineBinary = (input) =>
  input.at(-1) === LF_BINARY
    ? input.subarray(0, input.at(-2) === CR_BINARY ? -2 : -1)
    : input;

Then calling stripFinalNewlineBinary perhaps or just looking for it if headers were found. A rewrite might be needed.

[andris9]

I tried the following, seems to work fine. Is there something missing?

'use strict';

const { PassThrough } = require('stream');
const Splitter = require('mailsplit/lib/message-splitter');
const Joiner = require('mailsplit/lib/message-joiner');

let splitter = new Splitter();
let joiner = new Joiner();

const message = `Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,
`.replace(/\r?\n/g, '\r\n');

const stream = new PassThrough();

// pipe all streams together
stream.pipe(splitter).pipe(joiner).pipe(process.stdout);

stream.end(message);

Output:

Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

[titanism]

This is with Wildduck message splitter, not mailsplit's @andris9

[titanism]

Also definitely try it without the .replace(/\r?\n/g, '\r\n');

[titanism]

I'm going to try to get a reproducible case with Wildduck here in the next hour or so.

[titanism]

I'm not able to reproduce this so it must be something specific to my application code. Taking a look now.

[titanism]

Perhaps this is a bug with smtp-server instead.

[titanism]

I have not been able to reproduce with smtp-server yet, only via tests. Going to try to see how I got this to reproduce in the first place now.

For example, this works OK:

const process = require('node:process');
const { Buffer } = require('node:buffer');

const Splitter = require('mailsplit/lib/message-splitter');

const getStream = require('get-stream');
const mxConnect = require('mx-connect');
const nodemailer = require('nodemailer');
const pify = require('pify');
const { SMTPServer } = require('smtp-server');
const { listen } = require('async-listen');

const asyncMxConnect = pify(mxConnect);

const message = `Message-ID: <ui0f6sedxw@blanditiis.io>
To: test@foo.com
List-Unsubscribe: foo@foo.com
From: Test <prani@blanditiis.io>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

`;

(async () => {
  const smtp = new SMTPServer({
    disabledCommands: ['AUTH'],
    onRcptTo(address, session, fn) {
      fn();
    },
    onConnect(session, fn) {
      fn();
    },

    async onData(stream, session, fn) {
      // <https://github.com/nodemailer/wildduck/issues/781>
      // const splitter = new Splitter();
      // const joiner = new Joiner();
      // pipe all streams together
      // stream.pipe(splitter).pipe(joiner).pipe(process.stdout);

      const splitter = new Splitter();
      stream.pipe(splitter);

      const body = await getStream.buffer(splitter);

      const { headers } = splitter;

      console.log('headers', headers.build().toString());
      console.log('body', body.toString());

      const raw = Buffer.concat([headers.build(), body]);
      console.log('raw', raw.toString()); // <--- this is OK

      fn();
    },
    logger: console,
    secure: false
  });

  const url = await listen(smtp.server, 0);

  console.log('server started', url);

  console.log('connecting to mx');

  const mx = await asyncMxConnect({
    target: 'localhost', // url.hostname, [::] doesn't work
    port: url.port
  });

  console.log('mx', mx);

  const transporter = nodemailer.createTransport({
    logger: console,
    debug: true,
    host: mx.host,
    port: mx.port,
    connection: mx.socket,
    ignoreTLS: true,
    secure: false,
    tls: { rejectUnauthorized: false }
  });

  const info = await transporter.sendMail({
    envelope: {
      from: 'from@test.com',
      to: 'to@test.com'
    },
    raw: message
  });

  console.log('info', info);

  process.exit(0);
})();

[titanism]

Okay I've been able to reproduce this. This is 100% some issue when you consume a stream from SMTP server with WildDuck's message splitter. The moment we swap from WildDuck's to mailsplit the issue goes away.

[titanism]

Going to try to get a reproducible code sample and then share it here, and try to find underlying cause.

[titanism]

It appears that checkHeaders is getting invoked with data arg equal to this:

Message-ID: <5a1x54l4e1@日本語.org>
To: test@test.com
List-Unsubscribe: foo@foo.com
From: Test <yoshie@日本語.org>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

H

https://github.com/nodemailer/wildduck/blob/82445592301047d0b43992106091d10456ae5611/lib/message-splitter.js#L49

This indicates to me that it's not splitting it properly for some reason.

[titanism]

Okay @andris9 this appears to be due to 3 characters followed by the line break. Any less than 3 is OK, and any more than 3 is OK. So ending with a line break and immediately before it having AAA results in AA, or more visually you can do ABC followed by a line break and it will result in BC. The first character gets removed.

[titanism]

@andris9 the bug is somewhere in here I think due to length/integer comparison

https://github.com/nodemailer/wildduck/blob/82445592301047d0b43992106091d10456ae5611/lib/message-splitter.js#L64-L78

[titanism]

I think that this might be the culprit:

headerPos = i - lblen + 1;

Because it's using 4 as a hard-coded value, it's doing i which could be something like 63 + 4 + 1 so it's splitting incorrectly. It might instead need to use the index of the LF_BINARY or CR_BINARY.

Edit: I think this is definitely the bug. headerPos is getting set incorrectly. Then later this code occurs https://github.com/nodemailer/wildduck/blob/82445592301047d0b43992106091d10456ae5611/lib/message-splitter.js#L88 so the wrong chunk value is set.

[louis-lau]

Might be worth sending a single message once done instead of 11 messages within minutes of each other haha, kind of overwhelms the inbox ;)

Otherwise not complaining, keep up the investigation and bug fixing :)

[titanism]

This happens for these two cases (note that there are NO line breaks at the end and so that case is ruled out):

Input:

Message-ID: <iyezf8uz6i@日本語.org>
To: test@test.com
List-Unsubscribe: foo@foo.com
From: Test <xolani@日本語.org>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hiya,

Output:

Message-ID: <iyezf8uz6i@日本語.org>
To: test@test.com
List-Unsubscribe: foo@foo.com
From: Test <xolani@日本語.org>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

iya,

...and also...

Input:

Message-ID: <6d1aj0of25@日本語.org>
To: test@test.com
List-Unsubscribe: foo@foo.com
From: Test <eunice@日本語.org>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

ABC

Output:

Message-ID: <6d1aj0of25@日本語.org>
To: test@test.com
List-Unsubscribe: foo@foo.com
From: Test <eunice@日本語.org>
Subject: testing this
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

BC