improve handling of user provided external viewer (fixes #3868)

sumatrapdfreader · Nov 12, 2023 · 69fe826 · 69fe826
1 parent 9510b36
commit 69fe826
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 18 deletions.
diff --git a/src/ExternalViewers.cpp b/src/ExternalViewers.cpp
@@ -354,13 +354,11 @@ static TempStr FormatParamsTemp(const char* cmdLine, WindowTab* tab) {
         cmdLine = str::ReplaceTemp(cmdLine, "%d", dir);
         appendPath = false;
     }
-    if (str::Find(cmdLine, R"("%1")")) {
-        // "%1", is alrady quoted so no need to add quotes
+    if (str::Find(cmdLine, "%1")) {
+        // TODO: if %1 is un-quoted, we should quote it but it's complicated because
+        // it could be part of a pattern like %1.Page%p.txt
+        // (as in https://github.com/sumatrapdfreader/sumatrapdf/issues/3868)
         cmdLine = str::ReplaceTemp(cmdLine, "%1", path);
-    } else if (str::Find(cmdLine, R"(%1)")) {
-        // %1, not quoted, need to add
-        char* s = str::JoinTemp("\"", path, "\"");
-        cmdLine = str::ReplaceTemp(cmdLine, "%1", s);
     } else if (appendPath) {
         cmdLine = str::FormatTemp(R"(%s "%s")", cmdLine, path);
     }
@@ -415,16 +413,24 @@ bool ViewWithExternalViewer(WindowTab* tab, size_t idx) {
         return false;
     }
 
-    CmdLineArgsIter args(ToWStrTemp(ev->commandLine));
-    if (args.nArgs == 0) {
+    StrVec args;
+    ParseCmdLine(ToWStrTemp(ev->commandLine), args);
+    int nArgs = args.Size();
+    if (nArgs == 0) {
         return false;
     }
     const char* exePath = args.at(0);
     if (!file::Exists(exePath)) {
         return false;
     }
-    char* cmdLine = args.ParamsTemp();
-    TempStr params = FormatParamsTemp(cmdLine, tab);
+    StrVec argsQuoted;
+    for (int i = 1; i < nArgs; i++) {
+        char* s = args.at(i);
+        TempStr param = FormatParamsTemp(s, tab);
+        TempStr paramQuoted = QuoteCmdLineArgTemp(param);
+        argsQuoted.Append(paramQuoted);
+    }
+    TempStr params = JoinTemp(argsQuoted, " ");
     return LaunchFile(exePath, params);
 }
 

diff --git a/src/utils/CmdLineArgsIter.cpp b/src/utils/CmdLineArgsIter.cpp
@@ -4,6 +4,59 @@
 #include "utils/BaseUtil.h"
 #include "utils/CmdLineArgsIter.h"
 
+// TODO: quote '"' etc as per:
+// https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments?view=msvc-170&redirectedfrom=MSDN
+TempStr QuoteCmdLineArgTemp(char* arg) {
+    if (!arg) {
+        return nullptr;
+    }
+    int n = (int)str::Len(arg);
+    if (n < 2) {
+        return arg;
+    }
+    if (*arg == '"' && arg[n - 1] == '"') {
+        // already quoted, we assume correctly
+        return arg;
+    }
+    bool needsQuote = false;
+    char* s = arg;
+    char c = *s++;
+    while (c) {
+        if (c == ' ' || c == '"') {
+            needsQuote = true;
+            break;
+        }
+        c = *s++;
+    }
+    if (!needsQuote) {
+        return arg;
+    }
+    str::Str res;
+    // TODO: can't do it because PoolAllocator doesn't support Realloc()
+    // res.allocator = GetTempAllocator();
+    res.AppendChar('"');
+    s = arg;
+    c = *s++;
+    while (c) {
+        // TODO: quote '"' ?
+        res.AppendChar(c);
+        c = *s++;
+    }
+    res.AppendChar('"');
+    return res.StealData();
+}
+
+int ParseCmdLine(const WCHAR* cmdLine, StrVec& argsOut) {
+    int nArgs;
+    WCHAR** argsArr = CommandLineToArgvW(cmdLine, &nArgs);
+    for (int i = 0; i < nArgs; i++) {
+        char* arg = ToUtf8Temp(argsArr[i]);
+        argsOut.Append(arg);
+    }
+    LocalFree(argsArr);
+    return nArgs;
+}
+
 bool CouldBeArg(const char* s) {
     char c = *s;
     return (c == L'-') || (c == L'/');
@@ -13,12 +66,7 @@ CmdLineArgsIter::~CmdLineArgsIter() {
 }
 
 CmdLineArgsIter::CmdLineArgsIter(const WCHAR* cmdLine) {
-    WCHAR** argsArr = CommandLineToArgvW(cmdLine, &nArgs);
-    for (int i = 0; i < nArgs; i++) {
-        char* arg = ToUtf8Temp(argsArr[i]);
-        args.Append(arg);
-    }
-    LocalFree(argsArr);
+    nArgs = ParseCmdLine(cmdLine, args);
 }
 
 const char* CmdLineArgsIter::NextArg() {

diff --git a/src/utils/CmdLineArgsIter.h b/src/utils/CmdLineArgsIter.h
@@ -3,6 +3,9 @@
 
 bool CouldBeArg(const char*);
 
+int ParseCmdLine(const WCHAR* cmdLine, StrVec& argsOut);
+TempStr QuoteCmdLineArgTemp(char* arg);
+
 struct CmdLineArgsIter {
     StrVec args;
     int curr = 1; // first argument is exe path, which we skip

diff --git a/src/utils/StrUtil.h b/src/utils/StrUtil.h
@@ -468,6 +468,6 @@ struct StrVec {
 };
 
 size_t Split(StrVec& v, const char* s, const char* separator, bool collapse = false);
-char* Join(const StrVec& v, const char* joint = nullptr);
-TempStr JoinTemp(const StrVec& v, const char* joint);
+char* Join(const StrVec& v, const char* sep = nullptr);
+TempStr JoinTemp(const StrVec& v, const char* sep);
 ByteSlice ToByteSlice(const char* s);