Merge branches 'windows-process', 'adb-devices-parsing' and 'tcp-mdns' into issue6248.1

app/meson.build

@@ -76,6 +76,7 @@ conf.set('_GNU_SOURCE', true)
 if host_machine.system() == 'windows'
     windows = import('windows')
     src += [
+        'src/util/command.c',
         'src/sys/win/file.c',
         'src/sys/win/process.c',
         windows.compile_resources('scrcpy-windows.rc'),
@@ -238,6 +239,12 @@ if get_option('buildtype') == 'debug'
             'src/util/strbuf.c',
             'src/util/term.c',
         ]],
+        ['test_command_windows', [
+          'tests/test_command_windows.c',
+          'src/util/command.c',
+          'src/util/str.c',
+          'src/util/strbuf.c',
+        ]],
         ['test_control_msg_serialize', [
             'tests/test_control_msg_serialize.c',
             'src/control_msg.c',

app/src/adb/adb.c

@@ -331,56 +331,24 @@ sc_adb_reverse_remove(struct sc_intr *intr, const char *serial,
 bool
 sc_adb_push(struct sc_intr *intr, const char *serial, const char *local,
             const char *remote, unsigned flags) {
-#ifdef _WIN32
-    // Windows will parse the string, so the paths must be quoted
-    // (see sys/win/command.c)
-    local = sc_str_quote(local);
-    if (!local) {
-        return SC_PROCESS_NONE;
-    }
-    remote = sc_str_quote(remote);
-    if (!remote) {
-        free((void *) local);
-        return SC_PROCESS_NONE;
-    }
-#endif
-
     assert(serial);
     const char *const argv[] =
         SC_ADB_COMMAND("-s", serial, "push", local, remote);
 
     sc_pid pid = sc_adb_execute(argv, flags);
 
-#ifdef _WIN32
-    free((void *) remote);
-    free((void *) local);
-#endif
-
     return process_check_success_intr(intr, pid, "adb push", flags);
 }
 
 bool
 sc_adb_install(struct sc_intr *intr, const char *serial, const char *local,
                unsigned flags) {
-#ifdef _WIN32
-    // Windows will parse the string, so the local name must be quoted
-    // (see sys/win/command.c)
-    local = sc_str_quote(local);
-    if (!local) {
-        return SC_PROCESS_NONE;
-    }
-#endif
-
     assert(serial);
     const char *const argv[] =
         SC_ADB_COMMAND("-s", serial, "install", "-r", local);
 
     sc_pid pid = sc_adb_execute(argv, flags);
 
-#ifdef _WIN32
-    free((void *) local);
-#endif
-
     return process_check_success_intr(intr, pid, "adb install", flags);
 }
 

app/src/adb/adb_device.c

@@ -39,5 +39,11 @@ sc_adb_device_get_type(const char *serial) {
         return SC_ADB_DEVICE_TYPE_TCPIP;
     }
 
+    // TCP/IP devices provided by mDNS contain "adb-tls-connect"
+    // <https://github.com/Genymobile/scrcpy/issues/6248>
+    if (strstr(serial, "adb-tls-connect")) {
+        return SC_ADB_DEVICE_TYPE_TCPIP;
+    }
+
     return SC_ADB_DEVICE_TYPE_USB;
 }

app/src/adb/adb_parser.c

@@ -8,6 +8,31 @@
 #include "util/log.h"
 #include "util/str.h"
 
+static size_t
+rstrip_len(const char *s, size_t len) {
+    size_t i = len;
+
+    // Ignore trailing whitespaces
+    while (i > 0 && (s[i-1] == ' ' || s[i-1] == '\t')) {
+        --i;
+    }
+
+    return i;
+}
+
+static void
+locate_last_token(const char *s, size_t len, size_t *start, size_t *end) {
+    size_t i = rstrip_len(s, len);
+    *end = i; // excluded
+
+    // The token contains non-whitespace chars
+    while (i > 0 && (s[i-1] != ' ' && s[i-1] != '\t')) {
+        --i;
+    }
+
+    *start = i; // included
+}
+
 static bool
 sc_adb_parse_device(char *line, struct sc_adb_device *device) {
     // One device line looks like:
@@ -25,64 +50,54 @@ sc_adb_parse_device(char *line, struct sc_adb_device *device) {
         return false;
     }
 
-    char *s = line; // cursor in the line
+    size_t len = strlen(line);
 
-    // After the serial:
-    //  - "adb devices" writes a single '\t'
-    //  - "adb devices -l" writes multiple spaces
-    // For flexibility, accept both.
-    size_t serial_len = strcspn(s, " \t");
+    size_t start;
+    size_t end;
+
+    // The serial (the first token) may contain spaces, which are also token
+    // separators. To avoid ambiguity, parse the string backwards:
+    //  - first, parse all the trailing key-value pairs (containing ':'),
+    //  - then, the preceding token (not containing ':') is the device state,
+    //  - finally, the remaining leading token is the device serial.
+    //
+    // Refs:
+    //  - <https://github.com/Genymobile/scrcpy/issues/6248>
+    //  - <https://github.com/Genymobile/scrcpy/issues/3537>
+    const char *state;
+    const char *model = NULL;
+    for (;;) {
+        locate_last_token(line, len, &start, &end);
+        if (start == end) {
+            // No more tokens, unexpected
+            return false;
+        }
+
+        const char *token = &line[start];
+        line[end] = '\0';
+
+        if (!strncmp("model:", token, sizeof("model:") - 1)) {
+            model = &token[sizeof("model:") - 1];
+            // We only need the model
+        } else if (!strchr(token, ':')) {
+            // The first non-key:value token, it's the device state
+            state = token;
+            break;
+        }
+
+        // Remove the trailing parts already handled
+        len = start;
+    }
+
+    assert(state);
+
+    size_t serial_len = rstrip_len(line, start);
     if (!serial_len) {
         // empty serial
         return false;
     }
-    bool eol = s[serial_len] == '\0';
-    if (eol) {
-        // serial alone is unexpected
-        return false;
-    }
-    s[serial_len] = '\0';
-    char *serial = s;
-    s += serial_len + 1;
-    // After the serial, there might be several spaces
-    s += strspn(s, " \t"); // consume all separators
-
-    size_t state_len = strcspn(s, " ");
-    if (!state_len) {
-        // empty state
-        return false;
-    }
-    eol = s[state_len] == '\0';
-    s[state_len] = '\0';
-    char *state = s;
-
-    char *model = NULL;
-    if (!eol) {
-        s += state_len + 1;
-
-        // Iterate over all properties "key:value key:value ..."
-        for (;;) {
-            size_t token_len = strcspn(s, " ");
-            if (!token_len) {
-                break;
-            }
-            eol = s[token_len] == '\0';
-            s[token_len] = '\0';
-            char *token = s;
-
-            if (!strncmp("model:", token, sizeof("model:") - 1)) {
-                model = &token[sizeof("model:") - 1];
-                // We only need the model
-                break;
-            }
-
-            if (eol) {
-                break;
-            } else {
-                s+= token_len + 1;
-            }
-        }
-    }
+    char *serial = line;
+    line[serial_len] = '\0';
 
     device->serial = strdup(serial);
     if (!device->serial) {

app/src/sys/win/process.c

@@ -3,26 +3,14 @@
 #include <processthreadsapi.h>
 
 #include <assert.h>
+#include <stdlib.h>
 
+#include "util/command.h"
 #include "util/log.h"
 #include "util/str.h"
 
 #define CMD_MAX_LEN 8192
 
-static bool
-build_cmd(char *cmd, size_t len, const char *const argv[]) {
-    // Windows command-line parsing is WTF:
-    // <http://daviddeley.com/autohotkey/parameters/parameters.htm#WINPASS>
-    // only make it work for this very specific program
-    // (don't handle escaping nor quotes)
-    size_t ret = sc_str_join(cmd, argv, ' ', len);
-    if (ret >= len) {
-        LOGE("Command too long (%" SC_PRIsizet " chars)", len - 1);
-        return false;
-    }
-    return true;
-}
-
 enum sc_process_result
 sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags,
                      HANDLE *pin, HANDLE *pout, HANDLE *perr) {
@@ -137,8 +125,9 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags,
         si.lpAttributeList = lpAttributeList;
     }
 
-    char *cmd = malloc(CMD_MAX_LEN);
-    if (!cmd || !build_cmd(cmd, CMD_MAX_LEN, argv)) {
+    assert(argv && *argv);
+    char *cmd = sc_command_serialize_windows(argv);
+    if (!cmd) {
         LOG_OOM();
         goto error_free_attribute_list;
     }

app/src/util/command.c

@@ -0,0 +1,99 @@
+#include "command.h"
+
+#include <stdlib.h>
+
+#include "util/strbuf.h"
+
+char *
+sc_command_serialize_windows(const char *const argv[]) {
+    // <https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments>
+    // <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw>
+
+    struct sc_strbuf buf;
+    bool ok = sc_strbuf_init(&buf, 1024);
+    if (!ok) {
+        return NULL;
+    }
+
+#define BUF_PUSH(C) \
+    do { \
+        ok = sc_strbuf_append_char(&buf, C); \
+        if (!ok) { \
+            goto end; \
+        } \
+    } while (0)
+
+    while (*argv) {
+        const char *arg = *argv;
+
+        BUF_PUSH('"');
+
+        // <https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments>
+        //
+        // """
+        // If an even number of backslashes is followed by a double quote mark,
+        // then one backslash (\) is placed in the argv array for every pair of
+        // backslashes (\\), and the double quote mark (") is interpreted as a
+        // string delimiter.
+        //
+        // If an odd number of backslashes is followed by a double quote mark,
+        // then one backslash (\) is placed in the argv array for every pair of
+        // backslashes (\\). The double quote mark is interpreted as an escape
+        // sequence by the remaining backslash, causing a literal double quote
+        // mark (") to be placed in argv.
+        // """
+        //
+        // To produce correct escaping according to what the parser will do, we
+        // must count the number of successive backslashes.
+        unsigned backslashes = 0;
+
+        for (const char *c = arg; *c; c++) {
+            switch (*c) {
+                case '"':
+                    while (backslashes) {
+                        // Double all backslashes before a quote
+                        BUF_PUSH('\\');
+                        BUF_PUSH('\\');
+                        --backslashes;
+                    }
+                    BUF_PUSH('\\');
+                    BUF_PUSH('"');
+                    backslashes = 0;
+                    break;
+                case '\\':
+                    ++backslashes;
+                    break;
+                default:
+                    while (backslashes) {
+                        // Put all backslashes as literals
+                        BUF_PUSH('\\');
+                        --backslashes;
+                    }
+                    BUF_PUSH(*c);
+                    break;
+            }
+        }
+
+        while (backslashes) {
+            // Double all backslashes before a quote
+            BUF_PUSH('\\');
+            BUF_PUSH('\\');
+            --backslashes;
+        }
+
+        BUF_PUSH('"');
+
+        ++argv;
+
+        // Argument separator
+        if (*argv) {
+            BUF_PUSH(' ');
+        }
+    }
+
+    return buf.s;
+
+end:
+    free(buf.s);
+    return NULL;
+}

app/src/util/command.h

@@ -0,0 +1,17 @@
+#ifndef SC_COMMAND_H
+#define SC_COMMAND_H
+
+#include "common.h"
+
+/**
+ * Serialize an argv array for Windows
+ *
+ * Convert a NULL-terminated argument array into a single escaped string
+ * suitable for massing to CreateProcess() on Windows.
+ *
+ * The returned value must be freed by the caller.
+ */
+char *
+sc_command_serialize_windows(const char *const argv[]);
+
+#endif

app/src/util/str.c

@@ -49,21 +49,6 @@ truncated:
     return n;
 }
 
-char *
-sc_str_quote(const char *src) {
-    size_t len = strlen(src);
-    char *quoted = malloc(len + 3);
-    if (!quoted) {
-        LOG_OOM();
-        return NULL;
-    }
-    memcpy(&quoted[1], src, len);
-    quoted[0] = '"';
-    quoted[len + 1] = '"';
-    quoted[len + 2] = '\0';
-    return quoted;
-}
-
 char *
 sc_str_concat(const char *start, const char *end) {
     assert(start);

app/src/util/str.h

@@ -32,14 +32,6 @@ sc_strncpy(char *dest, const char *src, size_t n);
 size_t
 sc_str_join(char *dst, const char *const tokens[], char sep, size_t n);
 
-/**
- * Quote a string
- *
- * Return a new allocated string, surrounded with quotes (`"`).
- */
-char *
-sc_str_quote(const char *src);
-
 /**
  * Concat two strings
  *

app/tests/test_adb_parser.c

@@ -163,6 +163,25 @@ static void test_adb_devices_spaces(void) {
     sc_adb_devices_destroy(&vec);
 }
 
+static void test_adb_devices_serial_with_spaces(void) {
+    char output[] =
+        "List of devices attached\n"
+        "adb-AEORQMSAOWNQ-N7Ger8 (2)._adb-tls-connect._tcp device "
+        "product:blazer model:Pixel_10_Pro device:blazer transport_id:3\n";
+
+    struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER;
+    bool ok = sc_adb_parse_devices(output, &vec);
+    assert(ok);
+    assert(vec.size == 1);
+
+    struct sc_adb_device *device = &vec.data[0];
+    assert(!strcmp("adb-AEORQMSAOWNQ-N7Ger8 (2)._adb-tls-connect._tcp", device->serial));
+    assert(!strcmp("device", device->state));
+    assert(!strcmp("Pixel_10_Pro", device->model));
+
+    sc_adb_devices_destroy(&vec);
+}
+
 static void test_get_ip_single_line(void) {
     char ip_route[] = "192.168.1.0/24 dev wlan0  proto kernel  scope link  src "
                       "192.168.12.34\r\r\n";
@@ -265,6 +284,7 @@ int main(int argc, char *argv[]) {
     test_adb_devices_without_header();
     test_adb_devices_corrupted();
     test_adb_devices_spaces();
+    test_adb_devices_serial_with_spaces();
 
     test_get_ip_single_line();
     test_get_ip_single_line_without_eol();

app/tests/test_command_windows.c

@@ -0,0 +1,61 @@
+#include "common.h"
+
+#include <assert.h>
+#include <stdlib.h>
+
+#include "util/command.h"
+
+static void test_command_with_spaces(void) {
+    const char *const argv[] = {
+        "C:\\Program Files\\scrcpy\\adb",
+        "-s",
+        "serial with spaces",
+        "push",
+        "E:\\some folder\\scrcpy-server",
+        "/data/local/tmp/scrcpy-server.jar",
+        NULL,
+    };
+    char *cmd = sc_command_serialize_windows(argv);
+    const char *expected = "\"C:\\Program Files\\scrcpy\\adb\" "
+                           "\"-s\" "
+                           "\"serial with spaces\" "
+                           "\"push\" "
+                           "\"E:\\some folder\\scrcpy-server\" "
+                           "\"/data/local/tmp/scrcpy-server.jar\"";
+
+    assert(!strcmp(expected, cmd));
+    free(cmd);
+}
+
+static void test_command_with_backslashes(void) {
+    const char *const argv[] = {
+        "a\\\\ b\\",
+        "def \\",
+        "gh\"i\" \\\\",
+        "jkl\\\\",
+        "mno\\",
+        "p\\\"qr",
+        NULL,
+    };
+
+    char *cmd = sc_command_serialize_windows(argv);
+    const char *expected = "\"a\\\\ b\\\\\" "
+                           "\"def \\\\\" "
+                           "\"gh\\\"i\\\" \\\\\\\\\" "
+                           "\"jkl\\\\\\\\\" "
+                           "\"mno\\\\\" "
+                           "\"p\\\\\\\"qr\"";
+
+    assert(!strcmp(expected, cmd));
+    free(cmd);
+}
+
+int main(int argc, char *argv[]) {
+    (void) argc;
+    (void) argv;
+
+    test_command_with_spaces();
+    test_command_with_backslashes();
+
+    return 0;
+}

app/tests/test_str.c

@@ -131,16 +131,6 @@ static void test_join_truncated_after_sep(void) {
     assert(!strcmp("abc de ", s));
 }
 
-static void test_quote(void) {
-    const char *s = "abcde";
-    char *out = sc_str_quote(s);
-
-    // add '"' at the beginning and the end
-    assert(!strcmp("\"abcde\"", out));
-
-    free(out);
-}
-
 static void test_concat(void) {
     const char *s = "2024:11";
     char *out = sc_str_concat("my-prefix:", s);
@@ -398,7 +388,6 @@ int main(int argc, char *argv[]) {
     test_join_truncated_in_token();
     test_join_truncated_before_sep();
     test_join_truncated_after_sep();
-    test_quote();
     test_concat();
     test_utf8_truncate();
     test_parse_integer();