Re: [Libguestfs] [libnbd PATCH 0/2] Tighten URI parser
On 6/26/19 4:05 AM, Martin Kletzander wrote:
On Tue, Jun 25, 2019 at 09:09:58PM -0500, Eric Blake wrote:
> I'm not sure whether we want to go with just the first patch (reject
> nbd:unix:/path but still accept nbd:/path), or squash the two in order
> to go with the second (reject both abbreviated forms, and require
> scheme://...). Either way, though, nbdkit -U - --run '$nbd' will now
> error out rather than inadvertently connect over TCP to
> localhost:10809 instead of the intended Unix connection (and in the
> meantime, you want to use --run '$unixsocket', or maybe we should
> teach nbdkit to support --run '$uri').
>
Oh, I get it now. So "nbd:unix:/path" is a valid URL without authority
where the
path is "unix:/path". Well, TIL =)
It is nicely visible here:
https://tools.ietf.org/html/rfc3986#section-3
These patches are fine from my standpoint, although I must say thinking
about
the above as defined in the RFC makes me re-evaluate my feelings about it.
Having URI without authority just use a local file (socket) makes sense
now.
Except that our URI proposal is stating that we want:
nbd+unix:///exportname?socket=/path/to/sock
Using a mere nbd+unix:/path/to/sock would be wrong, as it would treat
'/path/to/sock' as the export name, not the socket name.
Even having the unix: prefix to show that it is a UNIX socket seems
sensible.
At least unless you realize that having an actual URI it makes sense to
have
that as a part of the scheme, as is customary.
What I am trying to say is that maybe just the first patch is enough. When
people want to just write, for example nbd:/tmp/my_nbd.sock by hand, it is
easier than typing the whole nbd+unix:?socket=/tmp/my_nbd.sock. On the
other
hand, how often do you need to write it manually...
My worry is that nbd:/path/to/sock is wrong for use as a Unix socket
connection, because it leaves no place for the export name (even if the
export name is ''). It is too confusing if 'nbd:export' sometimes
treats the path part as the export name, and sometimes as the socket
name, so I'd rather that we ALWAYS treat the path part as the export name.
Anyway, I don't really have strong feelings about this, so whatever you
choose
is fine as long as nbd:unix:/path does not try connecting to localhost over
network =)
> Eric Blake (2):
> uri: Reject nbd:unix:/path/to/socket as invalid URI
> uri: Reject nbd:unix:/path/to/socket as invalid URI
The second one should say "Reject nbd:/path/to/socket as invalid URL", I
guess.
My actual plan is to squash it into a single patch; I just posted as two
patches in case we only wanted to go halfway (the first patch). But
given this conversation, you've convinced me that it is more confusing
to allow anything not of the form nbd://, even if it is valid as a URI.
(That is, we are explicitly declaring that when using the nbd: or
nbd+unix: scheme, we have additional restrictions that make only a
subset of valid URIs usable in our scheme).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)