Skip to content

Commit

Permalink
fix: fixed message id issue, fixed test concurrency, fixed navbar bug…
Browse files Browse the repository at this point in the history
…, fixed header decoding, updated snapshots and synced locales
  • Loading branch information
titanism committed Aug 27, 2024
1 parent 91e3dfb commit 26b66f7
Show file tree
Hide file tree
Showing 21 changed files with 175 additions and 64 deletions.
8 changes: 5 additions & 3 deletions app/controllers/api/v1/emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ async function create(ctx) {
//
for (const header of EMOJI_HEADERS) {
if (isSANB(message[header])) {
for (const match of message[header].matchAll(EMOJI_REGEX)) {
const matches = message[header].match(EMOJI_REGEX);
if (!matches || !Array.isArray(matches)) continue;
for (const match of matches) {
message[header] = message[header].replaceAll(
match[0],
libmime.encodeWord(match[0])
match,
libmime.encodeWord(match)
);
}
}
Expand Down
28 changes: 23 additions & 5 deletions app/models/emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,29 @@ Emails.statics.queue = async function (
//
let from;

// <https://github.com/andris9/mailsplit/issues/21>
const lines = headers.headers
.toString()
.replace(/[\r\n]+$/, '')
.split(/\r?\n/);
//
// NOTE: we should only decode these headers, but we're going to decode all anyways for now
// <https://github.com/nodemailer/mailparser/blob/ac11f78429cf13da42162e996a05b875030ae1c1/lib/mail-parser.js#L329>
//
// NOTE: we decode header values because
// want them to be easily searchable
// (e.g. an emoji in a header line gets encoded as:
// > '=?UTF-8?Q?=F0=9F=8E=89_beep?='
// and we want output that looks like
// > 🎉 beep
// (e.g. so a user could search for 🎉)
//
// we can't use mailsplit b/c unicode characters get rewritten
// <https://github.com/andris9/mailsplit/issues/21>
//
let lines = headers.headers.toString();
try {
lines = headers.libmime.decodeWords(lines);
} catch {
// ignore, keep as is
}

lines = lines.replace(/[\r\n]+$/, '').split(/\r?\n/);

for (const line of lines) {
const index = line.indexOf(':');
Expand Down
2 changes: 1 addition & 1 deletion app/views/_nav.pug
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ nav.navbar(class=navbarClasses.join(" "))
aria-controls="navbar-header",
aria-expanded="false",
aria-label=t("Toggle navigation"),
class=isAuthenticated() ? "text-white" : "text-dark text-themed"
class=user ? "text-white" : "text-dark text-themed"
)
i.fas.fa-bars
//- once we have responsive border utilities added to bootstrap
Expand Down
7 changes: 5 additions & 2 deletions ava.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@
* SPDX-License-Identifier: BUSL-1.1
*/

// const isCI = require('is-ci');
const os = require('node:os');

const isCI = require('is-ci');

// const { familySync, GLIBC } = require('detect-libc');

module.exports = {
verbose: true,
failFast: true,
// serial: true,
// concurrency: 2, // <--- this defaults to `2` in CI environments
concurrency: isCI ? 2 : Math.floor(os.cpus().length / 2),
files: ['test/*.js', 'test/**/*.js', 'test/**/**/*.js', '!test/utils.js'],
// <https://github.com/lovell/sharp/issues/3164#issuecomment-1168328811>
// workerThreads: familySync() !== GLIBC,
Expand Down
6 changes: 3 additions & 3 deletions helpers/create-bounce.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ function createBounce(email, error, message) {
'Subject',
`Delivery Status Notification (${isDelayed ? 'Delayed' : 'Failure'})`
);
rootNode.setHeader('In-Reply-To', email.messageId);
rootNode.setHeader('References', email.messageId);
rootNode.setHeader('X-Original-Message-ID', email.messageId);
rootNode.setHeader('In-Reply-To', `<${email.messageId}>`);
rootNode.setHeader('References', `<${email.messageId}>`);
rootNode.setHeader('X-Original-Message-ID', `<${email.messageId}>`);

const response = convert(error.response || error.message, {
wordwrap: false,
Expand Down
4 changes: 1 addition & 3 deletions helpers/get-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ const parseRootDomain = require('#helpers/parse-root-domain');
const parseAddresses = require('#helpers/parse-addresses');

function getAttributes(headers, session) {
const replyToAddresses = parseAddresses(
getHeaders(headers, true, 'reply-to')
);
const replyToAddresses = parseAddresses(getHeaders(headers, 'reply-to'));

//
// check if the From, Reply-To, MAIL FROM, sender IP/host, or RCPT TO were silent banned
Expand Down
49 changes: 16 additions & 33 deletions helpers/get-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,12 @@
* SPDX-License-Identifier: BUSL-1.1
*/

// <https://github.com/nodemailer/mailparser/blob/ac11f78429cf13da42162e996a05b875030ae1c1/lib/mail-parser.js#L329>
const ENCODED_HEADERS = new Set([
'from',
'to',
'cc',
'bcc',
'sender',
'reply-to',
'delivered-to',
'return-path',

'subject',
'references',
'message-id',
'in-reply-to'
]);

function getHeaders(headers, decode = true, specificKey = null) {
function getHeaders(headers, specificKey = null) {
const _headers = {};

// NOTE: unicode characters get rewritten due to a (bug) in mailsplit
// <https://github.com/andris9/mailsplit/issues/21>
const lines = headers.headers
.toString()
.replace(/[\r\n]+$/, '')
.split(/\r?\n/);

//
// NOTE: we should only decode these headers, but we're going to decode all anyways for now
// <https://github.com/nodemailer/mailparser/blob/ac11f78429cf13da42162e996a05b875030ae1c1/lib/mail-parser.js#L329>
//
// NOTE: we decode header values because
// want them to be easily searchable
Expand All @@ -39,20 +18,24 @@ function getHeaders(headers, decode = true, specificKey = null) {
// > 🎉 beep
// (e.g. so a user could search for 🎉)
//
// we can't use mailsplit b/c unicode characters get rewritten
// <https://github.com/andris9/mailsplit/issues/21>
//
let lines = headers.headers.toString();
try {
lines = headers.libmime.decodeWords(lines);
} catch {
// ignore, keep as is
}

lines = lines.replace(/[\r\n]+$/, '').split(/\r?\n/);

for (const line of lines) {
const index = line.indexOf(':');
const key = line.slice(0, index);

_headers[key] = line.slice(index + 1).trim();

if (decode && ENCODED_HEADERS.has(key.toLowerCase())) {
try {
_headers[key] = headers.libmime.decodeWords(_headers[key]);
} catch {
// ignore, keep as is
}
}

if (
typeof specificKey === 'string' &&
key.toLowerCase() === specificKey.toLowerCase()
Expand Down
4 changes: 2 additions & 2 deletions helpers/is-arbitrary.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ const REGEX_APP_NAME = new RE2(new RegExp(env.APP_NAME, 'im'));

// eslint-disable-next-line complexity
function isArbitrary(session, headers, bodyStr) {
let subject = getHeaders(headers, true, 'subject');
let subject = getHeaders(headers, 'subject');
if (!isSANB(subject)) subject = null;

// <https://github.com/andris9/mailsplit/issues/21>
const from = getHeaders(headers, true, 'from');
const from = getHeaders(headers, 'from');

// rudimentary blocking
if (subject && BLOCKED_PHRASES.test(subject))
Expand Down
3 changes: 0 additions & 3 deletions helpers/on-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ async function onConnect(session, fn) {
// this is used for greylisting and in other places
session.arrivalTime = session.arrivalDate.getTime();

// return early if we're on SMTP server
if (this.constructor.name === 'SMTP') return fn();

// lookup the client hostname
try {
const [clientHostname] = await this.resolver.reverse(
Expand Down
2 changes: 1 addition & 1 deletion helpers/on-data-mx.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ async function updateMXHeaders(session, headers, body) {
// then we don't want to modify it of course
//
// <https://github.com/andris9/mailsplit/issues/21>
if (!getHeaders(headers, true, 'reply-to'))
if (!getHeaders(headers, 'reply-to'))
headers.update('Reply-To', session.originalFromAddress);

// rewrite ARC sealed headers with updated headers object value
Expand Down
2 changes: 1 addition & 1 deletion helpers/on-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ async function onData(stream, _session, fn) {
// in addition to RCPT TO being incorrect due to improperly configured server sending to SRS forwarded address
// we also need to rewrite the "To" header an rewrite any SRS forwarded addresses with their actual ones
//
const originalToAddresses = parseAddresses(getHeaders(headers, true, 'to'));
const originalToAddresses = parseAddresses(getHeaders(headers, 'to'));
for (const obj of originalToAddresses) {
const shouldThrow =
parseRootDomain(parseHostFromDomainOrAddress(obj.address)) ===
Expand Down
4 changes: 2 additions & 2 deletions helpers/process-email.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,9 @@ async function processEmail({ email, port = 25, resolver, client }) {

// set Message-ID header
const messageId = data.headers.getFirst('message-id');
if (!messageId || messageId !== email.messageId) {
if (!messageId || messageId !== `<${email.messageId}>`) {
data.headers.remove('message-id');
data.headers.add('Message-ID', email.messageId);
data.headers.add('Message-ID', `<${email.messageId}>`);
}

// set Date header
Expand Down
2 changes: 1 addition & 1 deletion helpers/update-session.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async function updateSession(raw, headers, session) {
session.headers = getHeaders(headers);

// <https://github.com/andris9/mailsplit/issues/21>
const from = getHeaders(headers, true, 'from');
const from = getHeaders(headers, 'from');

// getFromAddress will thrown an error if it's not RFC 5322 compliant
session.originalFromAddress = getFromAddress(from);
Expand Down
89 changes: 87 additions & 2 deletions test/api/v1.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const ip = require('ip');
const isBase64 = require('is-base64');
const ms = require('ms');
const pify = require('pify');
const pWaitFor = require('p-wait-for');
const test = require('ava');
const { SMTPServer } = require('smtp-server');

Expand Down Expand Up @@ -573,12 +574,12 @@ Test`.trim()
// validate header From was converted properly
t.is(
res.body.headers.From,
`${emoji('blush')} Test <${alias.name}@${domain.name}>`
`"${emoji('blush')} Test" <${alias.name}@${domain.name}>`
);
t.is(res.body.headers.Subject, `${emoji('blush')} testing this`);
t.true(
res.body.message.includes(
`From: =?UTF-8?Q?=F0=9F=98=8A_Test?= <${alias.name}@${domain.name}>`
`From: "=?UTF-8?Q?=F0=9F=98=8A?= Test" <${alias.name}@${domain.name}>`
)
);
}
Expand Down Expand Up @@ -670,6 +671,7 @@ Test`.trim()
await resolver.options.cache.mset(map);

// spin up a test smtp server that simply responds with OK
if (!getPort) await pWaitFor(() => Boolean(getPort), { timeout: ms('15s') });
const port = await getPort();
let attempted = false;
const server = new SMTPServer({
Expand Down Expand Up @@ -1082,6 +1084,7 @@ test('smtp outbound spam block detection', async (t) => {
await resolver.options.cache.mset(map);

// spin up a test smtp server that simply responds with OK
if (!getPort) await pWaitFor(() => Boolean(getPort), { timeout: ms('15s') });
const port = await getPort();
const server = new SMTPServer({
disabledCommands: ['AUTH'],
Expand Down Expand Up @@ -2044,3 +2047,85 @@ test('encrypts and decrypts TXT', async (t) => {
'[email protected]'
);
});

// TODO: this same test is on SMTP side too for consistency
test('parses UTF-8 encoded headers', async (t) => {
const user = await t.context.userFactory
.withState({
plan: 'enhanced_protection',
[config.userFields.planSetAt]: dayjs().startOf('day').toDate()
})
.create();

await t.context.paymentFactory
.withState({
user: user._id,
amount: 300,
invoice_at: dayjs().startOf('day').toDate(),
method: 'free_beta_program',
duration: ms('30d'),
plan: user.plan,
kind: 'one-time'
})
.create();

await user.save();

const domain = await t.context.domainFactory
.withState({
members: [{ user: user._id, group: 'admin' }],
plan: user.plan,
resolver,
has_smtp: true
})
.create();

const alias = await t.context.aliasFactory
.withState({
user: user._id,
domain: domain._id,
recipients: [user.email]
})
.create();

const raw = `From: Test <${alias.name}@${domain.name}>
To: Forward Email <[email protected]>
Subject: =?UTF-8?Q?Forward_Email_=E2=80=93_Code_Bug=3A_Erro?=
=?UTF-8?Q?r_-_Mail_command_failed=3A_550_5=2E7=2E1?=
=?UTF-8?Q?_Unfortunately=2C_messages_from_=5B164?=
=?UTF-8?Q?=2E92=2E70=2E200=5D_weren=27t_sent=2E_Pl?=
=?UTF-8?Q?ease_contact_your_Internet_service_provi?=
=?UTF-8?Q?der_since_part_of_their_network_is_on_ou?=
=?UTF-8?Q?r_block_list_=28S3150=29=2E_You_can_also?=
=?UTF-8?Q?_refer_your_provider_to_http=3A//mail=2E?=
=?UTF-8?Q?live=2Ecom/mail/troubleshooting=2Easpx?=
=?UTF-8?Q?=23errors=2E_=5BName=3DProtocol_Filter_A?=
=?UTF-8?Q?gent=5D=5BAGT=3DPFA=5D=5BMxId=3D11B98999?=
=?UTF-8?Q?1C72A5F2=5D_=5BBL6PEPF00022571=2Enamprd0?=
=?UTF-8?Q?2=2Eprod=2Eoutlook=2Ecom_2024-08-25T23?=
=?UTF-8?Q?=3A54=3A32=2E568Z_08DCC4CC8ECEBA56=5D_?=
=?UTF-8?Q?=2866cbc438d0090cee7c346279=29?=
Message-ID: <[email protected]>
Date: Sun, 25 Aug 2024 23:54:33 +0000
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
SYSTEM ALERT
`.trim();

const res = await t.context.api
.post('/v1/emails')
.auth(user[config.userFields.apiToken])
.set('Content-Type', 'application/json')
.set('Accept', 'application/json')
.send({
raw
});

t.is(res.status, 200);

t.is(
res.body.headers.Subject,
"Forward Email – Code Bug: Error - Mail command failed: 550 5.7.1 Unfortunately, messages from [164.92.70.200] weren't sent. Please contact your Internet service provider since part of their network is on our block list (S3150). You can also refer your provider to http://mail.live.com/mail/troubleshooting.aspx#errors. [Name=Protocol Filter Agent][AGT=PFA][MxId=11B989991C72A5F2] [BL6PEPF00022571.namprd02.prod.outlook.com 2024-08-25T23:54:32.568Z 08DCC4CC8ECEBA56] (66cbc438d0090cee7c346279)"
);
});
1 change: 1 addition & 0 deletions test/caldav/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ test.beforeEach(async (t) => {

t.context.client = client;

if (!getPort) await pWaitFor(() => Boolean(getPort), { timeout: ms('15s') });
const port = await getPort();
const sqlitePort = await getPort();

Expand Down
1 change: 1 addition & 0 deletions test/imap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ test.beforeEach(async (t) => {
await utils.setupRedisClient(t);
const secure = false;
t.context.secure = secure;
if (!getPort) await pWaitFor(() => Boolean(getPort), { timeout: ms('15s') });
const port = await getPort();
const sqlitePort = await getPort();
const sqlite = new SQLite({
Expand Down
Loading

0 comments on commit 26b66f7

Please sign in to comment.