From 38abebcb6ff417faf431dcaa103bb7f173c4b3f7 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 20 Apr 2026 10:59:41 +0200 Subject: zencore: CreateProc stdin pipes + BuildArgV quote stripping (#983) Two related improvements to `CreateProc`: ### 1. Stdin pipe support - Adds `StdinPipeHandles` + `CreateStdinPipe` alongside the existing `StdoutPipeHandles`, letting callers feed data into a child process's stdin. - Platform-agnostic RAII (Windows `HANDLE` pair / POSIX `pipe()` fd pair) with the same semantics as the stdout pipe: the inherited end goes to the child, the non-inherited end stays with the parent, destructor closes both. - `CreateProcOptions` gains a `StdinPipe*` field. - On Windows, `CreateProcNormal` is reworked so stdin/stdout redirection handles all combinations (stdin + stdout, each alone, neither) uniformly. POSIX already supported arbitrary fd redirection and just needed to honor the new option. - `zentest-appstub` gains a `-stdin_echo` mode that reads stdin to EOF and echoes it back (switching to binary mode on Windows so CRLF translation doesn't mangle bytes). - `zenserver-test` gets a `server.process` / `stdin_pipe.*` test group that exercises launching a child with a stdin pipe, writing, closing the write end, and reading back the echoed data. ### 2. Shell-style quote stripping in `BuildArgV` - Callers that build a single command-line string for `CreateProc` commonly wrap spacey paths in double quotes (e.g. `--tracefile="$path"`). The old `BuildArgV` only used quotes to suppress space-splitting and left the characters in the resulting argv element, so the spawned process saw literal `--tracefile="..."` and the value parser failed to open the quoted path. - `BuildArgV` now compacts in place, dropping quote chars as it goes, matching shell semantics for paired double quotes. --- src/zenserver-test/process-tests.cpp | 136 +++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) (limited to 'src/zenserver-test/process-tests.cpp') diff --git a/src/zenserver-test/process-tests.cpp b/src/zenserver-test/process-tests.cpp index 3f6476810..16af1879a 100644 --- a/src/zenserver-test/process-tests.cpp +++ b/src/zenserver-test/process-tests.cpp @@ -13,6 +13,7 @@ # if ZEN_PLATFORM_WINDOWS # include # else +# include # include # endif @@ -51,6 +52,45 @@ ReadAllFromPipe(StdoutPipeHandles& Pipe) return Result; } +// Write all of @p Data to the write end of a StdinPipeHandles. Returns true on success. +static bool +WriteAllToPipe(StdinPipeHandles& Pipe, std::string_view Data) +{ +# if ZEN_PLATFORM_WINDOWS + const char* Ptr = Data.data(); + size_t Remaining = Data.size(); + while (Remaining > 0) + { + DWORD BytesWritten = 0; + if (!::WriteFile(Pipe.WriteHandle, Ptr, static_cast(Remaining), &BytesWritten, nullptr) || BytesWritten == 0) + { + return false; + } + Ptr += BytesWritten; + Remaining -= BytesWritten; + } + return true; +# else + const char* Ptr = Data.data(); + size_t Remaining = Data.size(); + while (Remaining > 0) + { + ssize_t BytesWritten = write(Pipe.WriteFd, Ptr, Remaining); + if (BytesWritten <= 0) + { + if (BytesWritten < 0 && errno == EINTR) + { + continue; + } + return false; + } + Ptr += BytesWritten; + Remaining -= static_cast(BytesWritten); + } + return true; +# endif +} + TEST_SUITE_BEGIN("server.process"); ////////////////////////////////////////////////////////////////////////// @@ -276,6 +316,102 @@ TEST_CASE("pipe.separate_stderr") ////////////////////////////////////////////////////////////////////////// +TEST_CASE("stdin_pipe.echo_round_trip") +{ + StdinPipeHandles StdinPipe; + StdoutPipeHandles StdoutPipe; + REQUIRE(CreateStdinPipe(StdinPipe)); + REQUIRE(CreateStdoutPipe(StdoutPipe)); + + const std::string Input = "hello-from-stdin\nsecond-line\n"; + std::filesystem::path AppStub = GetAppStubPath(); + std::string CommandLine = "zentest-appstub -stdin_echo"; + + CreateProcOptions Options; + Options.StdinPipe = &StdinPipe; + Options.StdoutPipe = &StdoutPipe; + + ProcessHandle Process(CreateProc(AppStub, CommandLine, Options)); + + // Close the read end (child side) so that when we finish writing and close the write end, + // the child sees EOF. Also close the stdout write end so our reads see EOF after the child exits. + StdinPipe.CloseReadEnd(); + StdoutPipe.CloseWriteEnd(); + + REQUIRE(WriteAllToPipe(StdinPipe, Input)); + StdinPipe.CloseWriteEnd(); // signals EOF to the child + + std::string Output = ReadAllFromPipe(StdoutPipe); + + Process.Wait(); + + CHECK(Output.find(Input) != std::string::npos); + CHECK_EQ(Process.GetExitCode(), 0); +} + +TEST_CASE("stdin_pipe.raii_cleanup") +{ + for (int i = 0; i < 100; ++i) + { + StdinPipeHandles Pipe; + REQUIRE(CreateStdinPipe(Pipe)); + } +} + +TEST_CASE("stdin_pipe.move_semantics") +{ + StdinPipeHandles Original; + REQUIRE(CreateStdinPipe(Original)); + + StdinPipeHandles Moved(std::move(Original)); + +# if ZEN_PLATFORM_WINDOWS + CHECK(Moved.ReadHandle != nullptr); + CHECK(Moved.WriteHandle != nullptr); + CHECK(Original.ReadHandle == nullptr); + CHECK(Original.WriteHandle == nullptr); +# else + CHECK(Moved.ReadFd >= 0); + CHECK(Moved.WriteFd >= 0); + CHECK(Original.ReadFd == -1); + CHECK(Original.WriteFd == -1); +# endif + + StdinPipeHandles Assigned; + Assigned = std::move(Moved); + +# if ZEN_PLATFORM_WINDOWS + CHECK(Assigned.ReadHandle != nullptr); + CHECK(Assigned.WriteHandle != nullptr); + CHECK(Moved.ReadHandle == nullptr); + CHECK(Moved.WriteHandle == nullptr); +# else + CHECK(Assigned.ReadFd >= 0); + CHECK(Assigned.WriteFd >= 0); + CHECK(Moved.ReadFd == -1); + CHECK(Moved.WriteFd == -1); +# endif +} + +TEST_CASE("stdin_pipe.close_is_idempotent") +{ + StdinPipeHandles Pipe; + REQUIRE(CreateStdinPipe(Pipe)); + + Pipe.Close(); + Pipe.Close(); + +# if ZEN_PLATFORM_WINDOWS + CHECK(Pipe.ReadHandle == nullptr); + CHECK(Pipe.WriteHandle == nullptr); +# else + CHECK(Pipe.ReadFd == -1); + CHECK(Pipe.WriteFd == -1); +# endif +} + +////////////////////////////////////////////////////////////////////////// + TEST_SUITE_END(); } // namespace zen::tests -- cgit v1.2.3