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.

diff --git a/lib/tls.js b/lib/tls.js
index 976ff46..a758c8e 100644 (file)
--- 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 (file)
index 0000000..e9096b2
--- /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();
+    });
+  };
+});