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

Swap out child_spawn with cross-spawn to address windows jadx path space #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Leapward-Koex
Copy link

This PR replaces the NodeJS default child_spawn module with cross-spawn due to command given to child_process.spawn failing if it has a spaces in it nodejs/node#7367.

Why?

Cross-spawn automatically handles commands with spaces.
This issue could be resolved by enabling shell and quoting the command but then we'd have to be responsible for command and argument input sanitization.

I've tested that this resolves issue #193 for me on Windows, I've also briefly verified that decompiling an APK and setting up the default git repository still works on a Ubuntu and Catalina machine.

@Surendrajat
Copy link
Member

Thanks @Leapward-Koex for the PR.
I am concerned that node-cross-spawn seems to be unmaintained for almost 3 years now. There are issues that aren't being addressed such as error on apple M1(I haven't verified this however).

Looks like a simple change but can we solve this in any other way with child_process?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants