Skip to content

Commit

Permalink
fix: fixed concurrency check, prevent stack in webhook bounce error e…
Browse files Browse the repository at this point in the history
…mail
  • Loading branch information
titanism committed Aug 25, 2024
1 parent 631c0b0 commit 73d2675
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 9 deletions.
2 changes: 1 addition & 1 deletion helpers/imap-notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ class IMAPNotifier extends EventEmitter {
return fn(null, true);

// decrease # connections for this alias (or domain if using catch-all)
const key = `connections_${config.env}:${
const key = `concurrent_imap_${config.env}:${
data.session.user.alias_id || data.session.user.domain_id
}`;

Expand Down
4 changes: 3 additions & 1 deletion helpers/on-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,9 @@ async function onAuth(auth, session, fn) {
this.server &&
!(this.server instanceof POP3Server) // not needed but keeping here anyways
) {
const key = `connections_${config.env}:${alias ? alias.id : domain.id}`;
const key = `concurrent_${this.constructor.name.toLowerCase()}_${
config.env
}:${alias ? alias.id : domain.id}`;
const count = await this.client.incrby(key, 0);
if (count < 0) await this.client.del(key); // safeguard
else if (!adminExists && count > 60) {
Expand Down
9 changes: 6 additions & 3 deletions helpers/on-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@ const config = require('#config');
const logger = require('#helpers/logger');

async function onClose(session) {
// NOTE: do not change this prefix unless you also change it in `helpers/on-connect.js`
// NOTE: do not change this prefix unless you also change it in `helpers/on-connect.js` and `helpers/on-auth.js`
const prefix = `concurrent_${this.constructor.name.toLowerCase()}_${
config.env
}`;
await Promise.all([
//
// decrease # concurrent connections for
// client hostname or remote address
// client hostname or remote address (not applicable to SMTP)
//
(async () => {
if (!session?.resolvedRootClientHostname && !session?.remoteAddress)
if (
(!session?.resolvedRootClientHostname && !session?.remoteAddress) ||
this.constructor.name === 'SMTP'
)
return;
try {
const key = `${prefix}:${
Expand Down
3 changes: 3 additions & 0 deletions helpers/on-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ 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/process-email.js
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ async function processEmail({ email, port = 25, resolver, client }) {
domain.bounce_webhook
) +
`<pre><code>${JSON.stringify(
parseErr(err),
_.omit(parseErr(err), 'stack'),
null,
2
)}</code></pre>`;
Expand Down
8 changes: 7 additions & 1 deletion locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2825,5 +2825,11 @@
"Create your email template with a": "Create your email template with a",
"or a": "or a",
"file:": "file:",
"In this example, we use the": "In this example, we use the"
"In this example, we use the": "In this example, we use the",
"Domain has an incorrect DNS <span class=\"notranslate\">TXT</span> record for verification. Please ensure <span class=\"notranslate\">%s</span> is the only verification record that exists.": "Domain has an incorrect DNS <span class=\"notranslate\">TXT</span> record for verification. Please ensure <span class=\"notranslate\">%s</span> is the only verification record that exists.",
"We could not complete your request at this time to create a backup. Either a backup is already in progress or the queue is currently full. Please try again later, and if this problem persists please contact us for help.": "We could not complete your request at this time to create a backup. Either a backup is already in progress or the queue is currently full. Please try again later, and if this problem persists please contact us for help.",
"Mailbox does not exist": "Mailbox does not exist",
"System Alert": "System Alert",
"This is an automated system alert.": "This is an automated system alert.",
"Home": "Home"
}
4 changes: 2 additions & 2 deletions test/imap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ test.beforeEach(async (t) => {

await imapFlow.connect();

const key = `connections_${config.env}:${t.context.alias.id}`;
const key = `concurrent_imap_${config.env}:${t.context.alias.id}`;
const count = await client.incrby(key, 0);
t.is(count, 1);

Expand All @@ -195,7 +195,7 @@ test.beforeEach(async (t) => {
});

test.afterEach(async (t) => {
const key = `connections_${config.env}:${t.context.alias.id}`;
const key = `concurrent_imap_${config.env}:${t.context.alias.id}`;
const pttlBefore = await client.pttl(key);
t.true(pttlBefore > 0);
await t.context.imapFlow.logout();
Expand Down

0 comments on commit 73d2675

Please sign in to comment.