ev-print-operation, main: Use g_spawn_async with argv instead of cmdline string - #724
Closed
yuri-schmaltz wants to merge 2 commits into
Closed
ev-print-operation, main: Use g_spawn_async with argv instead of cmdline string#724yuri-schmaltz wants to merge 2 commits into
yuri-schmaltz wants to merge 2 commits into
Conversation
added 2 commits
July 28, 2026 00:07
…ring When launching the print previewer, the previous code built a command-line string with g_strdup_printf(), quoted the file paths with g_shell_quote(), and then fed that string to g_app_info_create_from_commandline(), which parses and re-splits the string. Although the file paths came from g_file_open_tmp() and the export job (so they are glib-controlled in practice), this pattern is fragile: any future change that lets a user-controlled value flow into print_settings_file or export->temp_file would re-introduce the same class of argument-splitter injection that was recently fixed in ev_spawn (commit 50052ea). Replace the cmdline-string approach with a direct g_spawn_async() call that uses an explicit argv[]. Each element is passed verbatim to the child, eliminating the parse/re-split step. The child inherits the parent's environment, so DISPLAY / Wayland socket are still propagated without an explicit GdkAppLaunchContext. This also removes the dependency on g_app_info_create_from_commandline() and the temporary GAppInfo / GdkAppLaunchContext objects.
The launch_previewer() helper in shell/main.c was building a command-line string with g_strdup_printf() / g_shell_quote() and then feeding it to g_app_info_create_from_commandline(). This is the same fragile pattern that was just replaced in ev-print-operation: the string is parsed and re-split by the helper, so any character that is meaningful to that parser (quotes, backslashes, whitespace) can change the meaning of the resulting argv. The two user-influenced arguments (print_settings, file_arguments) come from xreader's own command line, so the practical risk is limited to whatever the user / launching environment typed -- but the structural issue is identical to the print previewer case. Replace the cmdline-string path with a direct g_spawn_async() call that uses an explicit argv[] array built from the original option values, with no quoting or splitting involved.
Author
|
Closing: this fix will live in the author's personal fork (yuri-schmaltz/xreader) as part of a fork-only maintenance workflow. The branches |
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
1 participant
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.
Summary
Replace two sites that build a shell command-line string and feed it back to
g_app_info_create_from_commandline()with explicitargv[]arrays passed tog_spawn_async(). This is the same structural fix that the recentev_spawncommit (50052ea) partially addressed by addingg_shell_quote()to user-controlled values.Sites patched
libview/ev-print-operation.c--export_print_done()launches the print previewer (xreader-previewer) with a command string built from a temporary settings file and the export temp file. Both paths come from glib in practice (g_file_open_tmp+ the export job), but going throughg_app_info_create_from_commandline()means a future change that lets a user-controlled value flow in would re-introduce the argument-splitter injection that 50052ea just patched inev_spawn.shell/main.c--launch_previewer()re-spawnsxreader-previewerfrom the main xreader binary when invoked with--preview. Same string-build pattern; the two user-influenced arguments (print_settings,file_arguments) come from xreader's own command line, so practical risk is bounded by whatever the launching environment types -- but the structural issue is identical.What changes
g_app_info_create_from_commandline()/g_app_info_launch()->g_spawn_async(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, ...)g_shell_quote()/g_strdup_printf()of the cmdline -> directargv[]arrayGAppInfoandGdkAppLaunchContexttemporaries removed; child inherits parent env (DISPLAY / Wayland socket) without explicit launch contextBehavior preserved
xreader-previewer)Follow-up (separate PR if wanted)
shell/ev-application.c:279(ev_spawn) has the same string-build pattern, mitigated byg_shell_quote()in 50052ea. A future structural fix could replace the string with an argv[] array there too -- happy to send a follow-up PR if the maintainer is interested.Test plan
meson setup build && meson compile -C build-- clean buildninja -C build test-- existing Python test suite (test/testFileReloading.py, etc.) still passesDiff stat