Skip to content

Commit eeef06a

Browse files
apapirovskiMylesBorins
authored andcommitted
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
1 parent f6a725e commit eeef06a

File tree

4 files changed

+68
-5
lines changed

4 files changed

+68
-5
lines changed

lib/_tls_wrap.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,8 @@ TLSSocket.prototype._init = function(socket, wrap) {
429429
var ssl = this._handle;
430430

431431
// lib/net.js expect this value to be non-zero if write hasn't been flushed
432-
// immediately
433-
// TODO(indutny): revise this solution, it might be 1 before handshake and
434-
// represent real writeQueueSize during regular writes.
432+
// immediately. After the handshake is done this will represent the actual
433+
// write queue size
435434
ssl.writeQueueSize = 1;
436435

437436
this.server = options.server;

src/tls_wrap.cc

+22-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ using v8::Exception;
4242
using v8::Function;
4343
using v8::FunctionCallbackInfo;
4444
using v8::FunctionTemplate;
45+
using v8::Integer;
4546
using v8::Local;
4647
using v8::Object;
4748
using v8::String;
@@ -297,6 +298,7 @@ void TLSWrap::EncOut() {
297298

298299
// No data to write
299300
if (BIO_pending(enc_out_) == 0) {
301+
UpdateWriteQueueSize();
300302
if (clear_in_->Length() == 0)
301303
InvokeQueued(0);
302304
return;
@@ -551,6 +553,18 @@ bool TLSWrap::IsClosing() {
551553
}
552554

553555

556+
uint32_t TLSWrap::UpdateWriteQueueSize(uint32_t write_queue_size) {
557+
HandleScope scope(env()->isolate());
558+
if (write_queue_size == 0)
559+
write_queue_size = BIO_pending(enc_out_);
560+
object()->Set(env()->context(),
561+
env()->write_queue_size_string(),
562+
Integer::NewFromUnsigned(env()->isolate(),
563+
write_queue_size)).FromJust();
564+
return write_queue_size;
565+
}
566+
567+
554568
int TLSWrap::ReadStart() {
555569
return stream_->ReadStart();
556570
}
@@ -591,8 +605,12 @@ int TLSWrap::DoWrite(WriteWrap* w,
591605
ClearOut();
592606
// However, if there is any data that should be written to the socket,
593607
// the callback should not be invoked immediately
594-
if (BIO_pending(enc_out_) == 0)
608+
if (BIO_pending(enc_out_) == 0) {
609+
// net.js expects writeQueueSize to be > 0 if the write isn't
610+
// immediately flushed
611+
UpdateWriteQueueSize(1);
595612
return stream_->DoWrite(w, bufs, count, send_handle);
613+
}
596614
}
597615

598616
// Queue callback to execute it on next tick
@@ -642,13 +660,15 @@ int TLSWrap::DoWrite(WriteWrap* w,
642660

643661
// Try writing data immediately
644662
EncOut();
663+
UpdateWriteQueueSize();
645664

646665
return 0;
647666
}
648667

649668

650669
void TLSWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) {
651-
// Intentionally empty
670+
TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
671+
wrap->UpdateWriteQueueSize();
652672
}
653673

654674

src/tls_wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ class TLSWrap : public AsyncWrap,
132132

133133
AsyncWrap* GetAsyncWrap() override;
134134
bool IsIPCPipe() override;
135+
uint32_t UpdateWriteQueueSize(uint32_t write_queue_size = 0);
135136

136137
// Resource implementation
137138
static void OnAfterWriteImpl(WriteWrap* w, void* ctx);

test/parallel/test-tls-buffersize.js

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
const assert = require('assert');
6+
const fixtures = require('../common/fixtures');
7+
const tls = require('tls');
8+
9+
const iter = 10;
10+
const overhead = 30;
11+
12+
const server = tls.createServer({
13+
key: fixtures.readKey('agent2-key.pem'),
14+
cert: fixtures.readKey('agent2-cert.pem')
15+
}, common.mustCall((socket) => {
16+
socket.on('readable', common.mustCallAtLeast(() => {
17+
socket.read();
18+
}, 1));
19+
20+
socket.on('end', common.mustCall(() => {
21+
server.close();
22+
}));
23+
}));
24+
25+
server.listen(0, common.mustCall(() => {
26+
const client = tls.connect({
27+
port: server.address().port,
28+
rejectUnauthorized: false
29+
}, common.mustCall(() => {
30+
assert.strictEqual(client.bufferSize, 0);
31+
32+
for (let i = 1; i < iter; i++) {
33+
client.write('a');
34+
assert.strictEqual(client.bufferSize, i + overhead);
35+
}
36+
37+
client.on('finish', common.mustCall(() => {
38+
assert.strictEqual(client.bufferSize, 0);
39+
}));
40+
41+
client.end();
42+
}));
43+
}));

0 commit comments

Comments
 (0)