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.

Sign up for GitHub

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

tls: de-duplicate for TLSSocket methods #22142

Closed
wants to merge 1 commit into from
Closed

tls: de-duplicate for TLSSocket methods #22142

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 15 additions & 45 deletions lib/_tls_wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,6 @@ TLSSocket.prototype.setMaxSendFragment = function setMaxSendFragment(size) {
return this._handle.setMaxSendFragment(size) === 1;
};

TLSSocket.prototype.getTLSTicket = function getTLSTicket() {
return this._handle.getTLSTicket();
};

TLSSocket.prototype._handleTimeout = function() {
this._emitTLSError(new ERR_TLS_HANDSHAKE_TIMEOUT());
};
Expand Down Expand Up @@ -672,51 +668,25 @@ TLSSocket.prototype.getPeerCertificate = function(detailed) {
return null;
};

TLSSocket.prototype.getFinished = function() {
if (this._handle)
return this._handle.getFinished();
};

TLSSocket.prototype.getPeerFinished = function() {
if (this._handle)
return this._handle.getPeerFinished();
};

TLSSocket.prototype.getSession = function() {
if (this._handle) {
return this._handle.getSession();
}

return null;
};

TLSSocket.prototype.isSessionReused = function() {
if (this._handle) {
return this._handle.isSessionReused();
}

return null;
};

TLSSocket.prototype.getCipher = function(err) {
if (this._handle) {
return this._handle.getCurrentCipher();
} else {
// Proxy TLSSocket handle methods
function makeSocketMethodProxy(name) {
return function socketMethodProxy(...args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this show up in stack traces now instead of the actual proxied function name? I think having the actual function name in the stack trace would be more helpful.

Copy link
Member

@joyeecheung joyeecheung Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

V8 is able to infer the method names. This should show up as TLSSocket.socketMethodProxy <as getFinished> (previously TLSSocket.getFinished)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, like @joyeecheung is saying, if an error was thrown in getFinished it would now appear as at TLSSocket.socketMethodProxy [as getFinished]

if (this._handle)
return this._handle[name].apply(this._handle, args);
return null;
}
};

TLSSocket.prototype.getEphemeralKeyInfo = function() {
if (this._handle)
return this._handle.getEphemeralKeyInfo();
};
}

return null;
};
[
'getFinished', 'getPeerFinished', 'getSession', 'isSessionReused',
'getEphemeralKeyInfo', 'getProtocol', 'getTLSTicket'
].forEach((method) => {
TLSSocket.prototype[method] = makeSocketMethodProxy(method);
});

TLSSocket.prototype.getProtocol = function() {
TLSSocket.prototype.getCipher = function(err) {
if (this._handle)
return this._handle.getProtocol();

return this._handle.getCurrentCipher();
return null;
};

Expand Down