Do not escape double quotes for command line arguments on Windows (#113)

* refactor: Remove unused `util::is_ready()` function

Commit ffd4c731a2c7c09f34e7b81a16e04bdf91fa973d introduced the
`util::is_ready()` function, which has never been used internally and is
not part of the public API.

This change removes the function.

* refactor: Guard `util::quote_argument()` with `#ifdef __USING_WINDOWS__`

The `util::quote_argument()` function is specific to Windows and is used
in code already guarded by `#ifdef __USING_WINDOWS__`.

* Do not escape double quotes for command line arguments on Windows

This change fixes the handling of double quotes and aligns the behavior
with Python's `Popen` class. For example:
```
>py -3
>>> import subprocess
>>> p = subprocess.Popen("cmd.exe /c dir \"C:\\Program Files\"", stdout=subprocess.PIPE, text=True)
>>> print(f"Captured stdout:\n{stdout}")
```

Currently, the same command line processed by the `quote_argument()`
function looks like `cmd.exe /c dir "\"C:\Program" "Files\""`, which is
broken.

With this change, it looks correct: `cmd.exe /c dir "C:\Program Files"`.

* Add `test_double_quotes` test

The new test ensures that no regressions are introduced in the future.
This commit is contained in:
Hennadii Stepanov 2025-04-24 17:24:41 +01:00 committed by GitHub
parent 98a88e2499
commit ed313971c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 9 deletions

View File

@ -182,12 +182,7 @@ using env_vector_t = std::vector<platform_char_t>;
//--------------------------------------------------------------------
namespace util
{
template <typename R>
inline bool is_ready(std::shared_future<R> const &f)
{
return f.wait_for(std::chrono::seconds(0)) == std::future_status::ready;
}
#ifdef __USING_WINDOWS__
inline void quote_argument(const std::wstring &argument, std::wstring &command_line,
bool force)
{
@ -198,7 +193,7 @@ namespace util
//
if (force == false && argument.empty() == false &&
argument.find_first_of(L" \t\n\v\"") == argument.npos) {
argument.find_first_of(L" \t\n\v") == argument.npos) {
command_line.append(argument);
}
else {
@ -248,7 +243,6 @@ namespace util
}
}
#ifdef __USING_WINDOWS__
inline std::string get_last_error(DWORD errorMessageID)
{
if (errorMessageID == 0)

View File

@ -1,4 +1,4 @@
set(test_names test_subprocess test_cat test_env test_err_redirection test_exception test_split test_main test_ret_code)
set(test_names test_subprocess test_cat test_double_quotes test_env test_err_redirection test_exception test_split test_main test_ret_code)
set(test_files env_script.sh write_err.sh write_err.txt)

View File

@ -0,0 +1,30 @@
#include <subprocess.hpp>
#include <cassert>
#include <string>
namespace sp = subprocess;
// JSON requires the use of double quotes (see: https://json.org/).
// This test verifies proper handling of them.
void test_double_quotes()
{
// A simple JSON object.
const std::string expected{"{\"name\": \"value\"}"};
#ifdef __USING_WINDOWS__
const std::string command{"cmd.exe /c echo "};
#else
const std::string command{"echo "};
#endif
auto p = sp::Popen(command + expected, sp::output{sp::PIPE});
const auto out = p.communicate().first;
std::string result{out.buf.begin(), out.buf.end()};
// The `echo` command appends a newline.
result.erase(result.find_last_not_of(" \n\r\t") + 1);
assert(result == expected);
}
int main() {
test_double_quotes();
return 0;
}