-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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.