Thanks for your initiative on this and the great work debugging this issue, 
@knight9999.

However, I stand by what I said in 
https://github.com/apache/cordova-lib/pull/622#issuecomment-404836911:
> IMHO this should happen in `superspawn` not in `plugman`. `plugman` should 
> have no obligation to worry whether fetch might use its arguments unescaped 
> in a shell.

Or as @oliversalzburg puts it in [CB-14166]:

> The solution is still to properly escape when running commands on the shell, 
> and to not escape when not running on the shell. Proper abstractions will 
> take care of that for you.
>
> In this scenario, the only problem is that part of the code (`fetch.js`) 
> decides that it already knows how the command is executed in the end, which 
> it does not. The additional escaping introduced in this place is a mistake. 
> It needs to be removed and `superspawn` needs to be taught how to properly 
> escape, or, as I have been saying continuously, it should be replaced with 
> something that works (`cross-spawn`, `execa`).

To sum it up:
- The current code that handles the version escaping is broken and needs to be 
removed, not patched.
- We need a properly working abstraction for our use case of "I want to run a 
command, without worrying about the platform I'm on". IMHO, the options to 
achieve that, ordered by the amount of work involved (ascending) are:
  - Patch `superspawn` to handle this case properly
  - Change `superspawn` to use `cross-spawn` instead of `child_process.spawn`
  - Get rid of `superspawn` in favor of `cross-spawn` or `execa`

Incidentally, these options are also ordered by my approval for them 
(ascending).


[CB-14166]: 
https://issues.apache.org/jira/browse/CB-14166?focusedCommentId=16605736&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16605736

[ Full content available at: https://github.com/apache/cordova-lib/pull/688 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to