[212_2703] Fix gs path quoting for paths with spaces on macOS#2859
[212_2703] Fix gs path quoting for paths with spaces on macOS#2859dorodb-web22 wants to merge 1 commit intoMoganLab:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Ghostscript command-line construction so PS/EPS rendering works on macOS when temp paths contain spaces (e.g., user home paths like .../John Doe/...).
Changes:
- Replace
sys_concretize(url)withraw_quote(concretize(url))for Ghostscript file paths ings_utilities.cpp. - Update the Ghostscript PDF size probing path to quote
$TEXMACS_PATH/misc/convert/pdf_info.ps. - Add a developer note (
devel/212_2703.md) describing the issue, rationale, and test steps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/Plugins/Ghostscript/gs_utilities.cpp |
Updates how file paths are embedded in Ghostscript command strings to handle spaces. |
devel/212_2703.md |
Documents reproduction, fix rationale, and manual test procedure for #2703. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // real eps pages with proper bounding boxes have been recognized | ||
| // before this and will have their BoundingBox respected | ||
| cmd << sys_concretize (image); | ||
| cmd << raw_quote (concretize (image)); |
There was a problem hiding this comment.
raw_quote(concretize(image)) is not shell-safe on macOS/Linux: raw_quote just wraps in double quotes and does not escape $, backticks, or ". Since lolly::system::call uses wordexp(..., flags=0) (command substitution enabled), a crafted filename could trigger expansion/command execution. For positional file args, sys_concretize(image) already handles spaces via backslash-escaping while also escaping metacharacters; consider keeping sys_concretize here and only changing the truly GS-option-value cases, or introduce a dedicated safe quoting/escaping helper for wordexp.
| cmd << raw_quote (concretize (image)); | |
| cmd << sys_concretize (image); |
| cmd << "-dQUIET -dNOPAUSE -dBATCH -dSAFER "; | ||
| cmd << "-sDEVICE=" << eps_device (); | ||
| cmd << " -sOutputFile=" << sys_concretize (eps) << " "; | ||
| cmd << " -sOutputFile=" << raw_quote (concretize (eps)) << " "; | ||
| if (suffix (image) == "pdf") { |
There was a problem hiding this comment.
raw_quote(concretize(eps)) does not escape $/backticks/" and is therefore not safe to embed in a command string that is later parsed by wordexp (used by lolly::system::call on macOS/Linux). This is a potential command-injection vector via crafted filenames/paths. Please use a shell-safe quoting/escaping routine for paths here (or avoid string-based command parsing altogether).
| cmd << " -sOutputFile=" << raw_quote (concretize (ps)) << " "; | ||
| cmd << raw_quote (concretize (doc)); | ||
| cmd << " -c \"[ /Title (" << as_string (tail (ps)) << ") /DOCINFO pdfmark\" "; |
There was a problem hiding this comment.
The new raw_quote(concretize(ps/doc)) usage is not safe with the current subprocess implementation on macOS/Linux: raw_quote does not escape $, backticks, or embedded quotes, and lolly::system::call uses wordexp with command substitution enabled. A malicious filename could be expanded/executed. Please switch to a shell-safe quoting approach (or pass argv without wordexp).
| string cmd= gs_prefix (); | ||
| cmd << "-q -sDEVICE=x11alpha -dBATCH -dNOPAUSE -dSAFER -dNOEPS "; | ||
| cmd << sys_concretize (image); | ||
| cmd << raw_quote (concretize (image)); | ||
| lolly::system::call (cmd); |
There was a problem hiding this comment.
raw_quote(concretize(image)) is not a safe way to quote a path for execution on macOS/Linux because raw_quote does not escape $/backticks/", and lolly::system::call uses wordexp (command substitution enabled). This allows command substitution if an image path contains $(...)/backticks. Please use a shell-safe quoting/escaping function for paths passed to subprocesses.
|
@MoonL79 I'm inclined to keep the current approach because:
That said, if u would prefer a more conservative approach, I can limit raw_quote to |
|
Thank you for your work! I will get back to you today. |
|
Thanks for the clarification. I agree the space-in-path bug is real and your change fixes that behavior for GS option values. My concern is the security regression: raw_quote(concretize(...)) only adds double quotes, and on macOS/Linux our command path still goes through wordexp(..., 0), so Could you please update this to a shell-safe + GS-safe approach instead of plain raw_quote? At minimum, please add/cover cases for paths containing: space, $, backticks, ", ', and , for both standalone args and -sOutputFile= / -sFile= / --permit-file-read=forms. |
133bcce to
fbe2754
Compare
|
@MoonL79 Thanks, that makes sense. I agree we shouldn't rely on temp paths as a trust boundary. i have pushed an update addressing this. I added a new gs_path_quote helper function that wraps the path in double quotes (to preserve spaces for Ghostscript) but also explicitly escapes This ensures Please let me know if there's anything else you'd like me to change! |
Problem
On macOS, MoganSTEM fails to render PS/EPS images when the temp path contains
spaces (e.g., when the user's full name has a space, like
/var/folders/.../John Doe/...).Ghostscript receives malformed arguments and cannot process the file.
Closes #2703
Root Cause
sys_concretize(url) calls escape_sh(concretize(url)), which on macOS/Linux
backslash-escapes spaces:
gs -sOutputFile=/tmp/path\ with\ spaces/file.eps
The shell correctly passes standalone args with backslash-escapes, but Ghostscript
parses its own option values (
-sOutputFile=...,-sFile=...,--permit-file-read=...)as raw strings — it does not re-interpret shell backslash escapes. So GS splits
at the space and receives a malformed path.
Fix
Replace sys_concretize(url) with raw_quote(concretize(url)) at all path-in-option
sites in gs_utilities.cpp. Double-quoting passes the full path as one token:
gs -sOutputFile="/tmp/path with spaces/file.eps"
This is consistent with gs_prefix(), which already uses raw_quote(gs_executable())
on Windows for the same reason.
Changed functions: gs_image_size, gs_PDFimage_size, gs_to_eps, gs_to_ps, tm_gs.
How to Test
xmake build stem && xmake run stem