[libvirt] [PATCH] util: fix handling of unspecified port in URI

Daniel P. Berrangé posted 1 patch 5 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20181113113657.6819-1-berrange@redhat.com
Test syntax-check passed
src/util/viruri.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
[libvirt] [PATCH] util: fix handling of unspecified port in URI
Posted by Daniel P. Berrangé 5 years, 5 months ago
When no server name is provided in the URI, modern versions of libxml2
will set the port to '-1'. This is a change from behaviour with earlier
versions which set it to 0.

Libvirt expects the port to be 0 in these cases and as a result we get a
bug when connecting to URIs which lack a server name:

$ virsh  -c test+ssh:///default list
error: failed to connect to the hypervisor
error: Cannot recv data: Bad port '-1': Connection reset by peer

This libxml2 change was attempting to fix another bug identified by
libvirt where it didn't roundtrip URIs correctly in:

  https://github.com/GNOME/libxml2/commit/beb7281055dbf0ed4d041022a67c6c5cfd126f25

Essentially libxml2 was not expecting apps to look at the URI port
field when the server name is not provided. This was a reasonable
assumption, but none the less libvirt did look at it :-)

The fix is to ensure we explicitly set port to 0 when server name
is not present, avoiding undefined behaviour for the port field in
libxml2.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/viruri.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/util/viruri.c b/src/util/viruri.c
index d4b793f439..c8811f64c6 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -171,7 +171,16 @@ virURIParse(const char *uri)
         goto error;
     if (VIR_STRDUP(ret->server, xmluri->server) < 0)
         goto error;
-    ret->port = xmluri->port;
+    /* xmluri->port value is not defined if server was
+     * not given. Modern versions libxml2 fill port
+     * differently to old versions in this case, so
+     * don't rely on it. eg libxml2 git commit:
+     *   beb7281055dbf0ed4d041022a67c6c5cfd126f25
+     */
+    if (!ret->server || STREQ(ret->server, ""))
+        ret->port = 0;
+    else
+        ret->port = xmluri->port;
     if (VIR_STRDUP(ret->path, xmluri->path) < 0)
         goto error;
 #ifdef HAVE_XMLURI_QUERY_RAW
-- 
2.19.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: fix handling of unspecified port in URI
Posted by Erik Skultety 5 years, 5 months ago
On Tue, Nov 13, 2018 at 11:36:57AM +0000, Daniel P. Berrangé wrote:
> When no server name is provided in the URI, modern versions of libxml2
> will set the port to '-1'. This is a change from behaviour with earlier
> versions which set it to 0.
>
> Libvirt expects the port to be 0 in these cases and as a result we get a
> bug when connecting to URIs which lack a server name:
>
> $ virsh  -c test+ssh:///default list
> error: failed to connect to the hypervisor
> error: Cannot recv data: Bad port '-1': Connection reset by peer
>
> This libxml2 change was attempting to fix another bug identified by
> libvirt where it didn't roundtrip URIs correctly in:
>
>   https://github.com/GNOME/libxml2/commit/beb7281055dbf0ed4d041022a67c6c5cfd126f25
>
> Essentially libxml2 was not expecting apps to look at the URI port
> field when the server name is not provided. This was a reasonable
> assumption, but none the less libvirt did look at it :-)
>
> The fix is to ensure we explicitly set port to 0 when server name
> is not present, avoiding undefined behaviour for the port field in
> libxml2.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list