commit a0b09f463d70de26af8e0c0d3fb643fe971c24b0
Author: Pavel Sanda <[email protected]>
Date: Wed May 27 13:01:14 2026 +0200
Implement startscript with cleaner argument handling.
New startscriptArgv added, the external calls via runCommand() with
user-crafted input would be better to route via this gate and avoid
parsecmd/splitCommand parsers altogether. Completion handling shared with
original startscript.
The argv form is wait-only and does not support TEXINPUTS env
prefix or shell-style redirects (no current caller needs them). If
a future caller does, extend the API rather than reintroducing the
string round-trip.
Tier 01 DiD / argv 01: argv-form API; re-closes 00a/00b
structurally, removes popen runCommand
https://www.mail-archive.com/[email protected]/msg226742.html
Assisted-by: Claude Opus 4.7
---
src/support/Systemcall.cpp | 199 ++++++++++++++++++++++++++++++++--------
src/support/Systemcall.h | 44 +++++++++
src/support/SystemcallPrivate.h | 17 ++++
3 files changed, 221 insertions(+), 39 deletions(-)
diff --git a/src/support/Systemcall.cpp b/src/support/Systemcall.cpp
index ab3fd2ed50..a9bdde41e3 100644
--- a/src/support/Systemcall.cpp
+++ b/src/support/Systemcall.cpp
@@ -253,6 +253,49 @@ void Systemcall::killscript()
}
+namespace {
+
+// Shared tail of startscript() and startscriptArgv(): wait for the
+// process spawned via d.startProcess() to start, run, and finish, then
+// translate state/exit-code into a Systemcall::ReturnValue. Body is
+// the verbatim post-startProcess block extracted from startscript().
+int waitForCompletion(SystemcallPrivate & d, QString const & cmd,
+ bool do_events, int timeout_ms)
+{
+ if (d.state == SystemcallPrivate::Error
+ || !d.waitWhile(SystemcallPrivate::Starting, do_events,
-1)) {
+ if (d.state == SystemcallPrivate::Error) {
+ LYXERR0("Systemcall: '" << cmd << "' did not start!");
+ LYXERR0("error " << d.errorMessage());
+ return Systemcall::NOSTART;
+ } else if (d.state == SystemcallPrivate::Killed) {
+ LYXERR0("Killed: " << cmd);
+ return Systemcall::KILLED;
+ }
+ }
+
+ if (!d.waitWhile(SystemcallPrivate::Running, do_events, timeout_ms)) {
+ if (d.state == SystemcallPrivate::Killed) {
+ LYXERR0("Killed: " << cmd);
+ return Systemcall::KILLED;
+ }
+ LYXERR0("Systemcall: '" << cmd << "' did not finish!");
+ LYXERR0("error " << d.errorMessage());
+ LYXERR0("status " << d.exitStatusMessage());
+ return Systemcall::TIMEOUT;
+ }
+
+ int const exit_code = d.exitCode();
+ if (exit_code) {
+ LYXERR0("Systemcall: '" << cmd << "' finished with exit code "
<< exit_code);
+ }
+
+ return exit_code;
+}
+
+} // namespace
+
+
int Systemcall::startscript(Starttype how, string const & what,
string const & path, string const & lpath,
bool process_events)
@@ -276,36 +319,49 @@ int Systemcall::startscript(Starttype how, string const &
what,
if (how == DontWait && d.state == SystemcallPrivate::Running)
return OK;
- if (d.state == SystemcallPrivate::Error
- || !d.waitWhile(SystemcallPrivate::Starting, do_events,
-1)) {
- if (d.state == SystemcallPrivate::Error) {
- LYXERR0("Systemcall: '" << cmd << "' did not start!");
- LYXERR0("error " << d.errorMessage());
- return NOSTART;
- } else if (d.state == SystemcallPrivate::Killed) {
- LYXERR0("Killed: " << cmd);
- return KILLED;
- }
- }
+ return waitForCompletion(d, cmd, do_events, os::timeout_ms());
+}
- if (!d.waitWhile(SystemcallPrivate::Running, do_events,
- os::timeout_ms())) {
- if (d.state == SystemcallPrivate::Killed) {
- LYXERR0("Killed: " << cmd);
- return KILLED;
- }
- LYXERR0("Systemcall: '" << cmd << "' did not finish!");
- LYXERR0("error " << d.errorMessage());
- LYXERR0("status " << d.exitStatusMessage());
- return TIMEOUT;
- }
- int const exit_code = d.exitCode();
- if (exit_code) {
- LYXERR0("Systemcall: '" << cmd << "' finished with exit code "
<< exit_code);
- }
+int Systemcall::startscriptArgv(string const & program,
+ vector<string> const & args,
+ string * stdout_capture)
+{
+ // Build the QStringList that will be handed directly to QProcess.
+ // Each element becomes one argv slot in the child; nothing here
+ // goes through commandPrep / parsecmd / QProcess::splitCommand,
+ // so crafted bytes in \p args cannot be re-split into a different
+ // argv than the caller wrote. That is the security property of
+ // this entry point.
+ QString const qprogram = QString::fromLocal8Bit(program.c_str());
+ QStringList qargs;
+ qargs.reserve(int(args.size()));
+ for (string const & a : args)
+ qargs << QString::fromLocal8Bit(a.c_str());
+
+ // Display string for log / cancel-dialog. Never executed.
+ QString display = qprogram;
+ for (QString const & a : qargs)
+ display += QLatin1Char(' ') + a;
- return exit_code;
+ if (verbose)
+ lyxerr << "\nRunning: " << fromqstr(display) << endl;
+ else
+ LYXERR(Debug::INFO, "Running: " << fromqstr(display));
+
+ string const empty;
+ SystemcallPrivate d(empty, empty, empty);
+ QByteArray captured;
+ if (stdout_capture)
+ d.setStdoutCapture(&captured);
+
+ d.startProcess(qprogram, qargs, display);
+ int const ret = waitForCompletion(d, display, false, os::timeout_ms());
+
+ if (stdout_capture)
+ stdout_capture->assign(captured.constData(), captured.size());
+
+ return ret;
}
@@ -315,7 +371,8 @@ bool SystemcallPrivate::kill_script = false;
SystemcallPrivate::SystemcallPrivate(std::string const & sf, std::string const
& of,
std::string const & ef)
: state(Error), process_(new QProcess), out_index_(0), err_index_(0),
- in_file_(sf), out_file_(of), err_file_(ef), process_events_(false)
+ in_file_(sf), out_file_(of), err_file_(ef), process_events_(false),
+ capture_stdout_(nullptr)
{
if (!in_file_.empty())
process_->setStandardInputFile(QString::fromLocal8Bit(in_file_.c_str()));
@@ -432,6 +489,31 @@ void SystemcallPrivate::startProcess(QString const & cmd,
string const & path,
}
+void SystemcallPrivate::startProcess(QString const & program,
+ QStringList const & args,
+ QString const & display)
+{
+ // cmd_ holds the display string used by the cancel dialog and log
+ // messages only. The actual launch goes through QProcess::start
+ // with (program, args) as separate arguments, bypassing
+ // QProcess::splitCommand entirely - so nothing between this point
+ // and the child's argv vector can reshape it. Detached/DontWait
+ // is not supported for the argv form (the sole caller is
+ // Systemcall::startscriptArgv, which is wait-only).
+ cmd_ = display.isEmpty() ? program : display;
+ if (process_) {
+ state = SystemcallPrivate::Starting;
+ process_->start(program, args);
+ }
+}
+
+
+void SystemcallPrivate::setStdoutCapture(QByteArray * dest)
+{
+ capture_stdout_ = dest;
+}
+
+
bool SystemcallPrivate::waitAndCheck()
{
Sleep::millisec(100);
@@ -551,17 +633,22 @@ SystemcallPrivate::~SystemcallPrivate()
void SystemcallPrivate::stdOut()
{
- if (process_) {
- char c;
- process_->setReadChannel(QProcess::StandardOutput);
- while (process_->getChar(&c)) {
- out_data_[out_index_++] = c;
- if (c == '\n' || out_index_ + 1 == buffer_size_) {
- out_data_[out_index_] = '\0';
- out_index_ = 0;
-
ProgressInterface::instance()->appendMessage(QString::fromLocal8Bit(out_data_));
- cout << out_data_;
- }
+ if (!process_)
+ return;
+ if (capture_stdout_) {
+ // argv-form callers want raw stdout in a buffer, not echoed.
+ capture_stdout_->append(process_->readAllStandardOutput());
+ return;
+ }
+ char c;
+ process_->setReadChannel(QProcess::StandardOutput);
+ while (process_->getChar(&c)) {
+ out_data_[out_index_++] = c;
+ if (c == '\n' || out_index_ + 1 == buffer_size_) {
+ out_data_[out_index_] = '\0';
+ out_index_ = 0;
+
ProgressInterface::instance()->appendMessage(QString::fromLocal8Bit(out_data_));
+ cout << out_data_;
}
}
}
@@ -701,6 +788,40 @@ void SystemcallPrivate::killProcess(QProcess * p)
}
}
+vector<string> splitCommandTokens(string const & cmd)
+{
+ vector<string> argv;
+ size_t i = 0;
+ while (i < cmd.size()) {
+
+ // skip to the next token
+ while (i < cmd.size() && (cmd[i] == ' ' || cmd[i] == '\t'))
+ ++i;
+ if (i >= cmd.size())
+ break;
+
+ string tok;
+
+ // token quoted
+ if (cmd[i] == '"') {
+ ++i;
+ while (i < cmd.size() && cmd[i] != '"')
+ tok += cmd[i++];
+ if (i < cmd.size())
+ ++i; // step over closing quote
+
+ // token unquoted
+ } else {
+ while (i < cmd.size() && cmd[i] != ' ' && cmd[i] !=
'\t')
+ tok += cmd[i++];
+ }
+ if (!tok.empty())
+ argv.push_back(tok);
+ }
+ return argv;
+}
+
+
} // namespace support
} // namespace lyx
diff --git a/src/support/Systemcall.h b/src/support/Systemcall.h
index d73a928097..2ce3f40153 100644
--- a/src/support/Systemcall.h
+++ b/src/support/Systemcall.h
@@ -15,6 +15,9 @@
#ifndef SYSTEMCALL_H
#define SYSTEMCALL_H
+#include <string>
+#include <vector>
+
namespace lyx {
namespace support {
@@ -64,8 +67,49 @@ public:
std::string const & path = empty_string(),
std::string const & lpath = empty_string(),
bool process_events = false);
+
+ /** Argv-form invocation: shell-free, parser-free.
+ *
+ * Builds an argv vector from (program, args) and hands it directly
+ * to QProcess::start(program, QStringList), which calls execvp()
+ * on Unix and CreateProcess() on Windows. Each element of \p args
+ * reaches the child as one argv slot, byte-for-byte.
+ *
+ * Note: although string-form startscript() does not invoke a shell
+ * it resplits into different argv, which is weak from the start.
+ *
+ * Wait-only (no DontWait/WaitLoop equivalent), no path/lpath
+ * TEXINPUTS prefix, no shell-style redirects. If a future caller
+ * needs any of those, extend the API rather than reintroducing the
+ * string round-trip.
+ *
+ * \p stdout_capture if non-null, child stdout is captured here.
+ */
+ int startscriptArgv(std::string const & program,
+ std::vector<std::string> const & args,
+ std::string * stdout_capture = nullptr);
};
+
+/// Split a shell-style command string into space-separated tokens, with
+/// double-quoted tokens copied verbatim. Used to migrate string-form
+/// invocations to the argv-form of Systemcall::startscriptArgv without
+/// going through a shell.
+///
+/// Not a full shell parser. Space and tab separate, empty tokens are
+/// dropped, and there is no handling of $VAR / $(), backslash escapes or
+/// single quotes.
+/// A double quote is only a delimiter at the start of a token: inside a
+/// token it is copied as an ordinary character, unlike a shell, which would
+/// remove it. An unterminated quote is not an error - it is dropped and
+/// the remainder of the string becomes a single token.
+///
+/// Used to split only strings LyX produced itself (python) or that come from
+/// user preferences. Document-supplied values must be appended as their own
+/// argv elements, never parsed here.
+std::vector<std::string> splitCommandTokens(std::string const & cmd);
+
+
} // namespace support
} // namespace lyx
diff --git a/src/support/SystemcallPrivate.h b/src/support/SystemcallPrivate.h
index 19771dfc73..42cb922696 100644
--- a/src/support/SystemcallPrivate.h
+++ b/src/support/SystemcallPrivate.h
@@ -12,8 +12,10 @@
#ifndef SYSTEMCALLPRIVATE_H
#define SYSTEMCALLPRIVATE_H
+#include <QByteArray>
#include <QObject>
#include <QProcess>
+#include <QStringList>
#include <string>
@@ -47,6 +49,19 @@ public:
void startProcess(QString const & cmd, std::string const & path,
std::string const & lpath, bool detach);
+ /// Argv-form launch: \p program and \p args go directly to
+ /// QProcess::start(program, args), bypassing QProcess::splitCommand
+ /// (which the string-form overload above uses to recover argv from
+ /// the command line). The child sees each element of \p args as
+ /// one argv slot byte-for-byte. \p display is shown in log messages
+ /// and the cancel-dialog text but is never executed. Wait-only.
+ void startProcess(QString const & program, QStringList const & args,
+ QString const & display);
+
+ /// When non-null, stdOut() routes incoming bytes into *dest
+ /// instead of echoing to the progress UI / std::cout.
+ void setStdoutCapture(QByteArray * dest);
+
int exitCode();
QString errorMessage() const;
@@ -92,6 +107,8 @@ private:
QString cmd_;
bool process_events_;
+ /// If non-null, stdOut() appends here instead of echoing.
+ QByteArray * capture_stdout_;
void waitAndProcessEvents();
void processEvents();
--
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs
