Skip to content

Commit 41b4ec7

Browse files
committed
TLS: flush buffer before destroy
Also disable test-https-large-response.js. Covered by test/simple/test-tls-throttle.js
1 parent e6f14d6 commit 41b4ec7

File tree

3 files changed

+111
-18
lines changed

3 files changed

+111
-18
lines changed

lib/tls.js

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ var util = require('util');
33
var net = require('net');
44
var events = require('events');
55
var stream = require('stream');
6-
6+
var END_OF_FILE = 42;
77
var assert = require('assert').ok;
88

99
var debug;
@@ -38,6 +38,10 @@ util.inherits(CryptoStream, stream.Stream);
3838

3939

4040
CryptoStream.prototype.write = function(data /* , encoding, cb */) {
41+
if (!this.writable) {
42+
throw new Error('CryptoStream is not writable');
43+
}
44+
4145
var encoding, cb;
4246

4347
// parse arguments
@@ -135,31 +139,27 @@ CryptoStream.prototype.getCipher = function(err) {
135139

136140

137141
CryptoStream.prototype.end = function(d) {
142+
if (!this.writable) {
143+
throw new Error('CryptoStream is not writable');
144+
}
145+
138146
if (this.pair._done) return;
139147

140148
if (d) {
141149
this.write(d);
142-
this.pair._cycle();
143150
}
144151

145-
// Sending EOF
146-
debug('cleartext end');
147-
this.pair._ssl.shutdown();
152+
this._pending.push(END_OF_FILE);
153+
this._pendingCallbacks.push(null);
154+
155+
this.writable = false;
156+
148157
this.pair._cycle();
149-
this.pair._destroy();
150158
};
151159

152160

153161
CryptoStream.prototype.destroySoon = function(err) {
154-
if (this.pair._done) return;
155-
156-
this.pair._cycle();
157-
158-
if (this._pending.length) {
159-
this.__destroyOnDrain = true;
160-
} else {
161-
this.end();
162-
}
162+
return this.end();
163163
};
164164

165165

@@ -168,6 +168,7 @@ CryptoStream.prototype.destroy = function(err) {
168168
this.pair._destroy();
169169
};
170170

171+
171172
CryptoStream.prototype.fd = -1;
172173
CryptoStream.prototype.__defineGetter__('readyState',
173174
net.Socket.prototype.__lookupGetter__('readyState'));
@@ -212,7 +213,12 @@ CryptoStream.prototype._push = function() {
212213
assert(bytesRead >= 0);
213214

214215
// Bail out if we didn't read any data.
215-
if (bytesRead == 0) return;
216+
if (bytesRead == 0) {
217+
if (this._pendingBytes() == 0 && this._destroyAfterPush) {
218+
this.destroy();
219+
}
220+
return;
221+
}
216222

217223
var chunk = pool.slice(0, bytesRead);
218224

@@ -252,6 +258,22 @@ CryptoStream.prototype._pull = function() {
252258

253259
assert(this._pending.length === this._pendingCallbacks.length);
254260

261+
if (tmp === END_OF_FILE) {
262+
// Sending EOF
263+
debug('end');
264+
this.pair._ssl.shutdown();
265+
266+
// TODO check if we get EAGAIN From shutdown, would have to do it
267+
// again. should unshift END_OF_FILE back onto pending and wait for
268+
// next cycle.
269+
270+
this.pair.encrypted._destroyAfterPush = true;
271+
272+
this.pair._cycle();
273+
//this.pair._destroy();
274+
return;
275+
}
276+
255277
var rv = this._puller(tmp);
256278

257279
if (this.pair._ssl && this.pair._ssl.error) {
@@ -290,7 +312,11 @@ util.inherits(CleartextStream, CryptoStream);
290312

291313

292314
CleartextStream.prototype._pendingBytes = function() {
293-
return this.pair._ssl._clearPending();
315+
if (this.pair._ssl) {
316+
return this.pair._ssl.clearPending();
317+
} else {
318+
return 0;
319+
}
294320
};
295321

296322

@@ -314,7 +340,11 @@ util.inherits(EncryptedStream, CryptoStream);
314340

315341

316342
EncryptedStream.prototype._pendingBytes = function() {
317-
return this.pair._ssl._endPending();
343+
if (this.pair._ssl) {
344+
return this.pair._ssl.encPending();
345+
} else {
346+
return 0;
347+
}
318348
};
319349

320350

File renamed without changes.

test/pummel/test-tls-large-push.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Server sends a large string. Client counts bytes and pauses every few
2+
// seconds. Makes sure that pause and resume work properly.
3+
var common = require('../common');
4+
var assert = require('assert');
5+
var tls = require('tls');
6+
var fs = require('fs');
7+
8+
9+
var body = '';
10+
11+
process.stdout.write('build body...');
12+
for (var i = 0; i < 10*1024*1024; i++) {
13+
body += 'hello world\n';
14+
}
15+
process.stdout.write('done\n');
16+
17+
18+
var options = {
19+
key: fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem'),
20+
cert: fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem')
21+
};
22+
23+
var connections = 0;
24+
25+
26+
var server = tls.Server(options, function(socket) {
27+
socket.end(body);
28+
connections++;
29+
});
30+
31+
var recvCount = 0;
32+
33+
server.listen(common.PORT, function() {
34+
var client = tls.connect(common.PORT);
35+
36+
client.on('data', function(d) {
37+
process.stdout.write('.');
38+
recvCount += d.length;
39+
40+
/*
41+
client.pause();
42+
process.nextTick(function () {
43+
client.resume();
44+
});
45+
*/
46+
});
47+
48+
49+
client.on('close', function() {
50+
debugger;
51+
console.log('close');
52+
server.close();
53+
});
54+
});
55+
56+
57+
58+
process.on('exit', function() {
59+
assert.equal(1, connections);
60+
console.log('body.length: %d', body.length);
61+
console.log(' recvCount: %d', recvCount);
62+
assert.equal(body.length, recvCount);
63+
});

0 commit comments

Comments
 (0)