diff options
author | Anna Henningsen <anna@addaleax.net> | 2016-08-24 15:14:04 +0200 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2016-08-27 16:11:46 +0200 |
commit | 4863f6a1217d1805c4f1a98a32e7774a6379bf5a (patch) | |
tree | 9526a91e700ffd0af0b242d2053952c30ba26d26 | |
parent | 5bef38b627cd9522d31d1f3ceae6655a06081ed0 (diff) | |
download | android-node-v8-4863f6a1217d1805c4f1a98a32e7774a6379bf5a.tar.gz android-node-v8-4863f6a1217d1805c4f1a98a32e7774a6379bf5a.tar.bz2 android-node-v8-4863f6a1217d1805c4f1a98a32e7774a6379bf5a.zip |
net: make holding the buffer in memory more robust
Set the `req.buffer` property, which serves as a way of keeping
a `Buffer` alive that is being written to a stream, on the C++
side instead of the JS side.
This closes a hole where buffers that were temporarily created
in order to write strings with uncommon encodings (e.g. `hex`)
were passed to the native side without being set as `req.buffer`.
Fixes: https://github.com/nodejs/node/issues/8251
PR-URL: https://github.com/nodejs/node/pull/8252
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
-rw-r--r-- | lib/net.js | 1 | ||||
-rw-r--r-- | src/env.h | 1 | ||||
-rw-r--r-- | src/stream_base.cc | 1 | ||||
-rw-r--r-- | test/parallel/test-net-write-fully-async-buffer.js | 34 | ||||
-rw-r--r-- | test/parallel/test-net-write-fully-async-hex-string.js | 32 |
5 files changed, 68 insertions, 1 deletions
diff --git a/lib/net.js b/lib/net.js index a218ad17b4..298e7186aa 100644 --- a/lib/net.js +++ b/lib/net.js @@ -694,7 +694,6 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { } else { var enc; if (data instanceof Buffer) { - req.buffer = data; // Keep reference alive. enc = 'buffer'; } else { enc = encoding; @@ -69,6 +69,7 @@ namespace node { V(args_string, "args") \ V(async, "async") \ V(async_queue_string, "_asyncQueue") \ + V(buffer_string, "buffer") \ V(bytes_string, "bytes") \ V(bytes_parsed_string, "bytesParsed") \ V(bytes_read_string, "bytesRead") \ diff --git a/src/stream_base.cc b/src/stream_base.cc index 105c4ad458..585b84885c 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -227,6 +227,7 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) { err = DoWrite(req_wrap, bufs, count, nullptr); req_wrap_obj->Set(env->async(), True(env->isolate())); + req_wrap_obj->Set(env->buffer_string(), args[1]); if (err) req_wrap->Dispose(); diff --git a/test/parallel/test-net-write-fully-async-buffer.js b/test/parallel/test-net-write-fully-async-buffer.js new file mode 100644 index 0000000000..04fd1231fd --- /dev/null +++ b/test/parallel/test-net-write-fully-async-buffer.js @@ -0,0 +1,34 @@ +'use strict'; +// Flags: --expose-gc + +// Note: This is a variant of test-net-write-fully-async-hex-string.js. +// This always worked, but it seemed appropriate to add a test that checks the +// behaviour for Buffers, too. +const common = require('../common'); +const net = require('net'); + +const data = Buffer.alloc(1000000); + +const server = net.createServer(common.mustCall(function(conn) { + conn.resume(); +})).listen(0, common.mustCall(function() { + const conn = net.createConnection(this.address().port, common.mustCall(() => { + let count = 0; + + function writeLoop() { + if (count++ === 200) { + conn.destroy(); + server.close(); + return; + } + + while (conn.write(Buffer.from(data))); + global.gc(true); + // The buffer allocated above should still be alive. + } + + conn.on('drain', writeLoop); + + writeLoop(); + })); +})); diff --git a/test/parallel/test-net-write-fully-async-hex-string.js b/test/parallel/test-net-write-fully-async-hex-string.js new file mode 100644 index 0000000000..f3115d8d2f --- /dev/null +++ b/test/parallel/test-net-write-fully-async-hex-string.js @@ -0,0 +1,32 @@ +'use strict'; +// Flags: --expose-gc + +// Regression test for https://github.com/nodejs/node/issues/8251. +const common = require('../common'); +const net = require('net'); + +const data = Buffer.alloc(1000000).toString('hex'); + +const server = net.createServer(common.mustCall(function(conn) { + conn.resume(); +})).listen(0, common.mustCall(function() { + const conn = net.createConnection(this.address().port, common.mustCall(() => { + let count = 0; + + function writeLoop() { + if (count++ === 20) { + conn.destroy(); + server.close(); + return; + } + + while (conn.write(data, 'hex')); + global.gc(true); + // The buffer allocated inside the .write() call should still be alive. + } + + conn.on('drain', writeLoop); + + writeLoop(); + })); +})); |