tls: _handle.readStart/readStop for CryptoStream

lib/http.js is using stream._handle.readStart/readStop to control data-flow coming out from underlying stream. If this methods are not present - data might be buffered regardless of whether it'll be read. see nodejs#4657
indutny · Feb 21, 2013 · d534c2b · d534c2b
1 parent bbcb8b3
commit d534c2b
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 3 deletions.
diff --git a/lib/tls.js b/lib/tls.js
@@ -242,6 +242,7 @@ function CryptoStream(pair, options) {
   this._pendingCallback = null;
   this._doneFlag = false;
   this._resumingSession = false;
+  this._reading = true;
   this._destroyed = false;
   this._ended = false;
   this._finished = false;
@@ -311,8 +312,7 @@ CryptoStream.prototype._write = function write(data, cb) {
   //
   // TODO(indutny): Remove magic number, use watermark based limits
   if (!this._resumingSession &&
-      (this !== this.pair.cleartext ||
+      this._opposite._internallyPendingBytes() < 128 * 1024) {
-      this.pair.encrypted._internallyPendingBytes() < 128 * 1024)) {
     // Write current buffer now
     var written;
     if (this === this.pair.cleartext) {
@@ -386,7 +386,7 @@ CryptoStream.prototype._read = function read(size, cb) {
   if (!this.pair.ssl) return cb(null, null);
 
   // Wait for session to be resumed
-  if (this._resumingSession) return cb(null, '');
+  if (this._resumingSession || !this._reading) return cb(null, '');
 
   var out;
   if (this === this.pair.cleartext) {
@@ -637,6 +637,18 @@ Object.defineProperty(CryptoStream.prototype, 'readyState', {
 
 function CleartextStream(pair, options) {
   CryptoStream.call(this, pair, options);
+
+  var self = this;
+  this._handle = {
+    readStop: function() {
+      self._reading = false;
+    },
+    readStart: function() {
+      if (self._reading) return;
+      self._reading = true;
+      self.read(0);
+    }
+  };
 }
 util.inherits(CleartextStream, CryptoStream);
 

diff --git a/test/simple/test-https-no-reader.js b/test/simple/test-https-no-reader.js
@@ -0,0 +1,68 @@
+// 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.
+
+if (!process.versions.openssl) {
+  console.error('Skipping because node compiled without OpenSSL.');
+  process.exit(0);
+}
+
+var common = require('../common');
+var assert = require('assert');
+var https = require('https');
+var Buffer = require('buffer').Buffer;
+var fs = require('fs');
+var path = require('path');
+
+var options = {
+  key: fs.readFileSync(path.join(common.fixturesDir, 'test_key.pem')),
+  cert: fs.readFileSync(path.join(common.fixturesDir, 'test_cert.pem'))
+};
+
+var buf = new Buffer(1024 * 1024);
+var sent = 0;
+var received = 0;
+
+var server = https.createServer(options, function(req, res) {
+  res.writeHead(200);
+  for (var i = 0; i < 50; i++) {
+    res.write(buf);
+  }
+  res.end();
+});
+
+server.listen(common.PORT, function() {
+  var resumed = false;
+  var req = https.request({
+    method: 'POST',
+    port: common.PORT,
+    rejectUnauthorized: false
+  }, function(res) {
+    res.read(0);
+
+    setTimeout(function() {
+      // Read buffer should be somewhere near high watermark
+      // (i.e. should not leak)
+      assert(res._readableState.length < 100 * 1024);
+      process.exit(0);
+    }, 5000);
+  });
+  req.end();
+});