From 7196742852c9da5d29e2fb1e333bca9d21c8e205 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Wed, 11 Sep 2013 18:18:10 -0700 Subject: [PATCH] tls: don't push() incoming data when ondata is set Otherwise the data ends up "on the wire" twice, and switching between consuming the stream using `ondata` vs. `read()` would yield duplicate data, which was bad. --- lib/tls.js | 9 +++--- test/simple/test-tls-ondata.js | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 test/simple/test-tls-ondata.js diff --git a/lib/tls.js b/lib/tls.js index 976ff46352..a758c8e01c 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -505,16 +505,15 @@ CryptoStream.prototype._read = function read(size) { } else { // Give them requested data if (this.ondata) { - var self = this; this.ondata(pool, start, start + bytesRead); // Consume data automatically // simple/test-https-drain fails without it - process.nextTick(function() { - self.read(bytesRead); - }); + this.push(pool.slice(start, start + bytesRead)); + this.read(bytesRead); + } else { + this.push(pool.slice(start, start + bytesRead)); } - this.push(pool.slice(start, start + bytesRead)); } // Let users know that we've some internal data to read diff --git a/test/simple/test-tls-ondata.js b/test/simple/test-tls-ondata.js new file mode 100644 index 0000000000..e9096b2f0a --- /dev/null +++ b/test/simple/test-tls-ondata.js @@ -0,0 +1,54 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var tls = require('tls'); +var fs = require('fs'); + +var options = { + key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), + cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') +}; + +var server = tls.Server(options, function(socket) { + socket.end('hello world'); +}); + +server.listen(common.PORT, function() { + var client = tls.connect({ + port: common.PORT, + rejectUnauthorized: false + }); + // test that setting the `ondata` function *prevents* data from + // being pushed to the streams2 interface of the socket + client.ondata = function (buf, start, length) { + var b = buf.slice(start, length); + process.nextTick(function () { + var b2 = client.read(); + if (b2) { + assert.notEqual(b.toString(), b2.toString()); + } + client.destroy(); + server.close(); + }); + }; +});