-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
Avoid calling external processes through the system shell #10393
Avoid calling external processes through the system shell #10393
Conversation
This change allows Haxe to be used even if the system shell is unavailable. Certain ocaml functions which call external processes do so through the system shell. On systems where the shell is unavailable, this makes Haxe unusable. These problematic functions are `Unix.system`, the `Unix.open_process_*` family of functions, and `Sys.command`. Places where the shell was previously accessed: 1. When using the `-lib` argument to the compiler. - `Haxe.add_libs` (calls `haxelib path {libs}`) 2. When using the `-cmd` argument to the compiler. - `Haxe.run_command` (command is normally passed to the shell) 3. When generating code for certain targets. - `Gencpp.write_build_options` (calls `haxelib path hxcpp`) - `Gencpp.generate_source` (calls `haxelib run hxcpp ...`) - `Gencs.generate` (calls `haxelib run hxcs ...`) - `Genhl.generate` (calls `haxelib run hashlink ...`) - `Genjava.generate` (calls `haxelib run hxjava ...`) - `Genneko.generate` (calls `nekoc ...`) 4. By hl interpreter and eval, to check the system name. - `Hlinterp.load_native`, for `sys_string` (calls `uname` on unix) - `EvalStdLib.StdSys` for system name (calls `uname` on unix) Out of all of these, most don't need to be interpreted by the shell. All the calls to `haxelib`, `nekoc`, and `uname` are known in advance, and those processes can be started directly instead. The only one that actually needs to be interpreted by the shell is (2), using `-cmd`. As a replacement for the `Unix.open_process_*` family of functions, the Unix module provides `Unix.open_process_args_*` as well. This set of functions does not pass through the shell, but uses fork/exec/posix_spawn on unix and CreateProcess on Windows. Arguments are passed as an array of strings, and given that the shell is not involved, the strings should not be escaped like they would be if passed to the shell. The first argument in the array should be the name of the program. For the purpose of finding the full path, a simple path-searching function has been added in the new process_helper.ml. This path searcher does not function exactly as the built-in path-searching capabilities of system shells. Unfortunately, `Unix.open_process_args_*` is only available beginning with ocaml 4.08, so a backport that wraps around `Unix.create_process_env` has also been added in process_helper.ml.
In order to use |
I wonder whether this change would also solve the issue in #10297. Once the mac build is available, that would be good to test. |
Could you please resolve the conflict? I'll review this afterwards. |
I'm getting some warnings here:
The good news is that I can confirm that your change works with |
I don't have the ocaml environment set up that I used to make this change originally, so I'm using the CI to quickly see if it builds or not. On top of that, I have no previous experience with ocaml, so... please bear with me. |
No worries, if you create just one function that does something meaningful with the OCaml process API then you're already more successful in that regard than I've ever been. :P |
It looks like re-running the tests made the I'll need to set some time aside to get set back up with a Haxe dev environment on Linux to figure out what's going on with the others. |
Yes these tests are sometimes are bit fickle... |
I've pulled most of your changes in the linked PR. Currently it still just folds Thank you very much for this contribution! |
This change allows Haxe to be used even if the system shell is unavailable.
Certain ocaml functions which call external processes do so through the system
shell. On systems where the shell is unavailable, this makes Haxe unusable.
These problematic functions are
Unix.system
, theUnix.open_process_*
familyof functions, and
Sys.command
.Places where the shell was previously accessed:
-lib
argument to the compiler.Haxe.add_libs
(callshaxelib path {libs}
)-cmd
argument to the compiler.Haxe.run_command
(command is normally passed to the shell)Gencpp.write_build_options
(callshaxelib path hxcpp
)Gencpp.generate_source
(callshaxelib run hxcpp ...
)Gencs.generate
(callshaxelib run hxcs ...
)Genhl.generate
(callshaxelib run hashlink ...
)Genjava.generate
(callshaxelib run hxjava ...
)Genneko.generate
(callsnekoc ...
)Hlinterp.load_native
, forsys_string
(callsuname
on unix)EvalStdLib.StdSys
for system name (callsuname
on unix)Out of all of these, most don't need to be interpreted by the shell. All the
calls to
haxelib
,nekoc
, anduname
are known in advance, and thoseprocesses can be started directly instead. The only one that actually needs to
be interpreted by the shell is (2), using
-cmd
.As a replacement for the
Unix.open_process_*
family of functions, the Unixmodule provides
Unix.open_process_args_*
as well. This set of functions doesnot pass through the shell, but uses fork/exec/posix_spawn on unix and
CreateProcess on Windows. Arguments are passed as an array of strings, and
given that the shell is not involved, the strings should not be escaped like
they would be if passed to the shell. The first argument in the array should
be the name of the program.
Unfortunately,
Unix.open_process_args_*
is only available beginning withocaml 4.08, so a backport that wraps around
Unix.create_process_env
hasbeen added in the new process_helper.ml file.
For the purpose of finding the full path, a simple path-searching function has also
been added in process_helper.ml. This path searcher does not function exactly as
the built-in path-searching capabilities of system shells, but it should work similarly
to the built-in path searching functionality of the functions as of ocaml 4.12.
Testing that these changes work is pretty simple on Windows. Open a command prompt first, then in the registry add a DWORD with the value
1
atComputer\HKEY_CURRENT_USER\SOFTWARE\Policies\Microsoft\Windows\System\DisableCMD
. If a new command prompt is opened now, it will warn that you don't have access, but the existing command prompt will continue to function. In the existing command prompt, callhaxe -lib myhaxelib
and confirm that the output is something expected like "myhaxelib isn't installed."