Skip to content

Commit

Permalink
fix: incorrect parsing of closeFDs option (#580)
Browse files Browse the repository at this point in the history 
* chore: fix order of argument parsing

* chore: add test
  • Loading branch information
@deepak1556
deepak1556 authored Feb 13, 2023
1 parent 42b5ce4 commit aff2e43
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 33 deletions.
66 changes: 33 additions & 33 deletions src/unix/pty.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,15 @@ NAN_METHOD(PtyFork) {
!info[10]->IsFunction() ||
!info[11]->IsString()) {
return Nan::ThrowError(
"Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, closeFDs, utf8, onexit, helperPath)");
"Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, closeFDs, onexit, helperPath)");
}

// file
Nan::Utf8String file(info[0]);

// args
v8::Local<v8::Array> argv_ = v8::Local<v8::Array>::Cast(info[1]);

// env
v8::Local<v8::Array> env_ = v8::Local<v8::Array>::Cast(info[2]);
int envc = env_->Length();
Expand All @@ -170,49 +173,22 @@ NAN_METHOD(PtyFork) {
// cwd
Nan::Utf8String cwd_(info[3]);

// uid / gid
int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust();
int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust();

// closeFDs
bool closeFDs = Nan::To<bool>(info[8]).FromJust();
bool explicitlyCloseFDs = closeFDs && !HAVE_POSIX_SPAWN_CLOEXEC_DEFAULT;

// helperPath
Nan::Utf8String helper_path_(info[11]);
char *helper_path = strdup(*helper_path_);

// args
v8::Local<v8::Array> argv_ = v8::Local<v8::Array>::Cast(info[1]);

const int EXTRA_ARGS = 6;
int argc = argv_->Length();
int argl = argc + EXTRA_ARGS + 1;
char **argv = new char*[argl];
argv[0] = strdup(helper_path);
argv[1] = strdup(*cwd_);
argv[2] = strdup(std::to_string(uid).c_str());
argv[3] = strdup(std::to_string(gid).c_str());
argv[4] = strdup(explicitlyCloseFDs ? "1": "0");
argv[5] = strdup(*file);
argv[argl - 1] = NULL;
for (int i = 0; i < argc; i++) {
Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked());
argv[i + EXTRA_ARGS] = strdup(*arg);
}

// size
struct winsize winp;
winp.ws_col = info[4]->IntegerValue(Nan::GetCurrentContext()).FromJust();
winp.ws_row = info[5]->IntegerValue(Nan::GetCurrentContext()).FromJust();
winp.ws_xpixel = 0;
winp.ws_ypixel = 0;

// uid / gid
int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust();
int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust();

// termios
struct termios t = termios();
struct termios *term = &t;
term->c_iflag = ICRNL | IXON | IXANY | IMAXBEL | BRKINT;
if (Nan::To<bool>(info[9]).FromJust()) {
if (Nan::To<bool>(info[8]).FromJust()) {
#if defined(IUTF8)
term->c_iflag |= IUTF8;
#endif
Expand Down Expand Up @@ -243,6 +219,30 @@ NAN_METHOD(PtyFork) {
term->c_cc[VSTATUS] = 20;
#endif

// closeFDs
bool closeFDs = Nan::To<bool>(info[9]).FromJust();
bool explicitlyCloseFDs = closeFDs && !HAVE_POSIX_SPAWN_CLOEXEC_DEFAULT;

// helperPath
Nan::Utf8String helper_path_(info[11]);
char *helper_path = strdup(*helper_path_);

const int EXTRA_ARGS = 6;
int argc = argv_->Length();
int argl = argc + EXTRA_ARGS + 1;
char **argv = new char*[argl];
argv[0] = strdup(helper_path);
argv[1] = strdup(*cwd_);
argv[2] = strdup(std::to_string(uid).c_str());
argv[3] = strdup(std::to_string(gid).c_str());
argv[4] = strdup(explicitlyCloseFDs ? "1": "0");
argv[5] = strdup(*file);
argv[argl - 1] = NULL;
for (int i = 0; i < argc; i++) {
Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked());
argv[i + EXTRA_ARGS] = strdup(*arg);
}

cfsetispeed(term, B38400);
cfsetospeed(term, B38400);

Expand Down
78 changes: 78 additions & 0 deletions src/unixTerminal.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as assert from 'assert';
import * as cp from 'child_process';
import * as path from 'path';
import * as tty from 'tty';
import * as fs from 'fs';
import { constants } from 'os';
import { pollUntil } from './testUtils.test';

Expand Down Expand Up @@ -280,6 +281,83 @@ if (process.platform !== 'win32') {
done();
}
});
if (process.platform === 'linux') {
it('should not close on exec when closeFDs is not defined', (done) => {
const data = `
var pty = require('./lib/index');
var ptyProcess = pty.spawn('node', ['-e', 'setTimeout(() => console.log("hello from terminal"), 300);']);
ptyProcess.on('data', function (data) {
console.log(data);
});
setTimeout(() => null, 500);
console.log('ready', ptyProcess.pid);
`;
const buffer: string[] = [];
const readFd = fs.openSync(FIXTURES_PATH, 'r');
const p = cp.spawn('node', ['-e', data], {
stdio: ['ignore', 'pipe', 'pipe', readFd]
});
let sub = '';
p.stdout!.on('data', (data) => {
if (!data.toString().indexOf('ready')) {
sub = data.toString().split(' ')[1].slice(0, -1);
try {
fs.statSync(`/proc/${sub}/fd/${readFd}`);
} catch (_) {
done('not reachable');
}
setTimeout(() => {
process.kill(parseInt(sub), 'SIGINT'); // SIGINT to child
p.kill('SIGINT'); // SIGINT to parent
}, 200);
} else {
buffer.push(data.toString().replace(/^\s+|\s+$/g, ''));
}
});
p.on('close', () => {
done();
});
});
it('should close on exec when closeFDs is true', (done) => {
const data = `
var pty = require('./lib/index');
var ptyProcess = pty.spawn('node', ['-e', 'setTimeout(() => console.log("hello from terminal"), 300);'], {
closeFDs: true
});
ptyProcess.on('data', function (data) {
console.log(data);
});
setTimeout(() => null, 500);
console.log('ready', ptyProcess.pid);
`;
const buffer: string[] = [];
const readFd = fs.openSync(FIXTURES_PATH, 'r');
const p = cp.spawn('node', ['-e', data], {
stdio: ['ignore', 'pipe', 'pipe', readFd]
});
let sub = '';
p.stdout!.on('data', (data) => {
if (!data.toString().indexOf('ready')) {
sub = data.toString().split(' ')[1].slice(0, -1);
try {
fs.statSync(`/proc/${sub}/fd/${readFd}`);
done('not reachable');
} catch (error) {
assert.notStrictEqual(error.message.indexOf('ENOENT'), -1);
}
setTimeout(() => {
process.kill(parseInt(sub), 'SIGINT'); // SIGINT to child
p.kill('SIGINT'); // SIGINT to parent
}, 200);
} else {
buffer.push(data.toString().replace(/^\s+|\s+$/g, ''));
}
});
p.on('close', () => {
done();
});
});
}
});
});
}

0 comments on commit aff2e43

Please sign in to comment.