apr_proc_create(): Properly escape arguments containing whitespace characters

on Windows.

* CMakeLists.txt
  (single_source_programs): Add test/echoargs.c.

* test/echoargs.c: New test app for test_proc_args test.

* test/testproc.c
  (test_proc_args): New test.
  (testproc): Add test_proc_args to test list.

* threadproc/win32/proc.c
  (quote_arg): New. Helper for apr_proc_create().
  (apr_proc_create): Use quote_arg() helper to escape arguments in command
  line.

git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1868477 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 203 insertions, 6 deletions

diff --git a/CHANGES b/CHANGES
index 0a6b49b87..47d3d55bb 100644
--- a/CHANGES
+++ b/CHANGES
@@ -228,6 +228,9 @@ Changes for APR 2.0.0
 
   *) apr_atomic_read64(): Fix non-atomic read on 32-bit Windows [Ivan Zhakov]
 
+  *) apr_proc_create(): Properly escape arguments containing whitespace
+     characters on Windows [Ivan Zhakov]
+
 Changes for APR and APR-util 1.6.x and later:
 
   *) http://svn.apache.org/viewvc/apr/apr/branches/1.6.x/CHANGES?view=markup
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c731fc816..e42fd6d8d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -560,6 +560,7 @@ IF(APR_BUILD_TESTAPR)
   # requirements.
   SET(single_source_programs
     test/dbd.c
+    test/echoargs.c
     test/echod.c
     test/sendfile.c
     test/sockperf.c
diff --git a/test/echoargs.c b/test/echoargs.c
new file mode 100644
index 000000000..38ce5fa0f
--- /dev/null
+++ b/test/echoargs.c
@@ -0,0 +1,47 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* Simple echo daemon, designed to be used for network throughput
+ * benchmarks. The aim is to allow us to monitor changes in performance
+ * of APR networking code, nothing more.
+ */
+
+#include <stdio.h>
+
+#include <apr.h>
+#include <apr_general.h>
+#include <apr_file_io.h>
+
+int main(int argc, const char * const *argv, const char* const* env)
+{
+    apr_pool_t* pool;
+    apr_file_t* file;
+    int i;
+
+    apr_app_initialize(&argc, &argv, &env);
+    apr_pool_create(&pool, NULL);
+
+    apr_file_open_stdout(&file, pool);
+    for (i = 1; i < argc; i++)
+    {
+        if (i > 1) apr_file_puts(",", file);
+        apr_file_puts(argv[i], file);
+    }
+
+    apr_terminate();
+
+    return 0;
+}
diff --git a/test/testproc.c b/test/testproc.c
index c86fb2997..bf622fb65 100644
--- a/test/testproc.c
+++ b/test/testproc.c
@@ -160,6 +160,71 @@ static void test_file_redir(abts_case *tc, void *data)
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 }
 
