Skip to content

Commit 206ae31

Browse files
lpincatargos
authored andcommittedMay 20, 2019
http: always call response.write() callback
Ensure that the callback of `OutgoingMessage.prototype.write()` is called even when writing empty chunks. Fixes: #22066 PR-URL: #27709 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent bfb9356 commit 206ae31

4 files changed

+57
-29
lines changed
 

‎lib/_http_outgoing.js

+1-21
Original file line numberDiff line numberDiff line change
@@ -260,18 +260,6 @@ function _writeRaw(data, encoding, callback) {
260260
// There might be pending data in the this.output buffer.
261261
if (this.outputData.length) {
262262
this._flushOutput(conn);
263-
} else if (!data.length) {
264-
if (typeof callback === 'function') {
265-
// If the socket was set directly it won't be correctly initialized
266-
// with an async_id_symbol.
267-
// TODO(AndreasMadsen): @trevnorris suggested some more correct
268-
// solutions in:
269-
// https://github.com/nodejs/node/pull/14389/files#r128522202
270-
defaultTriggerAsyncIdScope(conn[async_id_symbol],
271-
process.nextTick,
272-
callback);
273-
}
274-
return true;
275263
}
276264
// Directly write to socket.
277265
return conn.write(data, encoding, callback);
@@ -593,22 +581,14 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
593581
['string', 'Buffer'], chunk);
594582
}
595583

596-
597-
// If we get an empty string or buffer, then just do nothing, and
598-
// signal the user to keep writing.
599-
if (chunk.length === 0) {
600-
debug('received empty string or buffer and waiting for more input');
601-
return true;
602-
}
603-
604584
if (!fromEnd && msg.connection && !msg[kIsCorked]) {
605585
msg.connection.cork();
606586
msg[kIsCorked] = true;
607587
process.nextTick(connectionCorkNT, msg, msg.connection);
608588
}
609589

610590
var len, ret;
611-
if (msg.chunkedEncoding) {
591+
if (msg.chunkedEncoding && chunk.length !== 0) {
612592
if (typeof chunk === 'string')
613593
len = Buffer.byteLength(chunk, encoding);
614594
else

‎test/parallel/test-http-outgoing-message-inheritance.js

+10-6
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,23 @@ const assert = require('assert');
1010
// Fixes: https://github.com/nodejs/node/issues/14381
1111

1212
class Response extends OutgoingMessage {
13-
constructor() {
14-
super({ method: 'GET', httpVersionMajor: 1, httpVersionMinor: 1 });
15-
}
16-
1713
_implicitHeader() {}
1814
}
1915

2016
const res = new Response();
17+
18+
let firstChunk = true;
19+
2120
const ws = new Writable({
2221
write: common.mustCall((chunk, encoding, callback) => {
23-
assert(chunk.toString().match(/hello world/));
22+
if (firstChunk) {
23+
assert(chunk.toString().endsWith('hello world'));
24+
firstChunk = false;
25+
} else {
26+
assert.strictEqual(chunk.length, 0);
27+
}
2428
setImmediate(callback);
25-
})
29+
}, 2)
2630
});
2731

2832
res.socket = ws;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
// This test ensures that the callback of `OutgoingMessage.prototype.write()` is
6+
// called also when writing empty chunks.
7+
8+
const assert = require('assert');
9+
const http = require('http');
10+
const stream = require('stream');
11+
12+
const expected = ['a', 'b', '', Buffer.alloc(0), 'c'];
13+
const results = [];
14+
15+
const writable = new stream.Writable({
16+
write(chunk, encoding, callback) {
17+
setImmediate(callback);
18+
}
19+
});
20+
21+
const res = new http.ServerResponse({
22+
method: 'GET',
23+
httpVersionMajor: 1,
24+
httpVersionMinor: 1
25+
});
26+
27+
res.assignSocket(writable);
28+
29+
for (const chunk of expected) {
30+
res.write(chunk, () => {
31+
results.push(chunk);
32+
});
33+
}
34+
35+
res.end(common.mustCall(() => {
36+
assert.deepStrictEqual(results, expected);
37+
}));

‎test/parallel/test-http-server-response-standalone.js

+9-2
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,18 @@ const res = new ServerResponse({
1515
httpVersionMinor: 1
1616
});
1717

18+
let firstChunk = true;
19+
1820
const ws = new Writable({
1921
write: common.mustCall((chunk, encoding, callback) => {
20-
assert(chunk.toString().match(/hello world/));
22+
if (firstChunk) {
23+
assert(chunk.toString().endsWith('hello world'));
24+
firstChunk = false;
25+
} else {
26+
assert.strictEqual(chunk.length, 0);
27+
}
2128
setImmediate(callback);
22-
})
29+
}, 2)
2330
});
2431

2532
res.assignSocket(ws);

0 commit comments

Comments
 (0)
Please sign in to comment.