No error was thrown when spawn command was existed but non-executable

Version:

10.2.0/11.12.0

Platform:

Darwin localhost 16.7.0 Darwin Kernel Version 16.7.0: Thu Jun 15 17:36:27 PDT 2017; root:xnu-3789.70.16~2/RELEASE_X86_64 x86_64

Details:

Suppose we have following test script and file notexe exist in the same folder. But notexe has no executable permission.

const spawn = require('child_process').spawn;
const path = require('path');

function test(name) {
    console.log('===', name);

    try {
        const child = spawn(name);
        console.log('spawn.returned - child.stdin', !!child.stdin);
    } catch (e) {
        console.log('spawn.error', e.message);
    }
}

// always spawn with child.stdin equals true, and crash with ENOENT
test('notexe');  // can't find in PATH
test('/notexe'); // can't find in root folder

// in Node 8.9.0, spawn throws EACCES and no crash
// in Node 10.2.0/11.12.0, spawn throws no error with child.stdin equals false, and crash
test('./notexe');
test(path.resolve(__dirname, './notexe'));

So, why did Node 10+ change spawn behaviors? On purpose or regressions?

IMO going from a throw to an emitted error is a good thing, since spawn is supposed to be async.

Let me show what I think is a real bug in Node.js.

With this script located in /tmp/child_process, and an empty, non-executable file /tmp/child_process/y:

'use strict';

const { spawn } = require('child_process');
const { existsSync } = require('fs');
const { resolve } = require('path');

function test(path) {
  const exists = existsSync(path);
  const child = spawn(path);
  const hasStdin = !!child.stdin;
  child.on('error', (e) => {
    console.log(`Path: "${path}".\nExists: ${exists}.\nHas stdin: ${hasStdin}\n\n`);
  });
}

test('x');
test('/tmp/child_process/x');
test('./x');
test('y');
test('/tmp/child_process/y');
test('./y');

Path: "x".
Exists: false.
Has stdin: true


Path: "/tmp/child_process/x".
Exists: false.
Has stdin: true


Path: "./x".
Exists: false.
Has stdin: true


Path: "y".
Exists: true.
Has stdin: true


Path: "/tmp/child_process/y".
Exists: true.
Has stdin: false


Path: "./y".
Exists: true.
Has stdin: false

We have a clear inconsistency. If the file does not exist, the object returned by spawn always has the stdin property. If the file exists but is not executable, stdin is only present if the path doesn't start with ./ or /.

/cc @nodejs/child_process

@bnoordhuis

@bnoordhuis What do you think?

So, why did Node 10+ change spawn behaviors? On purpose or regressions?

The behavior change comes from #19294, which began classifying EACCES as a runtime error (emitting vs. throwing).

If the file exists but is not executable, stdin is only present if the path doesn't start with ./ or /

I think this is more about the error that's occurring. The two cases where "has stdin" is false are both EACCES, while the rest are ENOENT.

ENOENT gets special treatment here. Since that comment was originally written, we've started handling UV_EAGAIN and UV_EACCES too, but they should be treated the same as UV_ENOENT in my opinion.

We can:

Remove the special handling of UV_ENOENT, and return all runtime errors. This is definitely a breaking change, as it makes one test fail. I prefer this option because that comment acknowledges its own silliness, and it's kind of wasteful to set up stdio for no reason. I'm willing to PR this change.
Include UV_EACCES and UV_EAGAIN in the special handling with UV_ENOENT. I didn't test this variation. It's also a breaking change, even if our test suite doesn't catch it.
Do something else, including nothing.

Actually, on second thought, we probably do want to setup stdio to prevent situations like nodejs/help#1769.

having stdio setup in all runtime error scenarios (though am unable to figure out what needs to be done in the code to meet this) looks to be the right way forward for me. That way, from programmer's perspective, they could meaningfully tap both stdio and error streams of the child without breaking the program.

child_process: setup stdio on error when possible

Navigation Menu

Repository navigation

No error was thrown when spawn command was existed but non-executable #26852

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

No error was thrown when spawn command was existed but non-executable #26852

Description

Activity

targos commented on Mar 25, 2019

targos commented on May 6, 2019

cjihrig commented on May 14, 2019

cjihrig commented on May 14, 2019

gireeshpunathil commented on May 20, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions