tls: properly track writeQueueSize during writes

Make writeQueueSize represent the actual size of the write queue within the TLS socket. Add tls test to confirm that bufferSize works as expected. PR-URL: #15791 Fixes: #15005 Refs: #15006 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
nodejs · apapirovski · Oct 5, 2017 · Oct 12, 2017 · Oct 25, 2017 · Dec 12, 2017
commit 7533f59bd288e65247508940194ca97f3e93bd67
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -401,9 +401,8 @@ TLSSocket.prototype._init = function(socket, wrap) {
   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.
+  // immediately. After the handshake is done this will represent the actual
+  // write queue size
   ssl.writeQueueSize = 1;
 
   this.server = options.server;

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -21,6 +21,7 @@ using v8::Exception;
 using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
+using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::String;
@@ -276,6 +277,7 @@ void TLSWrap::EncOut() {
 
   // No data to write
   if (BIO_pending(enc_out_) == 0) {
+    UpdateWriteQueueSize();
     if (clear_in_->Length() == 0)
       InvokeQueued(0);
     return;
@@ -530,6 +532,18 @@ bool TLSWrap::IsClosing() {
 }
 
 
+uint32_t TLSWrap::UpdateWriteQueueSize(uint32_t write_queue_size) {
+  HandleScope scope(env()->isolate());
+  if (write_queue_size == 0)
+    write_queue_size = BIO_pending(enc_out_);
+  object()->Set(env()->context(),
+                env()->write_queue_size_string(),
+                Integer::NewFromUnsigned(env()->isolate(),
+                                         write_queue_size)).FromJust();
+  return write_queue_size;
+}
+
+
 int TLSWrap::ReadStart() {
   return stream_->ReadStart();
 }
@@ -570,8 +584,12 @@ int TLSWrap::DoWrite(WriteWrap* w,
     ClearOut();
     // However, if there is any data that should be written to the socket,
     // the callback should not be invoked immediately
-    if (BIO_pending(enc_out_) == 0)
+    if (BIO_pending(enc_out_) == 0) {
+      // net.js expects writeQueueSize to be > 0 if the write isn't
+      // immediately flushed
+      UpdateWriteQueueSize(1);
       return stream_->DoWrite(w, bufs, count, send_handle);
+    }
   }
 
   // Queue callback to execute it on next tick
@@ -621,13 +639,15 @@ int TLSWrap::DoWrite(WriteWrap* w,
 
   // Try writing data immediately
   EncOut();
+  UpdateWriteQueueSize();
 
   return 0;
 }
 
 
 void TLSWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) {
-  // Intentionally empty
+  TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
+  wrap->UpdateWriteQueueSize();
 }
 
 

diff --git a/src/tls_wrap.h b/src/tls_wrap.h
@@ -111,6 +111,7 @@ class TLSWrap : public AsyncWrap,
 
   AsyncWrap* GetAsyncWrap() override;
   bool IsIPCPipe() override;
+  uint32_t UpdateWriteQueueSize(uint32_t write_queue_size = 0);
 
   // Resource implementation
   static void OnAfterWriteImpl(WriteWrap* w, void* ctx);

diff --git a/test/parallel/test-tls-buffersize.js b/test/parallel/test-tls-buffersize.js
@@ -0,0 +1,43 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const fixtures = require('../common/fixtures');
+const tls = require('tls');
+
+const iter = 10;
+const overhead = 30;
+
+const server = tls.createServer({
+  key: fixtures.readKey('agent2-key.pem'),
+  cert: fixtures.readKey('agent2-cert.pem')
+}, common.mustCall((socket) => {
+  socket.on('readable', common.mustCallAtLeast(() => {
+    socket.read();
+  }, 1));
+
+  socket.on('end', common.mustCall(() => {
+    server.close();
+  }));
+}));
+
+server.listen(0, common.mustCall(() => {
+  const client = tls.connect({
+    port: server.address().port,
+    rejectUnauthorized: false
+  }, common.mustCall(() => {
+    assert.strictEqual(client.bufferSize, 0);
+
+    for (let i = 1; i < iter; i++) {
+      client.write('a');
+      assert.strictEqual(client.bufferSize, i + overhead);
+    }
+
+    client.on('finish', common.mustCall(() => {
+      assert.strictEqual(client.bufferSize, 0);
+    }));
+
+    client.end();
+  }));
+}));