(new TLSSocket(new net.Socket())).connect() fails silently.

@Trott

(EDIT by @Trott: Turns out this is a documentation bug. Labeling good-first-contribution and doc.)

"use strict";

const NetSocket = require("net").Socket;
const TlsSocket = require("tls").TLSSocket;

const tlsSocket = new TlsSocket(new NetSocket());

tlsSocket.once("connect", function doStartup () {
    console.log("The tls socket connected. Yay!");
});

tlsSocket.connect({port: 6697, host: "irc.freenode.net"});
console.log("Sent connect.");

The program ends immediately after the connect is called, telling me that the connection isn't started.

Note that if we don't wrap the net.Socket in a TLSSocket, then the connect works as expected.

Thanks for submitting this. I had some time to think since our last conversation, and it looks like this is not the way it should be used. Sorry for not figuring it out in first place!

Could you please try using just new TlsSocket() instead? It seems to be working for me.

I guess this issue may be treated as a documentation bug, because it is not completely clear how TLSSocket should be used.

@mscdex

Is the usage for upgrading a net.Socket to a TLSSocket after it's already started?

I'm looking at the code, and it does specifically take a net.Socket and wraps it and uses the wrapped handle.

Trying const tlsSocket = new TlsSocket(); I get an error message:

_tls_wrap.js:314
    handle = options.pipe ? new Pipe() : new TCP();
                    ^

TypeError: Cannot read property 'pipe' of undefined
    at TLSSocket._wrapHandle (_tls_wrap.js:314:21)
    at new TLSSocket (_tls_wrap.js:256:18)
    at Object.<anonymous> (/home/havvy/workspace/test/tls_wrap.js:10:19)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:475:10)
    at startup (node.js:118:18)
    at node.js:952:3

Trying const tlsSocket = new TlsSocket(undefined, {isServer: false, host: "irc.freenode.net", port: 6697});, it does succeed at connecting.

As per @mscdex, I also tested const tlsSocket = require("tls").connect({socket: new NetSocket(), isServer: false, host: "irc.freenode.net", port: 6667}) which also fails silently.

That said, const tlsSocket = require("tls").connect({isServer: false, host: "irc.freenode.net", port: 6697}) does work (removing the "socket" property from the config object).

@Havvy

@Havvy No, I meant something like this:

var Socket = require('net').Socket;
var tls = require('tls');
var sock = new Socket();
var secureSock = tls.connect({ socket: s }, function() {
  console.log("The tls socket connected. Yay!");
});
sock.connect({port: 6697, host: "irc.freenode.net"});

That's typically how you upgrade an existing socket, but if you're using TLS from the start, then just use tls.connect():

var tls = require('tls');
var secureSock = tls.connect({port: 6697, host: "irc.freenode.net"}, function() {
  console.log("The tls socket connected. Yay!");
  secureSock.write(...);
});

Ah, okay. So if you pass in a net.Socket, you start the net.Socket.

That's the missing piece of information.

So, based on that...

The TLSSocket constructor documentation should be updated to point that out.

Should calling TLSSocket.connect() throw an error if there's a wrapped net socket?

@Havvy

@Havvy, mind submitting a PR for that doc?

Hi all: Was this resolved in minervapanda's commit? If not, I'm happy to tackle it.

@VerteDinde

@VerteDinde no it wasn't. That commit never made it to the official repo.

@TimothyGu

@TimothyGu Cool, I'll make a PR now. :)

Fixing TLS documentation error in Issue nodejs#3963. See line 879 - 9…

@TimothyGu

@TimothyGu PR submitted! Please let me know if I need to make any changes and thanks for all of the work that you do. ✨

Is this still open?

@nikshepsvn

@nikshepsvn Judging from #14062 I think this should be closed now.

@sam-github

I think this can be closed, following discussion in #14062 (starting at #14062 (review)).

FWIW #14062 (comment) contains a long list of documentation things that are probably good first contributions if they're still applicable. @sam-github might be worth putting that into a separate issue.

EDIT: Didn't see @joyeecheung 's comment

Navigation Menu

Repository navigation

(new TLSSocket(new net.Socket())).connect() fails silently. #3963

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

(new TLSSocket(new net.Socket())).connect() fails silently. #3963

Description

Activity

indutny commented on Nov 22, 2015

indutny commented on Nov 22, 2015

Havvy commented on Nov 22, 2015

mscdex commented on Nov 22, 2015

Havvy commented on Nov 22, 2015

tflanagan commented on Nov 22, 2015

VerteDinde commented on Jun 30, 2017

TimothyGu commented on Jul 1, 2017

VerteDinde commented on Jul 1, 2017

VerteDinde commented on Jul 3, 2017

nikshepsvn commented on Oct 8, 2017

joyeecheung commented on Oct 9, 2017

gibfahn commented on Oct 9, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions