On Mon, Sep 11, 2023 at 03:09:21PM +0100, Richard W.M. Jones wrote:
- case SERVICE_MODE_UNIXSOCKET: - fprintf (fp, "nbd%s+unix://", tls == 2 ? "s" : ""); - if (export_name && strcmp (export_name, "") != 0) { - putc ('/', fp); - uri_quote (export_name, fp); - } - fprintf (fp, "\\?socket="); - uri_quote (unixsocket, fp);
Beforehand, we were manually shell-quoting the ? in the Unix URI...
The shell quoting here was only marginally useful before this change. In theory there might be a file in a subdirectory called 'nbd+unix:/Xsocket=' which would match :-)
+ switch (service_mode) { + case SERVICE_MODE_UNIXSOCKET: + fprintf (fp, "nbd%s+unix://", tls == 2 ? "s" : ""); + if (export_name && strcmp (export_name, "") != 0) { + putc ('/', fp); + uri_quote (export_name, fp); + } + fprintf (fp, "?socket="); + uri_quote (unixsocket, fp);
...where the manual shell-quoting is no longer injected. Yes, this looks correct (the appearance of the quoting, using '' instead of \, may be different, but the resulting string as parsed by the shell is the same).
My point was that pre-patch, we had: nbd+unix://\?socket=... and post-patch, we have: 'nbd+unix://?socket=...' but both forms are equally quoted; after the shell removes \ or '' quoting, we are left with: nbd+unix://?socket=... and neither form allowed the glob expansion of the (unlikely) file nbd+unix:/Xsocket=... My comment was more about why changing the fprintf("\\?socket=") pre-patch to fprintf("?socket=") post-patch was correct. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org