From 31d4c778143b80befcba546390aeef2049738a7d Mon Sep 17 00:00:00 2001 From: titanism <101466223+titanism@users.noreply.github.com> Date: Wed, 18 Dec 2024 17:10:59 -0600 Subject: [PATCH] fix: optimize MX server by switching to `striptags` instead of `html-to-text` and improved performance of parseAddresses helper function, discovered bug where ARC broken due to DKIM on MX (will fix) --- app/models/emails.js | 10 +++--- app/models/logs.js | 3 +- helpers/get-attributes.js | 6 ++-- helpers/get-forwarding-addresses.js | 2 +- helpers/get-from-address.js | 4 +-- helpers/on-data-mx.js | 3 ++ helpers/on-data.js | 9 ++--- helpers/parse-addresses.js | 36 ++++++++++++++++---- helpers/parse-host-from-domain-or-address.js | 9 +---- helpers/parse-username.js | 2 +- helpers/refine-and-log-error.js | 8 +++-- helpers/send-email.js | 3 ++ 12 files changed, 58 insertions(+), 37 deletions(-) diff --git a/app/models/emails.js b/app/models/emails.js index c410ffae92..2fc7f7bfec 100644 --- a/app/models/emails.js +++ b/app/models/emails.js @@ -927,8 +927,8 @@ Emails.statics.queue = async function ( // rewrite from header to be without "+" symbol // so that users can send with "+" address filtering // - const name = parseUsername(addresses[0].address); // converts to ASCII - const domain = parseHostFromDomainOrAddress(addresses[0].address); // converts to ASCII + const name = parseUsername(addresses[0]); // converts to ASCII + const domain = parseHostFromDomainOrAddress(addresses[0]); // converts to ASCII from = `${name}@${domain}`; // ASCII formatted From address header } @@ -944,12 +944,12 @@ Emails.statics.queue = async function ( (isEnvelopeFromEmpty && lowercaseKey === 'from')) && SENDER_KEYS.has(lowercaseKey) ) - info.envelope.from = addresses[0].address; + info.envelope.from = addresses[0]; // envelope to else if (isEnvelopeToEmpty && RCPT_TO_KEYS.has(lowercaseKey)) { for (const address of addresses) { - if (info.envelope.to.includes(address.address)) continue; - info.envelope.to.push(address.address); + if (info.envelope.to.includes(address)) continue; + info.envelope.to.push(address); } } } diff --git a/app/models/logs.js b/app/models/logs.js index ef9fe43b21..0047f85049 100644 --- a/app/models/logs.js +++ b/app/models/logs.js @@ -405,8 +405,7 @@ Logs.pre('save', function (next) { // keywords.add(obj.name); // } - if (isSANB(obj.address) && isEmail(checkSRS(obj.address))) - keywords.add(checkSRS(obj.address)); + keywords.add(checkSRS(obj)); } } } diff --git a/helpers/get-attributes.js b/helpers/get-attributes.js index 572f96805c..a1ae79d9f3 100644 --- a/helpers/get-attributes.js +++ b/helpers/get-attributes.js @@ -48,14 +48,14 @@ async function getAttributes(headers, session, resolver, isAligned = false) { const replyTo = [ // check the Reply-To header - ...replyToAddresses.map((addr) => checkSRS(addr.address).toLowerCase()), + ...replyToAddresses.map((addr) => checkSRS(addr).toLowerCase()), // check the Reply-To header domains ...replyToAddresses.map((addr) => - parseHostFromDomainOrAddress(checkSRS(addr.address)) + parseHostFromDomainOrAddress(checkSRS(addr)) ), // check the Reply-To header root domains ...replyToAddresses.map((addr) => - parseRootDomain(parseHostFromDomainOrAddress(checkSRS(addr.address))) + parseRootDomain(parseHostFromDomainOrAddress(checkSRS(addr))) ) ]; diff --git a/helpers/get-forwarding-addresses.js b/helpers/get-forwarding-addresses.js index a46980ebfa..83b69340cd 100644 --- a/helpers/get-forwarding-addresses.js +++ b/helpers/get-forwarding-addresses.js @@ -32,7 +32,7 @@ const { decrypt } = require('#helpers/encrypt-decrypt'); const USER_AGENT = `${config.pkg.name}/${config.pkg.version}`; function parseFilter(address) { - ({ address } = parseAddresses(address)[0]); + address = parseAddresses(address)[0]; return address.includes('+') ? address.split('+')[1].split('@')[0] : ''; } diff --git a/helpers/get-from-address.js b/helpers/get-from-address.js index 6e63ab7a15..a08cb0bdc9 100644 --- a/helpers/get-from-address.js +++ b/helpers/get-from-address.js @@ -31,9 +31,7 @@ function getFromAddress(originalFrom) { ); // set original from address that was parsed - return checkSRS( - punycode.toASCII(originalFromAddresses[0].address) - ).toLowerCase(); + return checkSRS(punycode.toASCII(originalFromAddresses[0])).toLowerCase(); } module.exports = getFromAddress; diff --git a/helpers/on-data-mx.js b/helpers/on-data-mx.js index 03402faf3b..1f9a81836c 100644 --- a/helpers/on-data-mx.js +++ b/helpers/on-data-mx.js @@ -139,6 +139,7 @@ async function sendSysAdminEmail(template, err, session, headers) { const cache = await this.client.get(key); if (boolean(cache)) return; await this.client.set(key, true, 'PX', ms('30d')); + // TODO: use Emails.queue or something await emailHelper({ template, message: { @@ -1190,6 +1191,7 @@ async function forward(recipient, headers, session, raw, body) { const locals = { locale }; if (user) locals.user = user; + // TODO: use Emails.queue or something await emailHelper({ template: 'self-test', message: { to: normal }, @@ -1603,6 +1605,7 @@ async function onDataMX(raw, session, headers, body) { const cache = await this.client.get(key); if (cache) return; await this.client.set(key, true, 'PX', ms('30d')); + // TODO: use Emails.queue or something await emailHelper({ template: 'phishing', message: { to, bcc: config.email.message.from }, diff --git a/helpers/on-data.js b/helpers/on-data.js index ffd7e18c23..fcb046b3f6 100644 --- a/helpers/on-data.js +++ b/helpers/on-data.js @@ -156,15 +156,12 @@ async function onData(stream, _session, fn) { const originalToAddresses = parseAddresses(getHeaders(headers, 'to')); for (const obj of originalToAddresses) { const shouldThrow = - parseRootDomain(parseHostFromDomainOrAddress(obj.address)) === - env.WEB_HOST; + parseRootDomain(parseHostFromDomainOrAddress(obj)) === env.WEB_HOST; // rewrite the to line - if (checkSRS(obj.address, shouldThrow) !== obj.address) + if (checkSRS(obj, shouldThrow) !== obj) headers.update( 'to', - headers - .getFirst('to') - .replaceAll(obj.address, checkSRS(obj.address, shouldThrow)) + headers.getFirst('to').replaceAll(obj, checkSRS(obj, shouldThrow)) ); } diff --git a/helpers/parse-addresses.js b/helpers/parse-addresses.js index 9f25b93828..e3e594ebad 100644 --- a/helpers/parse-addresses.js +++ b/helpers/parse-addresses.js @@ -3,30 +3,54 @@ * SPDX-License-Identifier: BUSL-1.1 */ -const _ = require('lodash'); const addressParser = require('nodemailer/lib/addressparser'); const addrs = require('email-addresses'); const isSANB = require('is-string-and-not-blank'); const isEmail = require('#helpers/is-email'); +// +// NOTE: if we ever use `email-regex-safe` in future +// we need to ensure `tlds` are considered against punycode toASCII cases +// + // function parseAddresses(input) { if (!isSANB(input)) return []; + // + if ( + input.startsWith('<') && + input.endsWith('>') && + isEmail(input.slice(1, -1)) + ) + return [input.slice(1, -1)]; + + // foo@bar.com + if (isEmail(input)) return [input]; + + // more complex stuff here + // `"Adobe Acrobat" ` + // `"foo@bar.com" , "beep@boop.com" ` let addresses = addrs.parseAddressList({ input, partial: true }) || []; + // + // NOTE: we can't use `email-regex-safe` for values with quotes and such because it returns the wrong values (and dups) + // + // > const emailRegexSafe = require('email-regex-safe') + // > `"foo@bar.com" , "beep@boop.com" `.match(emailRegexSafe()) + // [ 'foo@bar.com', 'foo@bar.com', 'beep@boop.com', 'beep@boop.com' ] + // + if (addresses.length === 0) addresses = addrs.parseAddressList({ input }) || []; // safeguard if (addresses.length === 0) addresses = addressParser(input); - addresses = addresses.filter( - (addr) => _.isObject(addr) && isSANB(addr.address) && isEmail(addr.address) - ); - - return addresses; + return addresses + .filter((addr) => isEmail(addr?.address)) + .map((addr) => addr.address); } module.exports = parseAddresses; diff --git a/helpers/parse-host-from-domain-or-address.js b/helpers/parse-host-from-domain-or-address.js index 1f209a4ef0..ab71c984be 100644 --- a/helpers/parse-host-from-domain-or-address.js +++ b/helpers/parse-host-from-domain-or-address.js @@ -7,7 +7,6 @@ const punycode = require('node:punycode'); const { isIP } = require('node:net'); const URLParse = require('url-parse'); -const _ = require('lodash'); const isFQDN = require('is-fqdn'); const isSANB = require('is-string-and-not-blank'); const { isURL } = require('@forwardemail/validator'); @@ -25,13 +24,7 @@ function parseHostFromDomainOrAddress(address) { domain = url.hostname; } else { const parsedAddresses = parseAddresses(address); - if ( - _.isArray(parsedAddresses) && - _.isObject(parsedAddresses[0]) && - isSANB(parsedAddresses[0].address) - ) { - domain = parsedAddresses[0].address; - } + if (parsedAddresses[0]) domain = parsedAddresses[0]; } const atPos = domain.indexOf('@'); diff --git a/helpers/parse-username.js b/helpers/parse-username.js index 4c97282c31..eabc3edf69 100644 --- a/helpers/parse-username.js +++ b/helpers/parse-username.js @@ -8,7 +8,7 @@ const punycode = require('node:punycode'); const parseAddresses = require('#helpers/parse-addresses'); function parseUsername(address, ignorePlus = false) { - ({ address } = parseAddresses(address)[0]); + address = parseAddresses(address)[0]; let username = !ignorePlus && address.includes('+') ? address.split('+')[0] diff --git a/helpers/refine-and-log-error.js b/helpers/refine-and-log-error.js index 5d44c25842..dbd139b16f 100644 --- a/helpers/refine-and-log-error.js +++ b/helpers/refine-and-log-error.js @@ -5,7 +5,8 @@ const _ = require('lodash'); const splitLines = require('split-lines'); -const { convert } = require('html-to-text'); +const striptags = require('striptags'); +// const { convert } = require('html-to-text'); const getErrorCode = require('./get-error-code'); const isRetryableError = require('./is-retryable-error'); @@ -13,7 +14,7 @@ const isLockingError = require('./is-locking-error'); const isCodeBug = require('./is-code-bug'); const logger = require('./logger'); -const env = require('#config/env'); +// const env = require('#config/env'); // this is sourced from FE original codebase function refineAndLogError(err, session, isIMAP = false, instance) { @@ -72,6 +73,8 @@ function refineAndLogError(err, session, isIMAP = false, instance) { // NOTE: this was inspired from `koa-better-error-handler` response for API endpoints // (and it is used because some errors are translated with HTML tags, e.g. notranslate) // + err.message = striptags(err.message); + /* err.message = convert(err.message, { wordwrap: false, selectors: [ @@ -86,6 +89,7 @@ function refineAndLogError(err, session, isIMAP = false, instance) { ], linkBrackets: false }); + */ // // replace linebreaks diff --git a/helpers/send-email.js b/helpers/send-email.js index 990faf88a8..f4bb2fea4e 100644 --- a/helpers/send-email.js +++ b/helpers/send-email.js @@ -187,6 +187,8 @@ async function sendEmail( // signed the message with our DKIM-Signature yet, which we only // want to do if this was an MX server (since `#helpers/process-email` already applies it for SMTP) // + // TODO: this breaks ARC + /* if (!pgp && !email && !domain) { const signResult = await dkimSign(raw, { canonicalization: 'relaxed/relaxed', @@ -208,6 +210,7 @@ async function sendEmail( const signatures = Buffer.from(signResult.signatures, 'utf8'); raw = Buffer.concat([signatures, raw], signatures.length + raw.length); } + */ // // if we're in development mode then use preview-email to render queue processing