[Qemu-devel] [PATCH 0/2] Fix NBD hostname parsing issues

[Qemu-devel] [PATCH 0/2] Fix NBD hostname parsing issues

[Ján Tomko]

Fix parsing of NBD URIs with IPv6 addresses, broken by v1.4.0-736-gf17c90b
and fix parsing of NBD filenames with hostnames starting with a digit (fixed
by the same commit, but only for NBD URIs):
https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04719.html

Ján Tomko (2):
  qemu-socket: allow hostnames starting with a digit
  nbd: strip braces from literal IPv6 address in URI

 block/nbd.c         | 11 ++++++++++-
 util/qemu-sockets.c | 13 ++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)

-- 
1.8.1.5

[Qemu-devel] [PATCH 1/2] qemu-socket: allow hostnames starting with a digit

[Ján Tomko]

According to RFC 1123 [1], hostnames can start with a digit too.

[1] http://tools.ietf.org/html/rfc1123#page-13

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 util/qemu-sockets.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index fdd8dc4..727dafa 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -24,7 +24,6 @@
 
 #include "monitor/monitor.h"
 #include "qemu/sockets.h"
-#include "qemu-common.h" /* for qemu_isdigit */
 #include "qemu/main-loop.h"
 
 #ifndef AI_ADDRCONFIG
@@ -511,19 +510,15 @@ InetSocketAddress *inet_parse(const char *str, Error **errp)
             goto fail;
         }
         addr->ipv6 = addr->has_ipv6 = true;
-    } else if (qemu_isdigit(str[0])) {
-        /* IPv4 addr */
-        if (2 != sscanf(str, "%64[0-9.]:%32[^,]%n", host, port, &pos)) {
-            error_setg(errp, "error parsing IPv4 address '%s'", str);
-            goto fail;
-        }
-        addr->ipv4 = addr->has_ipv4 = true;
     } else {
-        /* hostname */
+        /* hostname or IPv4 addr */
         if (2 != sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos)) {
             error_setg(errp, "error parsing address '%s'", str);
             goto fail;
         }
+        if (strcspn(host, "0123456789.") == 0) {
+            addr->ipv4 = addr->has_ipv4 = true;
+        }
     }
 
     addr->host = g_strdup(host);
-- 
1.8.1.5

Re: [Qemu-devel] [PATCH 1/2] qemu-socket: allow hostnames starting with a digit

[Paolo Bonzini]

Il 03/06/2013 17:54, Ján Tomko ha scritto:
> According to RFC 1123 [1], hostnames can start with a digit too.
> 
> [1] http://tools.ietf.org/html/rfc1123#page-13
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  util/qemu-sockets.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index fdd8dc4..727dafa 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -24,7 +24,6 @@
>  
>  #include "monitor/monitor.h"
>  #include "qemu/sockets.h"
> -#include "qemu-common.h" /* for qemu_isdigit */
>  #include "qemu/main-loop.h"
>  
>  #ifndef AI_ADDRCONFIG
> @@ -511,19 +510,15 @@ InetSocketAddress *inet_parse(const char *str, Error **errp)
>              goto fail;
>          }
>          addr->ipv6 = addr->has_ipv6 = true;
> -    } else if (qemu_isdigit(str[0])) {
> -        /* IPv4 addr */
> -        if (2 != sscanf(str, "%64[0-9.]:%32[^,]%n", host, port, &pos)) {
> -            error_setg(errp, "error parsing IPv4 address '%s'", str);
> -            goto fail;
> -        }
> -        addr->ipv4 = addr->has_ipv4 = true;
>      } else {
> -        /* hostname */
> +        /* hostname or IPv4 addr */
>          if (2 != sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos)) {
>              error_setg(errp, "error parsing address '%s'", str);
>              goto fail;
>          }
> +        if (strcspn(host, "0123456789.") == 0) {

I think what you want here is:

        if (host[strspn(host, "0123456789.")] == '\0') {

Otherwise, you're still basically testing

	qemu_isdigit(str[0]) || str[0] == '.'

Paolo

> +            addr->ipv4 = addr->has_ipv4 = true;
> +        }
>      }
>  
>      addr->host = g_strdup(host);
> 

Re: [Qemu-devel] [PATCH 1/2] qemu-socket: allow hostnames starting with a digit

[Ján Tomko]

On 06/18/2013 11:42 AM, Paolo Bonzini wrote:
> Il 03/06/2013 17:54, Ján Tomko ha scritto:
>> According to RFC 1123 [1], hostnames can start with a digit too.
>>
>> [1] http://tools.ietf.org/html/rfc1123#page-13
>>
>> Signed-off-by: Ján Tomko <jtomko@redhat.com>
>> ---

>>      } else {
>> -        /* hostname */
>> +        /* hostname or IPv4 addr */
>>          if (2 != sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos)) {
>>              error_setg(errp, "error parsing address '%s'", str);
>>              goto fail;
>>          }
>> +        if (strcspn(host, "0123456789.") == 0) {
> 
> I think what you want here is:
> 
>         if (host[strspn(host, "0123456789.")] == '\0') {
> 

Yes, thank you for catching that.

Jan

> Otherwise, you're still basically testing
> 
> 	qemu_isdigit(str[0]) || str[0] == '.'
> 
> Paolo
> 
>> +            addr->ipv4 = addr->has_ipv4 = true;
>> +        }
>>      }
>>  
>>      addr->host = g_strdup(host);
>>
> 
> 

