tls: route event listener exceptions through error handlers

Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #63822
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>

Author: aduh95

lib/internal/tls/wrap.js

@@ -270,7 +270,14 @@ function loadSession(hello) {
   if (hello.sessionId.length <= 0 ||
       hello.tlsTicket ||
       (owner.server &&
-      !owner.server.emit('resumeSession', hello.sessionId, onSession))) {
+      !(() => {
+        try {
+          return owner.server.emit('resumeSession', hello.sessionId, onSession);
+        } catch (err) {
+          onSession(err);
+          return true;
+        }
+      })())) {
     // Sessions without identifiers can't be resumed.
     // Sessions with tickets can be resumed directly from the ticket, no server
     // session storage is necessary.
@@ -399,10 +406,14 @@ function requestOCSP(socket, info) {
   };
 
   debug('server oncertcb emit OCSPRequest');
-  socket.server.emit('OCSPRequest',
-                     ctx.getCertificate(),
-                     ctx.getIssuer(),
-                     onOCSP);
+  try {
+    socket.server.emit('OCSPRequest',
+                       ctx.getCertificate(),
+                       ctx.getIssuer(),
+                       onOCSP);
+  } catch (err) {
+    socket.destroy(err);
+  }
 }
 
 function requestOCSPDone(socket) {
@@ -455,7 +466,14 @@ function onnewsession(sessionId, session) {
   };
 
   owner._newSessionPending = true;
-  if (!owner.server.emit('newSession', sessionId, session, done))
+
+  let hasEmitted = false;
+  try {
+    hasEmitted = owner.server.emit('newSession', sessionId, session, done);
+  } catch (err) {
+    owner.server.emit('error', err);
+  }
+  if (!hasEmitted)
     done();
 }
 

test/parallel/test-tls-psk-alpn-callback-exception-handling.js

@@ -21,6 +21,7 @@ if (process.features.openssl_is_boringssl) {
 
 const assert = require('assert');
 const { describe, it } = require('node:test');
+const https = require('node:https');
 const tls = require('tls');
 const fixtures = require('../common/fixtures');
 
@@ -117,6 +118,170 @@ describe('TLS callback exception handling', () => {
     await promise;
   });
 
+  it('newSession synchronous error emits error', async (t) => {
+    const server = tls.createServer({
+      key: fixtures.readKey('agent2-key.pem'),
+      cert: fixtures.readKey('agent2-cert.pem'),
+      sessionTimeout: 3600,
+    });
+    const agent = new https.Agent({
+      keepAlive: false,
+      keepAliveMsecs: 10_000,
+      maxSockets: 5,
+    });
+
+    t.after(() => {
+      agent.destroy();
+      server.close();
+    });
+
+    const { promise, resolve } = createTestPromise();
+
+    const expectedError = new Error();
+    server.on('newSession', common.mustCall(() => {
+      setImmediate(resolve);
+      throw expectedError;
+    }, 2));
+
+    server.on('error', common.mustCall((e) => {
+      assert.strictEqual(e, expectedError);
+    }, 2));
+
+    await new Promise((res) => server.listen(0, res));
+
+    // We might need one or two requests to trigger the `resumeSession` event
+    const clientOptions = {
+      port: server.address().port,
+      host: '127.0.0.1',
+      rejectUnauthorized: false,
+    };
+    // First connection: Establish a session
+    const socket = tls.connect(clientOptions, common.mustCall(() => {
+    // Close immediately to allow session resumption on next connect
+      socket.end();
+    }));
+
+    socket.on('error', common.mustNotCall());
+
+    socket.on('close', common.mustCall());
+
+    await promise;
+  });
+
+  it('OCSPRequest listener synchronous error emits tlsClientError', async (t) => {
+    const server = tls.createServer({
+      key: fixtures.readKey('agent2-key.pem'),
+      cert: fixtures.readKey('agent2-cert.pem'),
+      requestOCSP: true,
+    });
+
+    t.after(() => server.close());
+
+    const { promise, resolve, reject } = createTestPromise();
+
+    const expectedError = new Error();
+    server.on('OCSPRequest', (cert, issuer, cb) => {
+      throw expectedError;
+    });
+
+    server.on('tlsClientError', common.mustCall((err, socket) => {
+      try {
+        assert.strictEqual(err, expectedError);
+        socket.destroy();
+        resolve();
+      } catch (e) {
+        reject(e);
+      }
+    }));
+    server.on('secureConnection', common.mustNotCall('secureConnection listener'));
+    server.on('error', common.mustNotCall('server error listener'));
+
+    await new Promise((res) => server.listen(0, res));
+
+    const client = tls.connect({
+      port: server.address().port,
+      host: '127.0.0.1',
+      rejectUnauthorized: false,
+      requestOCSP: true,
+    });
+
+    client.on('error', common.mustCall());
+
+    await promise;
+  });
+
+  it('resumeSession listener synchronous error emits tlsClientError', async (t) => {
+    const server = tls.createServer({
+      key: fixtures.readKey('agent2-key.pem'),
+      cert: fixtures.readKey('agent2-cert.pem'),
+      sessionTimeout: 3600,
+    });
+    const agent = new https.Agent({
+      keepAlive: true,
+      keepAliveMsecs: 10_000,
+      maxSockets: 5,
+    });
+
+    t.after(() => {
+      agent.destroy();
+      server.close();
+    });
+
+    const { promise, resolve, reject } = createTestPromise();
+
+    const expectedError = new Error();
+    server.on('resumeSession', (id, cb) => {
+      throw expectedError;
+    });
+
+    server.on('tlsClientError', common.mustCall((err, socket) => {
+      try {
+        assert.strictEqual(err, expectedError);
+        socket.destroy();
+        resolve();
+      } catch (e) {
+        reject(e);
+      }
+    }));
+    server.on('error', common.mustNotCall('server error listener'));
+
+    await new Promise((res) => server.listen(0, res));
+
+    const expectErrorOnResume = common.expectsError();
+
+    // We might need one or two requests to trigger the `resumeSession` event
+    let triggeredOnFirstRequest = true;
+    const reqOptions = {
+      port: server.address().port,
+      host: '127.0.0.1',
+      path: '/',
+      method: 'GET',
+      agent,
+      rejectUnauthorized: false,
+    };
+    const req = https.request(reqOptions, (res) => {
+      triggeredOnFirstRequest = false;
+      res.resume();
+      res.on('end', () => {
+        const req = https.request(reqOptions, (res) => {
+          res.resume();
+        });
+        req.on('error', expectErrorOnResume);
+        req.end();
+      });
+    });
+
+    req.on('error', expectErrorOnResume);
+    req.end();
+
+    await promise;
+
+    if (!triggeredOnFirstRequest) {
+      // The second request would have received the error instead.
+      req.off('error', expectErrorOnResume);
+    }
+  });
+
   // Test 2: PSK server callback throwing should emit tlsClientError
   it('pskCallback throwing emits tlsClientError', async (t) => {
     const server = tls.createServer({