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

Windows doesn't like single quotes #9

Open
seangenabe opened this issue Sep 9, 2015 · 10 comments
Open

Windows doesn't like single quotes #9

seangenabe opened this issue Sep 9, 2015 · 10 comments

Comments

@seangenabe
Copy link

Windows doesn't like single quotes to be used as argument "wrappers", it will try to include them literally in the command-line argument.

Example:
write-file

var FS = require('fs')
var Path = require('path')

var file = process.argv[2]
FS.writeFileSync(Path.join(__dirname, file), 'foo')

When I call node write-file 'hello.txt', it creates literally the file 'hello.txt', with quotes.

@xxorax
Copy link
Owner

xxorax commented Oct 10, 2015

This lib is unfortunately not compatible at all with the Windows shell.
Feel free to make a pull request. It will need complete rewrite of the function.

This was referenced Oct 13, 2015
@binki
Copy link

binki commented Nov 21, 2016

As there appears to be no interest in the pull requests, I'd like to note that others might be interested in using cross-spawn instead. It, like child_process.spawn(), accepts parameters and makes sure they are safely escaped—all the while working on Windows!

The reason one might want to use child_process.exec() instead of child_process.spawn() in the first place is that child_process.exec() succeeds more reliably at actually doing things on Windows (unlike child_process.spawn()). So one naturally looks for a shell escaping library to use child_process.exec() safely. And then finds that no such library that’s compatible with Windows/everything exists—because everybody else already moved on to cross-spawn.

@xxorax
Copy link
Owner

xxorax commented Jan 27, 2018

As just mentionned in my comment on the PR, this lib is not ready to support Windows shell.

In fact it support the most common linux shells (only): sh, bash, zsh.

Adding support for other shells (cmd.exe, powershell...etc) needs to define others functions or options to let the user set for what shell he want to escape.

I'm open to it, but it will be much more complicated.

@binki
Copy link

binki commented Jul 7, 2018

@xxorax May you at least update the README to indicate that this project is intended only for systems with POSIX shells and does not support Windows for now?

@ChuckJonas
Copy link

ChuckJonas commented Nov 1, 2018

can you please update the readme this so that it states which OS it's compatible for (so others don't waste time)

@thieman
Copy link

thieman commented Apr 13, 2019

As yet another person that just wasted time on this repo since I need to support Windows, I will echo what the others have asked for 😄

@mkg20001
Copy link

I've made an updated fork using @seangenabe's code

https://github.com/mkg20001/escape-it

Tests are currently broken for windows, since I don't know what the intended behavior is (it doesn't seem to escape ';')

Also, the api changed. Now it is escape(platform)(list, of, strings) (so you need to use the spread operator like so escape(platform)(...arrayOfStrings) to pass an array as argument)

Platform is assumed to be process.platform by default

@xmedeko
Copy link

xmedeko commented Jun 11, 2020

@binki
Copy link

binki commented Jun 11, 2020

@mkg20001 @xmedeko Please use cross-spawn instead of escaping parameters and interpolating shellouts yourselves. That style of executing programs is only asking for trouble/security issues/weird-hard-to-debug-brokenness.

@xmedeko
Copy link

xmedeko commented Jun 12, 2020

@binki I am well aware that the spawn or execFile is preferred over shell escaping, but for various reasons, I need shell escaping now. Maybe you can submit a PR to README.md with your comment.

fregante added a commit to fregante/ghat that referenced this issue Nov 12, 2020
POSIX-only solution, incompatible with Windows: xxorax/node-shell-escape#9

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

No branches or pull requests

7 participants