http: always call response.write() callback

Ensure that the callback of `OutgoingMessage.prototype.write()` is
called even when writing empty chunks.

Fixes: https://github.com/nodejs/node/issues/22066

PR-URL: https://github.com/nodejs/node/pull/27709
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Luigi Pinca 2019-05-15 07:34:12 +02:00 committed by Anna Henningsen
parent 4fc0238a66
commit 0df581c307
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
4 changed files with 57 additions and 29 deletions

View File

@ -260,18 +260,6 @@ function _writeRaw(data, encoding, callback) {
// There might be pending data in the this.output buffer.
if (this.outputData.length) {
this._flushOutput(conn);
} else if (!data.length) {
if (typeof callback === 'function') {
// If the socket was set directly it won't be correctly initialized
// with an async_id_symbol.
// TODO(AndreasMadsen): @trevnorris suggested some more correct
// solutions in:
// https://github.com/nodejs/node/pull/14389/files#r128522202
defaultTriggerAsyncIdScope(conn[async_id_symbol],
process.nextTick,
callback);
}
return true;
}
// Directly write to socket.
return conn.write(data, encoding, callback);
@ -593,14 +581,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
['string', 'Buffer'], chunk);
}
// If we get an empty string or buffer, then just do nothing, and
// signal the user to keep writing.
if (chunk.length === 0) {
debug('received empty string or buffer and waiting for more input');
return true;
}
if (!fromEnd && msg.connection && !msg[kIsCorked]) {
msg.connection.cork();
msg[kIsCorked] = true;
@ -608,7 +588,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
}
var len, ret;
if (msg.chunkedEncoding) {
if (msg.chunkedEncoding && chunk.length !== 0) {
if (typeof chunk === 'string')
len = Buffer.byteLength(chunk, encoding);
else

View File

@ -10,19 +10,23 @@ const assert = require('assert');
// Fixes: https://github.com/nodejs/node/issues/14381
class Response extends OutgoingMessage {
constructor() {
super({ method: 'GET', httpVersionMajor: 1, httpVersionMinor: 1 });
}
_implicitHeader() {}
}
const res = new Response();
let firstChunk = true;
const ws = new Writable({
write: common.mustCall((chunk, encoding, callback) => {
assert(chunk.toString().match(/hello world/));
if (firstChunk) {
assert(chunk.toString().endsWith('hello world'));
firstChunk = false;
} else {
assert.strictEqual(chunk.length, 0);
}
setImmediate(callback);
})
}, 2)
});
res.socket = ws;

View File

@ -0,0 +1,37 @@
'use strict';
const common = require('../common');
// This test ensures that the callback of `OutgoingMessage.prototype.write()` is
// called also when writing empty chunks.
const assert = require('assert');
const http = require('http');
const stream = require('stream');
const expected = ['a', 'b', '', Buffer.alloc(0), 'c'];
const results = [];
const writable = new stream.Writable({
write(chunk, encoding, callback) {
setImmediate(callback);
}
});
const res = new http.ServerResponse({
method: 'GET',
httpVersionMajor: 1,
httpVersionMinor: 1
});
res.assignSocket(writable);
for (const chunk of expected) {
res.write(chunk, () => {
results.push(chunk);
});
}
res.end(common.mustCall(() => {
assert.deepStrictEqual(results, expected);
}));

View File

@ -15,11 +15,18 @@ const res = new ServerResponse({
httpVersionMinor: 1
});
let firstChunk = true;
const ws = new Writable({
write: common.mustCall((chunk, encoding, callback) => {
assert(chunk.toString().match(/hello world/));
if (firstChunk) {
assert(chunk.toString().endsWith('hello world'));
firstChunk = false;
} else {
assert.strictEqual(chunk.length, 0);
}
setImmediate(callback);
})
}, 2)
});
res.assignSocket(ws);