sheredom / subprocess.h

🐜 single header process launching solution for C and C++
The Unlicense
1.1k stars 97 forks source link

Incorrect argument quoting on Windows #59

Closed takase1121 closed 2 years ago

takase1121 commented 2 years ago

Yeah. I'm also speechless. Apparently on Windows you MUSTN'T quote too little or too much.

#include <stdio.h>
#include <stddef.h>
#include <malloc.h>
#include "subprocess.h"

int main() {
        const char *cmd[] = { "cmd", "/c", "echo hello", NULL };
        struct subprocess_s sub;
        int r = subprocess_create(cmd, 0, &sub);
        FILE *p = subprocess_stderr(&sub);
        char bruh[32];
        while (fgets(bruh, 32, p)) {
                printf("%s\n", bruh);
        }
        int pr;
        subprocess_join(&sub, &pr);
        return 0;
}

The code is a bit messy, but its obvious this is your typical run of the mill echo example adapted for Windows. The only problem is it doesn't work.

Apparently cmd.exe doesn't like its flags to be quoted. This library quotes every argument, so something from the above example would produce "cmd" "/c" "echo hello world". Perfectly fine for you and me (and sh), but for cmd... it interpreted "/c" "echo hello world" as /c "\"echo hello world"! Suddenly there's an extra quote there. The solution is apparently

const char *cmd[] = { "cmd", "/c echo hello", NULL };

This entire thing is just mind-boggling. Apparently the gold standard's force parameter is actually important! The only way I see to fix this problem is to not quote unescaped parameter, but I am not entirely sure (also pretty hard to do).

EDIT: some potential fixes:

454a455
>   int need_quoting;
625,626c626,631
<     // For the ' ' and two '"' between items and trailing '\0'
<     len += 3;
---
>     // For the trailing \0
>     len++;
>
>     // Quote the argument if it has space or tabs in it
>     if (strpbrk(commandLine[i], "\t\v ") != NULL)
>       len += 2;
659c664,668
<     commandLineCombined[len++] = '"';
---
>
>     need_quoting = strpbrk(commandLine[i], "\t\v ") != NULL;
>     if (need_quoting) {
>       commandLineCombined[len++] = '"';
>     }
678c687,690
<     commandLineCombined[len++] = '"';
---
>
>     if (need_quoting) {
>       commandLineCombined[len++] = '"';
>     }
sheredom commented 2 years ago

Happy to accept a PR.

aardappel commented 2 years ago

Just ran into this as well.. weirdly, quoting "/c" directly on the command line works, but not when run from CreateProcessA ?

takase1121 commented 2 years ago

Just ran into this as well.. weirdly, quoting "/c" directly on the command line works, but not when run from CreateProcessA ?

That's because the quotes are intepreted by cmd/powershell and removed because there's no space in it.