Repository navigation
fix(shell_cmd): accept Windows paths in quoted arguments, and refuse what cmd.exe expands - #183
Merged
Merged
Conversation
…what cmd.exe expands eos_shell_arg_is_safe() refused the backslash everywhere. That is correct for a POSIX shell, which reads \ inside double quotes. On Windows it is the path separator, so after #156 the cargo/pip/npm backends refused every real path, and the MSVC leg failed test_backend_install with 0xc0000409. On Windows the rule now follows cmd.exe and the CRT: - a backslash is accepted, since it means nothing inside double quotes; - a trailing backslash is refused, because the CRT reads \" as a literal quote and the arguments after it would merge; - %VAR% and !VAR! are refused, because they expand even inside quotes. The POSIX rule is unchanged, and tests assert both. Host ctest passes 46/46 and test_shell_cmd 12/12. The Windows branch compiles with -D_WIN32 -fsyntax-only, but has not been run on Windows here; the MSVC CI leg is the check. Fixes #182
The Windows install ran 'copy *.bin & copy *.elf & copy *.hex' through cmd.exe and took its exit status from the last copy. A build that produced app.bin but no .hex was reported as a failed install, which is what test_backend_install hit on the MSVC leg once Windows paths were accepted (test_backend_install.c:231). It now does what the POSIX find does, in-process with Win32: refuse a missing build directory, create install_dir, and copy each *.bin, *.elf and *.hex that exists. A kind with no match is not an error. A failed copy or an over-long path is. The POSIX branch is unchanged. Checked here: host ctest 46/46, and a -D_WIN32 -Wall -Wextra syntax check against a minimal windows.h stub. Not run on Windows; the MSVC CI leg is the check. Refs #182
…as every backend does test_shell_cmd's test_every_backend_refuses_a_hostile_directory requires EOS_ERR_INVALID for a directory like '/tmp/build"; touch pwned; "'. The in-process Windows install never runs a shell, so the name is harmless there, but it returned EOS_ERR_NOT_FOUND. It now applies the same eos_shell_arg_is_safe() check first, so every backend keeps one contract. Refs #182
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #182
Master's Windows MSVC leg has been red since #156, because
eos_shell_arg_is_safe()refuses\and so every Windows path. It also blocks #179 and #181.\\\"as a literal quote)%!WSL host: ctest 46/46,
test_shell_cmd12/12. Windows: compiles with-D_WIN32 -fsyntax-only. The MSVC CI leg on this PR is the real check.