Skip to content
This repository has been archived by the owner on Jul 6, 2019. It is now read-only.

Windows: npx is not escaping paths correctly #84

Closed
zkat opened this issue Jul 15, 2017 · 10 comments
Closed

Windows: npx is not escaping paths correctly #84

zkat opened this issue Jul 15, 2017 · 10 comments
Labels

Comments

@zkat
Copy link
Owner

zkat commented Jul 15, 2017

Ref: #62 (comment)

I'm confused 'cause this should totally be the right way to escape things for child_process.exec. Note that C:\"Program Files"\... is there, which should be the way to escape paths on Windows.

I'm thinking it's possible that this sort of escaping only applies when you're dealing with the binary path itself, and arguments don't get processed the same way and thus need to be escaped as args?

@zkat zkat added the bug label Jul 15, 2017
@katemihalikova
Copy link
Collaborator

Right. What if npx just quotes whole args and all of them? "C:\Program Files\nodejs\node.exe" "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js" "-v" works just fine in cmd.exe.

@zkat
Copy link
Owner Author

zkat commented Jul 15, 2017

I think that's what needs to be done, yeah.

@simonua
Copy link
Contributor

simonua commented Jul 17, 2017

Thanks a lot for keeping on this! Pointing out the obvious but just so you know for certain what my setup looks like:

Node is located at C:\Program Files\nodejs\node.exe (spaces in path)
npm is located at C:\Users\[userid]\AppData\Roaming\npm\npm.cmd (no spaces in path)

@zkat
Copy link
Owner Author

zkat commented Jul 17, 2017

Let's see if #87 works now?

@katemihalikova
Copy link
Collaborator

It's working fine for me. Works in both cmd.exe and git bash.

Since my npm path doesn't contain spaces, I've tried with npx --npm="C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js" cowsay hi and it's also working fine.

@zkat zkat closed this as completed in 314e5eb Jul 17, 2017
@simonua
Copy link
Contributor

simonua commented Jul 18, 2017

Nice! =) Thank you kindly, @zkat and @katemihalikova!

image

@simonua
Copy link
Contributor

simonua commented Jul 18, 2017

@zkat, sorry, quick note for the back of your mind: Looks like it should say "installed 5 packages in 21.205s.", yes?

@zkat
Copy link
Owner Author

zkat commented Jul 19, 2017

lol probably, but adding a single word to that string involves updating 17 other translations, many to languages I don't even know how to touch 😂

@simonua
Copy link
Contributor

simonua commented Jul 19, 2017

And pluralization. =P

@scambier
Copy link

@aivo0 for information, I solved it by upgrading to node v8.12.0

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

No branches or pull requests

4 participants