This is not composable

[jnavila]

This lib can not be used with Lwt, such as Lwt_process.exec

Also,why use system instead of exec, as it would allow injection of code?

[smolkaj]

I don’t use Lwt or Async, so I’m not sure what the problem is. Maybe you could expand on that a bit? A pull request would also be very welcome.

[smolkaj]

Similarly for the second issue - how would one inject code and why is exec safer? Again, feel free to submit a pull request.

[jnavila]

About composability: it's just to allow to defer the actual process spawning and management to another framework, like the standard Unix module or Lwt or Async. Using Unix direct calls in asynchronous frameworks breaks the framework's ability to manage concurrency. So a functorized version would be great. I can provide a patch for it.

About injection, what is passed is a complete command line which will be sent to a shell interpreter which will expand the variables from the environment and manage command aliases (completely outside the scope of the original developer) and for which quoting must be managed (see https://caml.inria.fr/mantis/print_bug_page.php?bug_id=7672 for a laugh). This is to be compared to exec which is basically a call to C's exec and passes the arguments separately as-is. Both behaviors can be useful, but the second one does not suffer from external interferences.

I looked around a bit about how to handle sub-commands and files on different OSes, and Windows is clearly a pain. This is not as simple as it seems.

[smolkaj]

Thanks for the detailed explanation. That makes sense.

If you provide a patch to functorize things, I’m happy to merge it in and make a new release.

Regarding potential vulnerabilities, that’s definitely something we should keep on the radar. Since it sounds like there is no simple fix, we should at least open a separate issue to track this.

[jnavila]

OK. And for the laugh, sorry for quoting a bad URL. The good one is:
ocaml/ocaml#1492