[Libguestfs] [libnbd PATCH 0/2] Tighten URI parser

[Martin Kletzander]

On Wed, Jun 26, 2019 at 08:45:28AM -0500, Eric Blake wrote:
>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).
>

Yes, and after you explanation I agree with you.  I'd also prefer having smaller
unambiguous subset working then supporting something confusing.

>--
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3226
>Virtualization:  qemu.org | libvirt.org
>



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190626/efcb5e92/attachment.sig>