Skip to content

Commit 6a356fd

Browse files
andris9andris9
committed
fix: prevent null dereference crash in getImapConnection during connection drops
Centralize null guard in getImapConnection() so it throws instead of returning null when imapClient is gone. Tag error with code IMAPConnectionClosing and statusCode 503 so catch sites can downgrade to debug logging (stale sync on a dead connection is expected noise, not an actionable error). Apply the same error code to all existing null guards in mailbox.js and sync-operations.js for consistent handling.
1 parent 3ddf49b commit 6a356fd

File tree

3 files changed

+55
-21
lines changed

3 files changed

+55
-21
lines changed

‎lib/email-client/imap-client.js‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ class IMAPClient extends BaseClient {
191191
let syncing = this.syncing || ['init', 'connecting', 'syncing'].includes(this.state);
192192
if (!noPool && (!syncing || !allowSecondary)) {
193193
// Return the primary connection for most operations
194+
if (!this.imapClient) {
195+
let err = new Error('IMAP connection not available');
196+
err.code = 'IMAPConnectionClosing';
197+
err.statusCode = 503;
198+
throw err;
199+
}
194200
return this.imapClient;
195201
}
196202

@@ -202,14 +208,19 @@ class IMAPClient extends BaseClient {
202208
if (connectionClient && connectionClient.usable) {
203209
connectionOptions.connectionClient = connectionClient;
204210
return connectionClient;
205-
} else {
206-
// fall back to default connection
207-
return this.imapClient;
208211
}
209212
} catch (err) {
210213
this.logger.error({ msg: 'Failed to acquire command connection', reason, err });
211-
return this.imapClient;
212214
}
215+
216+
// Fall back to primary connection
217+
if (!this.imapClient) {
218+
let err = new Error('IMAP connection not available');
219+
err.code = 'IMAPConnectionClosing';
220+
err.statusCode = 503;
221+
throw err;
222+
}
223+
return this.imapClient;
213224
}
214225

215226
/**
@@ -535,14 +546,6 @@ class IMAPClient extends BaseClient {
535546
this.checkIMAPConnection(connectionOptions);
536547

537548
const connectionClient = await this.getImapConnection(connectionOptions, 'getCurrentListing');
538-
if (!connectionClient) {
539-
if (this.imapClient) {
540-
this.imapClient.close();
541-
}
542-
let error = new Error('Failed to get connection');
543-
error.code = 'ConnectionError';
544-
throw error;
545-
}
546549

547550
let accountData = await this.accountObject.loadAccountData();
548551

@@ -802,7 +805,11 @@ class IMAPClient extends BaseClient {
802805
try {
803806
await mailbox.onOpen(event);
804807
} catch (err) {
805-
imapClient.log.error({ msg: 'Open error', err });
808+
if (err.code === 'IMAPConnectionClosing') {
809+
imapClient.log.debug({ msg: 'Open skipped, connection closing', err });
810+
} else {
811+
imapClient.log.error({ msg: 'Open error', err });
812+
}
806813
}
807814
});
808815

‎lib/email-client/imap/mailbox.js‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ class Mailbox {
9494
getMailboxStatus(connectionClient) {
9595
connectionClient = connectionClient || this.connection.imapClient;
9696
if (!connectionClient) {
97-
throw new Error('IMAP connection not available');
97+
let err = new Error('IMAP connection not available');
98+
err.code = 'IMAPConnectionClosing';
99+
err.statusCode = 503;
100+
throw err;
98101
}
99102

100103
let mailboxInfo = connectionClient.mailbox;
@@ -491,7 +494,10 @@ class Mailbox {
491494
*/
492495
async select(skipIdle) {
493496
if (!this.connection.imapClient) {
494-
throw new Error('IMAP connection not available');
497+
let err = new Error('IMAP connection not available');
498+
err.code = 'IMAPConnectionClosing';
499+
err.statusCode = 503;
500+
throw err;
495501
}
496502
const currentLock = this.connection.imapClient.currentLock;
497503
// Avoid interfering with any active operations
@@ -554,7 +560,10 @@ class Mailbox {
554560
connectionClient = connectionClient || this.connection.imapClient;
555561

556562
if (!connectionClient) {
557-
throw new Error('IMAP connection not available');
563+
let err = new Error('IMAP connection not available');
564+
err.code = 'IMAPConnectionClosing';
565+
err.statusCode = 503;
566+
throw err;
558567
}
559568

560569
let lock = await connectionClient.getMailboxLock(this.path, options || {});
@@ -616,7 +625,13 @@ class Mailbox {
616625
return false;
617626
})
618627
.then(() => this.select())
619-
.catch(err => this.logger.error({ msg: 'Sync error', err }));
628+
.catch(err => {
629+
if (err.code === 'IMAPConnectionClosing') {
630+
this.logger.debug({ msg: 'Sync skipped, connection closing', err });
631+
} else {
632+
this.logger.error({ msg: 'Sync error', err });
633+
}
634+
});
620635
}, 1000);
621636
}
622637

‎lib/email-client/imap/sync-operations.js‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,10 @@ class SyncOperations {
197197
this.mailbox.syncing = true;
198198
try {
199199
if (!this.connection.imapClient) {
200-
throw new Error('IMAP connection not available');
200+
let err = new Error('IMAP connection not available');
201+
err.code = 'IMAPConnectionClosing';
202+
err.statusCode = 503;
203+
throw err;
201204
}
202205

203206
let knownUidNext = typeof storedStatus.uidNext === 'number' ? storedStatus.uidNext || 1 : 1;
@@ -351,7 +354,10 @@ class SyncOperations {
351354
this.mailbox.syncing = true;
352355
try {
353356
if (!this.connection.imapClient) {
354-
throw new Error('IMAP connection not available');
357+
let err = new Error('IMAP connection not available');
358+
err.code = 'IMAPConnectionClosing';
359+
err.statusCode = 503;
360+
throw err;
355361
}
356362

357363
let fields = { uid: true, flags: true, modseq: true, emailId: true, labels: true, internalDate: true };
@@ -378,7 +384,10 @@ class SyncOperations {
378384
let imapClient = this.connection.imapClient;
379385
if (!imapClient || !imapClient.usable) {
380386
this.logger.error({ msg: 'IMAP client not available for partial sync' });
381-
throw new Error('IMAP connection not available');
387+
let err = new Error('IMAP connection not available');
388+
err.code = 'IMAPConnectionClosing';
389+
err.statusCode = 503;
390+
throw err;
382391
}
383392

384393
try {
@@ -556,7 +565,10 @@ class SyncOperations {
556565
const imapClient = this.connection.imapClient;
557566
if (!imapClient || !imapClient.usable) {
558567
this.logger.error({ msg: 'IMAP client not available for FETCH' });
559-
throw new Error('IMAP connection not available');
568+
let err = new Error('IMAP connection not available');
569+
err.code = 'IMAPConnectionClosing';
570+
err.statusCode = 503;
571+
throw err;
560572
}
561573

562574
try {

0 commit comments

Comments
 (0)