Skip to content

Commit

Permalink
http: always call response.write() callback
Browse files Browse the repository at this point in the history
Ensure that the callback of `OutgoingMessage.prototype.write()` is
called even when writing empty chunks.

Fixes: #22066

PR-URL: #27709
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lpinca authored and targos committed May 20, 2019
1 parent bfb9356 commit 206ae31
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 29 deletions.
22 changes: 1 addition & 21 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -593,22 +581,14 @@ 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;
process.nextTick(connectionCorkNT, msg, msg.connection);
}

var len, ret;
if (msg.chunkedEncoding) {
if (msg.chunkedEncoding && chunk.length !== 0) {
if (typeof chunk === 'string')
len = Buffer.byteLength(chunk, encoding);
else
Expand Down
16 changes: 10 additions & 6 deletions test/parallel/test-http-outgoing-message-inheritance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions test/parallel/test-http-outgoing-message-write-callback.js
Original file line number Diff line number Diff line change
@@ -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);
}));
11 changes: 9 additions & 2 deletions test/parallel/test-http-server-response-standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 206ae31

Please sign in to comment.