Re: [Libguestfs] [PATCH nbdkit 04/10] server: Calculate $uri in one place
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