Skip to content

Commit

Permalink
net: handle allowHalfOpen before calling super ctor
Browse files Browse the repository at this point in the history
If the `allowHalfOpen` option is not provided, use its default value
before calling the `Duplex` constructor. This allows the no-half-open
enforcer to be properly added.

Fixes: nodejs#19764
  • Loading branch information
lpinca committed Apr 3, 2018
1 parent fb938e4 commit 36b5e01
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
12 changes: 4 additions & 8 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,11 @@ function Socket(options) {
this[kLastWriteQueueSize] = 0;

if (typeof options === 'number')
options = { fd: options }; // Legacy interface.
else if (options === undefined)
options = {};
options = { allowHalfOpen: false, fd: options }; // Legacy interface.
else
options = util._extend({}, options);

options.allowHalfOpen = Boolean(options.allowHalfOpen);
stream.Duplex.call(this, options);

if (options.handle) {
Expand All @@ -236,8 +237,6 @@ function Socket(options) {
this._writev = null;
this._write = makeSyncWrite(fd);
}
this.readable = options.readable !== false;
this.writable = options.writable !== false;
} else {
// these will be set once there is a connection
this.readable = this.writable = false;
Expand All @@ -255,9 +254,6 @@ function Socket(options) {
// handle strings directly
this._writableState.decodeStrings = false;

// default to *not* allowing half open sockets
this.allowHalfOpen = options && options.allowHalfOpen || false;

// if we have a handle, then start the flow of data into the
// buffer. if not, then this will happen when we connect
if (this._handle && options.readable !== false) {
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-child-process-stdout-stderr-finish.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';
const { mustCall } = require('../common');
const { path } = require('../common/fixtures');

// This test ensures that the `'finish'` event is emitted on the child process's
// `stdout` and `stderr` when using `child_process.spawn()`.
// This is a regression test for https://github.com/nodejs/node/issues/19764.

const { spawn } = require('child_process');

const child = spawn(process.argv[0], [path('print-chars.js'), 10]);

child.stdout.on('finish', mustCall());
child.stdout.on('end', mustCall());
child.stdout.resume();

child.stderr.on('finish', mustCall());
child.stderr.on('end', mustCall());
child.stderr.resume();

0 comments on commit 36b5e01

Please sign in to comment.