Fix NBD hostname parsing issues
cover Fix NBD hostname parsing issues
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(-)
Signature · 1 line
1.8.1.5
PATCH 1/2 qemu-socket: allow hostnames starting with a digit
According to RFC 1123 [1], hostnames can start with a digit too.
[1] http://tools.ietf.org/html/rfc1123#page-13
--- util/qemu-sockets.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
util/qemu-sockets.c +4 -9
@@ -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);
Signature · 1 line
1.8.1.5
Re: [Qemu-devel] [PATCH 1/2] qemu-socket: allow hostnames starting with a digit
Il 03/06/2013 17:54, Ján Tomko ha scritto:
Quoted 30 lines
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 <[email protected]>--- util/qemu-sockets.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)util/qemu-sockets.c +0
-6@@ -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;
util/qemu-sockets.c InetSocketAddress *inet_parse(const char *str, Error **errp) · line 519
Quoted 10 lines
util/qemu-sockets.c +2
-3- } - 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
Quoted 6 lines
util/qemu-sockets.c +2
-0+ 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
Quoted 17 lines · Paolo Bonzini
Il 03/06/2013 17:54, Ján Tomko ha scritto:
Quoted 8 lines
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 <[email protected]>---
} else {util/qemu-sockets.c InetSocketAddress *inet_parse(const char *str, Error **errp) · line 519
Quoted 7 lines
util/qemu-sockets.c +2
-1-/* 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 ) {
Quoted 4 lines
I think what you want here is:
if (host[strspn(host, "0123456789.")] == '\0') {
Yes, thank you for catching that.
Jan
Quoted 14 lines
Otherwise, you're still basically testing
qemu_isdigit(str[0]) || str[0] == '.'Paolo
Quoted 6 lines
util/qemu-sockets.c +2
-0+ addr->ipv4 = addr->has_ipv4 = true; + } } addr->host =g_strdup (host);
PATCH 2/2 nbd: strip braces from literal IPv6 address in URI
In reply to Ján TomkoOtherwise 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)
--- block/nbd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
block/nbd.c +10 -1
@@ -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));
Signature · 1 line
1.8.1.5
Re: [Qemu-devel] [PATCH 2/2] nbd: strip braces from literal IPv6 address in URI
Il 03/06/2013 17:54, Ján Tomko ha scritto:
Quoted 2 lines
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
Quoted 36 lines
(Broken by commit v1.4.0-736-gf17c90b)
Signed-off-by Ján Tomko <[email protected]>--- block/nbd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)block/nbd.c +10
-1@@ -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
Quoted 9 lines · Paolo Bonzini
Il 03/06/2013 17:54, Ján Tomko ha scritto:
Quoted 2 lines
Otherwise they would get passed to getaddrinfo and fail with:
address resolution failed for [::1]:1234: Name or service not knownHmm... 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]
Quoted 3 lines
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
Paolo Bonzini
AppliedIl 03/06/2013 17:54, Ján Tomko ha scritto:
Quoted 5 lines
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
Quoted 8 lines
Ján Tomko (2):
qemu-socket: allow hostnames starting with a digit
nbd: strip braces from literal IPv6 address in URIblock/nbd.c | 11 ++++++++++- util/qemu-sockets.c | 13 ++++--------- 2 files changed, 14 insertions(+), 10 deletions(-)
Thanks, applied both to nbd-next tree.
Paolo