Re: [PATCH for-9.1 6/9] block/nbd: Use URI parsing code from glib

From: Eric Blake <eblake@redhat.com>
To: "Richard W.M. Jones" <rjones@redhat.com>
Cc: "Thomas Huth" <thuth@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	qemu-devel@nongnu.org, "Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-block@nongnu.org, nbd@other.debian.org
Subject: Re: [PATCH for-9.1 6/9] block/nbd: Use URI parsing code from glib
Date: Thu, 28 Mar 2024 10:06:01 -0500	[thread overview]
Message-ID: <jhweyusyjhha5hvffrtkwvuce35fajiy73dymgjre3jkjcjk7v@lrgdiintwb6i> (raw)
In-Reply-To: <20240328141342.GK7636@redhat.com>

Adjusting cc list to add upstream NBD and drop developers unrelated to
this part of the qemu series...

On Thu, Mar 28, 2024 at 02:13:42PM +0000, Richard W.M. Jones wrote:
> On Thu, Mar 28, 2024 at 03:06:03PM +0100, Thomas Huth wrote:
> > Since version 2.66, glib has useful URI parsing functions, too.
> > Use those instead of the QEMU-internal ones to be finally able
> > to get rid of the latter. The g_uri_get_host() also takes care
> > of removing the square brackets from IPv6 addresses, so we can
> > drop that part of the QEMU code now, too.
> > 

> >  
> >      if (is_unix) {
> >          /* nbd+unix:///export?socket=path */
> > -        if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
> > +        const char *uri_socket = g_hash_table_lookup(qp, "socket");
> > +        if (uri_server || uri_port != -1 || !uri_socket) {
> >              ret = -EINVAL;
> >              goto out;
> >          }

The spec for NBD URIs is at:

https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md

Should any of this spec mention case-insensitive concerns, such as
whether 'NBD://' may be equivalent to 'nbd://', and whether
'nbd+unix:///?socket=x' is equivalent to 'nbd+unix:///?Socket=x'?
Right now, I think that most implementations of NBD servers and
clients happen to use case-sensitive parsing; but glib provides the
option to do case-insensitive query parsing.

If I read https://docs.gtk.org/glib/type_func.Uri.parse_params.html
correctly, passing G_URI_PARAMS_CASE_INSENSITIVE (which you did not
do) would mean that 'nbd+unix:///?socket=ignore&Socket=/for/real'
would result in this g_hash_table_lookup finding only "Socket", not
"socket".  Maybe it is worth an explicit addition to the NBD URI spec
to mention that we intend to be case-sensitive (in the parts where it
can be; I'm not sure if the schema part must be handled
case-insensitively without re-reading the RFCs), and therefore that
'Socket=' does NOT have the same meaning as 'socket='.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org