[Qemu-devel] [PATCH 2/2] nbd: strip braces from literal IPv6 address in URI

[Ján Tomko]

Otherwise they would get passed to getaddrinfo and fail with:
address resolution failed for [::1]:1234: Name or service not known

(Broken by commit v1.4.0-736-gf17c90b)

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 block/nbd.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/block/nbd.c b/block/nbd.c
index 30e3b78..9c480b8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -118,13 +118,22 @@ static int nbd_parse_uri(const char *filename, QDict *options)
         }
         qdict_put(options, "path", qstring_from_str(qp->p[0].value));
     } else {
+        QString *host;
         /* nbd[+tcp]://host[:port]/export */
         if (!uri->server) {
             ret = -EINVAL;
             goto out;
         }
 
-        qdict_put(options, "host", qstring_from_str(uri->server));
+        /* strip braces from literal IPv6 address */
+        if (uri->server[0] == '[') {
+            host = qstring_from_substr(uri->server, 1,
+                                       strlen(uri->server) - 2);
+        } else {
+            host = qstring_from_str(uri->server);
+        }
+
+        qdict_put(options, "host", host);
         if (uri->port) {
             char* port_str = g_strdup_printf("%d", uri->port);
             qdict_put(options, "port", qstring_from_str(port_str));
-- 
1.8.1.5

Re: [Qemu-devel] [PATCH 2/2] nbd: strip braces from literal IPv6 address in URI

[Paolo Bonzini]

Il 03/06/2013 17:54, Ján Tomko ha scritto:
> Otherwise they would get passed to getaddrinfo and fail with:
> address resolution failed for [::1]:1234: Name or service not known

Hmm... Hai Huang found a similar problem:

error: internal error unable to execute QEMU command 'nbd-server-start':
address resolution failed for [::]:5900: Name or service not known

This one is a libvirt bug, but perhaps it's simpler to just have a
wrapper for getaddrinfo that strips brackets (and not strip the brackets
in inet_parse, too).

Paolo

> (Broken by commit v1.4.0-736-gf17c90b)
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  block/nbd.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 30e3b78..9c480b8 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -118,13 +118,22 @@ static int nbd_parse_uri(const char *filename, QDict *options)
>          }
>          qdict_put(options, "path", qstring_from_str(qp->p[0].value));
>      } else {
> +        QString *host;
>          /* nbd[+tcp]://host[:port]/export */
>          if (!uri->server) {
>              ret = -EINVAL;
>              goto out;
>          }
>  
> -        qdict_put(options, "host", qstring_from_str(uri->server));
> +        /* strip braces from literal IPv6 address */
> +        if (uri->server[0] == '[') {
> +            host = qstring_from_substr(uri->server, 1,
> +                                       strlen(uri->server) - 2);
> +        } else {
> +            host = qstring_from_str(uri->server);
> +        }
> +
> +        qdict_put(options, "host", host);
>          if (uri->port) {
>              char* port_str = g_strdup_printf("%d", uri->port);
>              qdict_put(options, "port", qstring_from_str(port_str));
> 

Re: [Qemu-devel] [PATCH 2/2] nbd: strip braces from literal IPv6 address in URI

[Ján Tomko]

On 06/04/2013 11:27 PM, Paolo Bonzini wrote:
> Il 03/06/2013 17:54, Ján Tomko ha scritto:
>> Otherwise they would get passed to getaddrinfo and fail with:
>> address resolution failed for [::1]:1234: Name or service not known
> 
> Hmm... Hai Huang found a similar problem:
> 
> error: internal error unable to execute QEMU command 'nbd-server-start':
> address resolution failed for [::]:5900: Name or service not known
> 

That one should be fixed in libvirt-1.0.5.2 and libvirt-1.0.6 now. [1]

> This one is a libvirt bug, but perhaps it's simpler to just have a
> wrapper for getaddrinfo that strips brackets (and not strip the brackets
> in inet_parse, too).

I'm not sure if there are places other than calling getaddrinfo where having
brackets would be bad, I'm not that familiar with QEMU code.

I've thought about stripping the brackets in uri_parse too, as we do in
libvirt when parsing URIs. This wouldn't fix the case of someone specifying a
bracket-ecaped hostname where there is no need for escaping it, like the
libvirt bug.

And my patch is incomplete, because there are other block drivers calling
uri_parse without stripping the brackets.

Jan

[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3accd7eb

Re: [Qemu-devel] [PATCH 0/2] Fix NBD hostname parsing issues

[Paolo Bonzini]

Il 03/06/2013 17:54, Ján Tomko ha scritto:
> Fix parsing of NBD URIs with IPv6 addresses, broken by v1.4.0-736-gf17c90b
> and fix parsing of NBD filenames with hostnames starting with a digit (fixed
> by the same commit, but only for NBD URIs):
> https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04719.html
> 
> Ján Tomko (2):
>   qemu-socket: allow hostnames starting with a digit
>   nbd: strip braces from literal IPv6 address in URI
> 
>  block/nbd.c         | 11 ++++++++++-
>  util/qemu-sockets.c | 13 ++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 

Thanks, applied both to nbd-next tree.

Paolo