From 521dc2511fc9ba90c4441a193b2a732e66391970 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 5 Oct 2017 16:16:20 -0400 Subject: [PATCH] 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. Backport-PR-URL: https://github.com/nodejs/node/pull/16420 PR-URL: https://github.com/nodejs/node/pull/15791 Fixes: https://github.com/nodejs/node/issues/15005 Refs: https://github.com/nodejs/node/pull/15006 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Refael Ackermann Reviewed-By: Fedor Indutny --- lib/_tls_wrap.js | 5 ++-- src/tls_wrap.cc | 24 ++++++++++++++-- src/tls_wrap.h | 1 + test/parallel/test-tls-buffersize.js | 43 ++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-tls-buffersize.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index c435d792e0a2cc..c1ad58c0e87b4f 100644 --- 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 index a0a2232884a805..bc0b9711438a8d 100644 --- 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(ctx); + wrap->UpdateWriteQueueSize(); } diff --git a/src/tls_wrap.h b/src/tls_wrap.h index dbb1a0199e4e66..0a59c80688ef25 100644 --- 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 new file mode 100644 index 00000000000000..49848cd865aca5 --- /dev/null +++ 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(); + })); +}));