Skip to content

TLSSocket.bufferSize is always +1 before and after write() #15005

Closed
@micooz

Description

@micooz
  • 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?

node/lib/_tls_wrap.js

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.

node/lib/net.js

Lines 465 to 471 in 3cf27f1

Object.defineProperty(Socket.prototype, 'bufferSize', {
get: function() {
if (this._handle) {
return this._handle.writeQueueSize + this._writableState.length;
}
}
});

node/lib/net.js

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

No one assigned

    Labels

    tlsIssues and PRs related to the tls subsystem.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions