-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
Closed
Labels
tlsIssues and PRs related to the tls subsystem.Issues and PRs related to the tls subsystem.
Description
- Version: v8.4.0
- Platform: Linux 4.4.0-92-generic x86_64 GNU/Linux
- Subsystem: net,tls
reproduce:
const tls = require('tls');
const fs = require('fs');
const key = fs.readFileSync('key.pem');
const cert = fs.readFileSync('cert.pem');
const server = tls.createServer({key, cert}, (socket) => {
socket.pipe(socket);
socket.on('end', () => {
server.close();
});
});
server.listen(0, () => {
const client = tls.connect(server.address().port, {rejectUnauthorized: false}, () => {
console.log(client.bufferSize); // 1
client.write(Buffer.alloc(10));
console.log(client.bufferSize); // 11
client.end();
});
client.on('finish', () => {
console.log(client.bufferSize); // 1
});
});
bufferSize
is obviously wrong in this case, this affects users who rely on this property to make judgments, for example:
if (socket.bufferSize <= 0) {
// oops! never reach here...
}
why?
Lines 427 to 437 in 3cf27f1
TLSSocket.prototype._init = function(socket, wrap) { | |
var self = this; | |
var options = this._tlsOptions; | |
var ssl = this._handle; | |
// lib/net.js expect this value to be non-zero if write hasn't been flushed | |
// immediately | |
// TODO(indutny): revise this solution, it might be 1 before handshake and | |
// represent real writeQueueSize during regular writes. | |
ssl.writeQueueSize = 1; | |
It seems a legacy code here. After I searched the entire lib
, writeQueueSize
is only use for calculating bufferSize
and decide if should wait until write flushed.
Lines 465 to 471 in 3cf27f1
Object.defineProperty(Socket.prototype, 'bufferSize', { | |
get: function() { | |
if (this._handle) { | |
return this._handle.writeQueueSize + this._writableState.length; | |
} | |
} | |
}); |
Lines 768 to 773 in 3cf27f1
// If it was entirely flushed, we can write some more right now. | |
// However, if more is left in the queue, then wait until that clears. | |
if (req.async && this._handle.writeQueueSize !== 0) | |
req.cb = cb; | |
else | |
cb(); |
Simply remove writeQueueSize
works fine for me, and I also opened a PR to fix the problem, all local tests passed but I still not very sure whether it's a right approach, so please make a review.
Metadata
Metadata
Assignees
Labels
tlsIssues and PRs related to the tls subsystem.Issues and PRs related to the tls subsystem.