Quotes are not handled correctly when child_process.spawn() parses args

[SetTrend]

require('child_process')
    .spawn('cmd', ['/c', 'node "my test/__args_out.js" b c d e f'])
    .stderr.addListener("data", data => {console.log(data.toString());});

fails because quotes don't seem to be recognized and parsed by child_process.spawn() as delimiters for parameters with spaces:

Error: Cannot find module 'd:\Documents\VS Code\"my test\__args_out.js"'
    at Function.Module._resolveFilename (module.js:326:15)
    at Function.Module._load (module.js:277:25)
    at Function.Module.runMain (module.js:430:10)
    at startup (node.js:141:18)
    at node.js:980:3

This happens on Windows platforms. I can't tell whether the above error occurs on other platforms, too.


The above code is taken from jake source.

[cjihrig]

What if you add the /s flag too? That's what child_process.exec() does.

See

node/lib/child_process.js

Lines 332 to 333 in 4501a28

	args = ['/s', '/c', '"' + command + '"'];
	options.windowsVerbatimArguments = true;

[SetTrend]

Good call!

I changed as you recommended. And now it works:

require('child_process')
    .spawn('cmd', ['/s', '/c', '"node "my test/__args_out.js" b c d e f"'])
    .stderr.addListener("data", data => {console.log(data.toString());});

It's required to surround the arguments list with additional quotes, too.

[MylesBorins]

@SetTrend did you mean to reopen this?

[SetTrend]

@thealphanerd :

Yes. I tested with different variations, but to no avail.

I'm currently debugging into what happens when the above code is spawned. Please give me five minutes and I'll come back with a report.

[SetTrend]

I have now run a number of tests and logged the actual process creation with SysInternals Process Monitor. Here's the result:

Executing above node code, using /s switch:

with surrounding quotes:

.spawn('cmd', ['/s/c', '"node "test/__args_out.js" b c d e f"'])

... results in this system log:

Command line:        cmd /s/c "\"node \"test/__args_out.js\" b c d e f\""
Current directory:   d:\Documents\Visual Studio-Projekte\GitHub\jake\
...
Query open:          D:\"node \"test        NAME INVALID 

👉 The quotes are getting escaped by node.

🔴

without surrounding quotes:

.spawn('cmd', ['/s/c', 'node "test/__args_out.js" b c d e f'])

... results in this system log:

Command line:        cmd /s/c "node \"test/__args_out.js\" b c d e f"
Current directory:   d:\Documents\Visual Studio-Projekte\GitHub\jake\
...
Command line:        node  \"test/__args_out.js\" b c d e f
...
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js"     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js".js     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js".json     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js".node     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js"\package.json     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js"\index.js     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js"\index.json     INVALID NAME
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\"test\__args_out.js"\index.node     INVALID NAME

👉 The quotes are getting escaped by node.

🔴

Executing above node code, NOT using /s switch:

.spawn('cmd', ['/c', '"node "test/__args_out.js" b c d e f"'])

.spawn('cmd', ['/c', 'node "test/__args_out.js" b c d e f'])

👉 Same results as above.

🔴

Running directly from the command line:

D:\Documents\Visual Studio-Projekte\GitHub\jake>cmd /s/c "node "test/__args_out.js" b c d e f"

results in this system log:

Command line:        cmd  /s/c "node "test/__args_out.js" b c d e f"
Current directory:   d:\Documents\Visual Studio-Projekte\GitHub\jake\
...
Create file:          D:\Documents\Visual Studio-Projekte\GitHub\jake\test\__args_out.js     SUCCESS

👉 Successful execution.

💚

Conclusion:

Apparently, node erroneously escapes quotes when spawning a child process (on Windows platform).

Proposed solution: node should not escape quotes when spawning a child process (on Windows platform).

[cjihrig]

The following works for me when command contains nested quotes and spaces.

const command = '"node "foo bar.js" a b c"';
const child = cp.spawn('cmd', ['/s', '/c', command], {windowsVerbatimArguments: true});

[SetTrend]

😮 Cool. What is that option doing? It doesn't seem to be documented. Let me check here whether I'll get it going, too ...

[SetTrend]

Thanks a lot for enlighten me on this option. It seems to work.

I may be focussed here ... but before I try to amend a public project like TypeScript (or potentionally any other project on GitHub lacking this option 😉), can someone please enlighten me on this undocumented option?

• What does it do on Windows platforms in particular?
• What does it do on Apple/Linux platforms?
• It seems that using windowsVerbatimArguments: true is reasonable for using it on any call to child_process.spawn(). What's the reason for not having windowsVerbatimArguments: true be the default behaviour of child_process.spawn() on Windows platforms?

[cjihrig]

What does it do on Windows platforms in particular?

Tells libuv that the command is a string that can be passed verbatim to cmd.

What does it do on Apple/Linux platforms?

It is silently ignored.

It seems that using windowsVerbatimArguments: true is reasonable for using it on any call to child_process.spawn(). What's the reason for not having windowsVerbatimArguments: true be the default behaviour of child_process.spawn() on Windows platforms?

I'm not sure about that. Probably because it isn't needed in most cases. exec() turns it on by default. In the next stable release, spawn() will support a boolean shell option that will allow you to turn this functionality on, like it is currently done for exec().

[cjihrig]

@SetTrend can this be closed?

[SetTrend]

I'm not sure. Shouldn't this behaviour get documented for future use - for both, exec() and spawn()?

[cjihrig]

The shell option is documented, it just hasn't been in a release yet, so it doesn't show up on the site. Long ago, there was a PR to document windowsVerbatimArguments. Looks like it was closed because the author didn't want to sign the old CLA.

[SetTrend]

I comprehend. Will the documentation update get released then soon?

What would you suggest on how to proceed? I would very much appreciate to close this issue. On the other hand I would very much appreciate to get the documentation updated final so authors can rely on this feature/option without being required to refer back to this issue.

[cjihrig]

@SetTrend the state of the documentation in master is available here. If there is anything missing there that you think should be included, you could open a documentation PR. Otherwise, I'm inclined to close this.

[jasnell]

Closing given that there does not appear to be anything further to do (other than perhaps some better docs) /cc @nodejs/documentation