Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net,tls: pass a valid socket on the tlsClientError event #44021

Merged
merged 1 commit into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/_tls_wrap.js
Expand Up @@ -423,6 +423,10 @@ function onerror(err) {
if (!owner._secureEstablished) {
// When handshake fails control is not yet released,
// so self._tlsError will return null instead of actual error

// Set closing the socket after emitting an event since the socket needs to
// be accessible when the `tlsClientError` event is emmited.
owner._closeAfterHandlingError = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with TLS connections over Duplex streams that are not net.Sockets?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that parts, I entirely relied on test cases. Could you walk me through any example to verify it?

owner.destroy(err);
} else if (owner._tlsOptions?.isServer &&
owner._rejectUnauthorized &&
Expand Down
33 changes: 26 additions & 7 deletions lib/net.js
Expand Up @@ -102,6 +102,7 @@ const {
uvExceptionWithHostPort,
} = require('internal/errors');
const { isUint8Array } = require('internal/util/types');
const { queueMicrotask } = require('internal/process/task_queues');
const {
validateAbortSignal,
validateFunction,
Expand Down Expand Up @@ -289,6 +290,19 @@ function initSocketHandle(self) {
}
}

function closeSocketHandle(self, isException, isCleanupPending = false) {
if (self._handle) {
self._handle.close(() => {
debug('emit close');
self.emit('close', isException);
if (isCleanupPending) {
self._handle.onread = noop;
self._handle = null;
self._sockname = null;
}
});
}
}

const kBytesRead = Symbol('kBytesRead');
const kBytesWritten = Symbol('kBytesWritten');
Expand Down Expand Up @@ -337,6 +351,7 @@ function Socket(options) {
this[kBuffer] = null;
this[kBufferCb] = null;
this[kBufferGen] = null;
this._closeAfterHandlingError = false;

if (typeof options === 'number')
options = { fd: options }; // Legacy interface.
Expand Down Expand Up @@ -755,15 +770,19 @@ Socket.prototype._destroy = function(exception, cb) {
});
if (err)
this.emit('error', errnoException(err, 'reset'));
} else if (this._closeAfterHandlingError) {
// Enqueue closing the socket as a microtask, so that the socket can be
// accessible when an `error` event is handled in the `next tick queue`.
queueMicrotask(() => closeSocketHandle(this, isException, true));
} else {
this._handle.close(() => {
debug('emit close');
this.emit('close', isException);
});
closeSocketHandle(this, isException);
}

if (!this._closeAfterHandlingError) {
this._handle.onread = noop;
this._handle = null;
this._sockname = null;
}
this._handle.onread = noop;
this._handle = null;
this._sockname = null;
cb(exception);
} else {
cb(exception);
Expand Down
31 changes: 31 additions & 0 deletions test/internet/test-https-issue-43963.js
@@ -0,0 +1,31 @@
'use strict';
const common = require('../common');
const https = require('node:https');
const assert = require('node:assert');

const server = https.createServer();

server.on(
'tlsClientError',
common.mustCall((exception, tlsSocket) => {
assert.strictEqual(exception !== undefined, true);
assert.strictEqual(Object.keys(tlsSocket.address()).length !== 0, true);
assert.strictEqual(tlsSocket.localAddress !== undefined, true);
assert.strictEqual(tlsSocket.localPort !== undefined, true);
assert.strictEqual(tlsSocket.remoteAddress !== undefined, true);
assert.strictEqual(tlsSocket.remoteFamily !== undefined, true);
assert.strictEqual(tlsSocket.remotePort !== undefined, true);
}),
);

server.listen(0, () => {
const req = https.request({
hostname: '127.0.0.1',
port: server.address().port,
});
req.on(
'error',
common.mustCall(() => server.close()),
);
req.end();
});