Description
- Version: v12.16.3+, v14.0.0+, v15.0.0+, master
- Platform: n/a
- Subsystem: stream
What steps will reproduce the bug?
The following code needs a private key and certificate for an HTTPS server.
const { readFileSync } = require('fs');
const tls = require('tls');
const https = require('https');
const net = require('net');
const { Duplex } = require('stream');
class CustomAgent extends https.Agent {
constructor() {
super();
}
createConnection(options, cb) {
const realSocket = net.createConnection(options);
const stream = new Duplex({
emitClose: false,
read(n) {
(function retry() {
const data = realSocket.read();
if (data === null)
return realSocket.once('readable', retry);
stream.push(data);
})();
},
write(chunk, enc, callback) {
realSocket.write(chunk, enc, callback);
},
});
realSocket.on('end', () => stream.push(null));
stream.on('end', () => {
console.log('stream end');
}).on('close', () => {
console.log('stream close');
});
return tls.connect({ ...options, socket: stream });
}
}
const httpsServer = https.createServer({
key: readFileSync('https_key.pem'),
cert: readFileSync('https_cert.pem'),
}, (req, res) => {
httpsServer.close();
res.end('hello world!');
});
httpsServer.listen(0, 'localhost', () => {
const agent = new CustomAgent();
https.get({
host: 'localhost',
port: httpsServer.address().port,
agent,
headers: { Connection: 'close' },
ca: readFileSync('https_cert.pem'),
}, (res) => {
res.resume();
});
});How often does it reproduce? Is there a required condition?
Every time.
What is the expected behavior?
"stream end" should be displayed
What do you see instead?
Nothing is displayed.
Additional information
This was originally discovered while adding tests for custom HTTP and HTTPS agents for the ssh2 module. I believe the key here is mainly using a custom stream for the socket passed to tls.connect(). However the regression only became evident once I started setting emitClose: false because my agent implementation was relying on the 'close' event being emitted to know when to close the underlying ssh connection and emitting 'close' is expected to be handled by ssh2 (because the protocol has an explicit close message separate from "EOF").
Bisecting reveals ed21d32 as the bad commit. What's happening is that when TLSSocket sees that the socket option is some custom object/stream and not a net.Socket instance, it wraps the stream with a JSStreamSocket. JSStreamSocket happens to have a readStop() implementation that simply pauses the socket. Since ed21d32 added a check for this method's existence, it now gets called when the TLS portion ends, which means the underlying/original stream now stays paused and will never emit 'end' like it used to.
Judging by the added code comments, it appears that this change was made to appease some error on Windows. I think the original error should be solved in a more compatible way.