Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash with input option and absolute path without execute permissions #166

Closed
targos opened this issue Dec 20, 2018 · 8 comments · Fixed by #212
Closed

Crash with input option and absolute path without execute permissions #166

targos opened this issue Dec 20, 2018 · 8 comments · Fixed by #212
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Milestone

Comments

@targos
Copy link

targos commented Dec 20, 2018

Issuehunt badges

To reproduce, create /non-executable-file and don't give it execute permissions.

const execa = require('execa');
execa('/non-executable-file', { input: 'foo' });

It crashes here because stdin doesn't exist:

execa/index.js

Line 92 in bfc9a4e

spawned.stdin.end(input);

stroncium earned $30.00 by resolving this issue!

@IssueHuntBot
Copy link

@IssueHunt has funded $30.00 to this issue.


@chinesedfan
Copy link

chinesedfan commented Mar 21, 2019

@targos Do you remember detailed environments? Like Node.js/OS/execa version. I failed to reproduce with Node8&10/OSX 10.14/execa 1.0.0. It throws an error during childProcess.spawn, but is caught by the next line immediately. Function handleInput is not executed at all.

@targos targos changed the title Crash with input option and file without execute permissions Crash with input option and absolute path without execute permissions Mar 21, 2019
@targos
Copy link
Author

targos commented Mar 21, 2019

Node.js 11.12.0 / Linux (Fedora 29) / execa 1.0.0.

I just realized now that this only happens when the path to the file is absolute (it can be located anywhere).

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 21, 2019

I can reproduce:

$ touch test.sh
$ ls -l test.sh
-rw-r--r-- 1 ether ether 0 Mar 21 16:06 test.sh

Then:

const execa = require('execa')
execa('./test.sh', { input: 'foo' })

Output:

Thrown:
TypeError: Cannot read property 'end' of undefined
    at handleInput (/home/ehmicky/Desktop/execa/index.js:114:17)
    at execa (/home/ehmicky/Desktop/execa/index.js:387:2)

I am using the latest execa (1.0.0).
Node: 11.12.0
OS: Ubuntu 18.10.

@targos
Copy link
Author

targos commented Mar 21, 2019

Actually, it's not only when the path is absolute:
./test.sh: crash
/test.sh: crash
test.sh: reject (no crash)

@chinesedfan
Copy link

It's a change in Node 10+. I have submitted an issue, nodejs/node#26852. Let's waiting for response. And should we add a simple check for spawned.stdin first?

@ehmicky
Copy link
Collaborator

ehmicky commented May 6, 2019

PR #212 implements that simple check.

@issuehunt-oss issuehunt-oss bot added the 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt label May 7, 2019
@IssueHuntBot
Copy link

@sindresorhus has rewarded $27.00 to @stroncium. See it on IssueHunt

  • 💰 Total deposit: $30.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $3.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants