Skip to content

Commit

Permalink
tls: properly track writeQueueSize during writes
Browse files Browse the repository at this point in the history
Make writeQueueSize represent the actual size of the write queue
within the TLS socket. Add tls test to confirm that bufferSize
works as expected.

Fixes: nodejs#15005
Refs: nodejs#15006
  • Loading branch information
apapirovski committed Oct 18, 2017
1 parent 0a090d3 commit 4b94697
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 5 deletions.
5 changes: 2 additions & 3 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,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;
Expand Down
24 changes: 22 additions & 2 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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;
Expand Down Expand Up @@ -297,6 +298,7 @@ void TLSWrap::EncOut() {

// No data to write
if (BIO_pending(enc_out_) == 0) {
UpdateWriteQueueSize();
if (clear_in_->Length() == 0)
InvokeQueued(0);
return;
Expand Down Expand Up @@ -551,6 +553,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();
}
Expand Down Expand Up @@ -591,8 +605,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
Expand Down Expand Up @@ -642,13 +660,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();
}


Expand Down
1 change: 1 addition & 0 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,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);
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-tls-buffersize.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}));

0 comments on commit 4b94697

Please sign in to comment.