+static void test_proc_args(abts_case* tc, void* data)
+{
+    const char* args[10];
+    apr_procattr_t* attr;
+    apr_status_t rv;
+    char *progname;
+    const char *expected;
+    const char *actual;
+
+    apr_filepath_merge(&progname, NULL, "echoargs" EXTENSION, 0, p);
+
+    rv = apr_procattr_create(&attr, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_procattr_io_set(attr, APR_NO_PIPE, APR_FULL_BLOCK, APR_NO_PIPE);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_procattr_cmdtype_set(attr, APR_PROGRAM_ENV);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    args[0] = progname;
+    args[1] = "1";
+    args[2] = "";
+    args[3] = "\"te st";
+    args[4] = " a\\b";
+    args[5] = " a\\\\b";
+    args[6] = " \\";
+    args[7] = "new\nline";
+    args[8] = " \\\\";
+    args[9] = NULL;
+
+    rv = apr_proc_create(&newproc, progname, args, NULL, attr, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    actual = "";
+    while (1)
+    {
+        char buf[1024];
+        apr_size_t length = sizeof(buf);
+
+        rv = apr_file_read(newproc.out, buf, &length);
+        if (APR_STATUS_IS_EOF(rv)) {
+            break;
+        }
+        else if (rv != APR_SUCCESS)
+        {
+            ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+            break;
+        }
+
+        buf[length] = 0;
+        actual = apr_pstrcat(p, actual, buf, NULL);
+    }
+
+    expected = "1" ","
+               "" ","
+               "\"te st" ","
+               " a\\b" ","
+               " a\\\\b" ","
+               " \\" ","
+               "new\nline" ","
+               " \\\\";
+    ABTS_STR_EQUAL(tc, expected, actual);
+}
+
 abts_suite *testproc(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -168,6 +233,7 @@ abts_suite *testproc(abts_suite *suite)
     abts_run_test(suite, test_create_proc, NULL);
     abts_run_test(suite, test_proc_wait, NULL);
     abts_run_test(suite, test_file_redir, NULL);
+    abts_run_test(suite, test_proc_args, NULL);
 
     return suite;
 }
diff --git a/threadproc/win32/proc.c b/threadproc/win32/proc.c
index 05953e347..5fee0c286 100644
--- a/threadproc/win32/proc.c
+++ b/threadproc/win32/proc.c
@@ -331,6 +331,91 @@ static const char* has_space(const char *str)
     return NULL;
 }
 
+/* Quote one command line argument if needed. See documentation for
+ * CommandLineToArgV() for details:
+ * https://docs.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw
+ */
+static const char * quote_arg(const char *str, apr_pool_t *pool)
+{
+    const char *ch;
+    apr_size_t needed;
+    char *escaped;
+    char *dst;
+
+    /* Perform quoting only if neccessary. */
+    if (*str && !strpbrk(str, " \f\n\r\t\v\"")) {
+        return str;
+    }
+
+    needed = 0;
+    /* One char for leading quote. */
+    needed++;
+    for (ch = str; ; ch++) {
+        apr_size_t backslash_count = 0;
+
+        while (*ch == '\\') {
+            backslash_count++;
+            ch++;
+        }
+
+        if (*ch == 0) {
+            /* Escape backslashes. */
+            needed += backslash_count * 2;
+            break;
+        }
+        else if (*ch == '"') {
+            /* Escape backslashes. */
+            needed += backslash_count * 2 + 1;
+            /* Double quote char. */
+            needed += 1;
+        }
+        else {
+            /* Unescaped backslashes. */
+            needed += backslash_count;
+            /* Original character. */
+            needed += 1;
+        }
+    }
+
+    /* For trailing quote. */
+    needed++;
+    /* Zero terminator. */
+    needed++;
+
+    escaped = apr_palloc(pool, needed);
+
+    dst = escaped;
+    *dst++ = '"';
+    for (ch = str; ; ch++) {
+        apr_size_t backslash_count = 0;
+
+        while (*ch == '\\') {
+            backslash_count++;
+            ch++;
+        }
+
+        if (*ch == 0) {
+            memset(dst, '\\', backslash_count * 2);
+            dst += backslash_count * 2;
+            break;
+        }
+        else if (*ch == '"') {
+            memset(dst, '\\', backslash_count * 2 + 1);
+            dst += backslash_count * 2 + 1;
+            *dst++ = *ch;
+        }
+        else {
+            memset(dst, '\\', backslash_count);
+            dst += backslash_count;
+            *dst++ = *ch;
+        }
+    }
+    *dst++ = '"';
+    *dst = 0;
+
+    return escaped;
+}
+
 static char *apr_caret_escape_args(apr_pool_t *p, const char *str)
 {
     char *cmd;
@@ -583,12 +668,7 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
              * Handle the args, seperate from argv0 */
             cmdline = argv0;
             for (i = 1; args && args[i]; ++i) {
-                if (has_space(args[i]) || !args[i][0]) {
-                    cmdline = apr_pstrcat(pool, cmdline, " \"", args[i], "\"", NULL);
-                }
-                else {
-                    cmdline = apr_pstrcat(pool, cmdline, " ", args[i], NULL);
-                }
+                cmdline = apr_pstrcat(pool, cmdline, " ", quote_arg(args[i], pool), NULL);
             }
 
             /* Do not pass the first arg to CreateProc() for APR_PROGRAM_PATH