[Spice-devel,spice-gtk] Rewrite URI parsing to work with IPv6 raw addresses

Submitted by Daniel P. Berrangé on April 18, 2012, 3:36 p.m.

Details

Message ID 1334763395-7326-1-git-send-email-berrange@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Daniel P. Berrangé April 18, 2012, 3:36 p.m.
From: "Daniel P. Berrange" <berrange@redhat.com>

The SPICE URI parser did not cope with URI containing
raw IPv6 addresses ie

   spice://[2001:41c8:1:4fd4::2]/?port=5901

This replaces the custom written URI parser with code that
calls out to libxml2's URI APIs which are RFC compliant.

NB, libxml2 will not add/remove the [] around raw addresses,
so we still do that bit ourselves.
---
 configure.ac        |    2 +
 gtk/Makefile.am     |    4 +
 gtk/spice-session.c |  139 ++++++++--------------------
 gtk/spice-uri.c     |  257 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gtk/spice-uri.h     |   45 +++++++++
 5 files changed, 346 insertions(+), 101 deletions(-)
 create mode 100644 gtk/spice-uri.c
 create mode 100644 gtk/spice-uri.h

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 45d99dc..7abe8dc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,6 +17,8 @@  AC_SUBST(GETTEXT_PACKAGE)
 AC_DEFINE_UNQUOTED([GETTEXT_PACKAGE],"$GETTEXT_PACKAGE", [GETTEXT package name])
 AM_GLIB_GNU_GETTEXT
 
+LIBXML2_REQUIRED="2.6.0"
+PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED)
 
 SPICE_GTK_LOCALEDIR=[${datadir}/locale]
 AC_SUBST(SPICE_GTK_LOCALEDIR)
diff --git a/gtk/Makefile.am b/gtk/Makefile.am
index 7b29e61..bbabf66 100644
--- a/gtk/Makefile.am
+++ b/gtk/Makefile.am
@@ -82,6 +82,7 @@  SPICE_COMMON_CPPFLAGS =						\
 	$(GST_CFLAGS)						\
 	$(SMARTCARD_CFLAGS)					\
 	$(USBREDIR_CFLAGS)					\
+	$(LIBXML2_CFLAGS)					\
 	$(NULL)
 
 AM_CPPFLAGS =					\
@@ -179,6 +180,7 @@  libspice_client_glib_2_0_la_LIBADD =					\
 	$(SASL_LIBS)							\
 	$(SMARTCARD_LIBS)						\
 	$(USBREDIR_LIBS)						\
+	$(LIBXML2_LIBS)							\
 	$(NULL)
 
 if WITH_POLKIT
@@ -205,6 +207,8 @@  libspice_client_glib_2_0_la_SOURCES =			\
 	spice-option.c					\
 							\
 	spice-client.c					\
+	spice-uri.h					\
+	spice-uri.c					\
 	spice-session.c					\
 	spice-session-priv.h				\
 	spice-channel.c					\
diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index 02b35f3..0f656fb 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -25,6 +25,7 @@ 
 #include "spice-channel-priv.h"
 #include "spice-util-priv.h"
 #include "spice-session-priv.h"
+#include "spice-uri.h"
 #include "gio-coroutine.h"
 #include "glib-compat.h"
 
@@ -222,114 +223,51 @@  spice_session_finalize(GObject *gobject)
         G_OBJECT_CLASS(spice_session_parent_class)->finalize(gobject);
 }
 
-static int spice_uri_create(SpiceSession *session, char *dest, int len)
+static gchar *spice_session_uri_format(SpiceSession *session)
 {
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
-    int pos = 0;
+    SpiceURI *uri = NULL;
+    gchar *uristr;
 
-    if (s->host == NULL || (s->port == NULL && s->tls_port == NULL)) {
-        return 0;
-    }
+    if (s->host == NULL || (s->port == NULL && s->tls_port == NULL))
+        return NULL;
+
+    uri = spice_uri_new(s->host,
+                        s->port,
+                        s->tls_port,
+                        s->password);
+
+    uristr = spice_uri_format(uri);
 
-    pos += snprintf(dest + pos, len-pos, "spice://%s?", s->host);
-    if (s->port && strlen(s->port))
-        pos += snprintf(dest + pos, len - pos, "port=%s;", s->port);
-    if (s->tls_port && strlen(s->tls_port))
-        pos += snprintf(dest + pos, len - pos, "tls-port=%s;", s->tls_port);
-    return pos;
+    spice_uri_free(uri);
+
+    return uristr;
 }
 
-static int spice_uri_parse(SpiceSession *session, const char *original_uri)
+static gboolean spice_session_uri_parse(SpiceSession *session, const char *uristr, GError **error)
 {
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
-    char host[128], key[32], value[128];
-    char *port = NULL, *tls_port = NULL, *uri = NULL, *password = NULL;
-    char **target_key;
-    int punctuation = 0;
-    int len, pos = 0;
-
-    g_return_val_if_fail(original_uri != NULL, -1);
-
-    uri = g_uri_unescape_string(original_uri, NULL);
-    g_return_val_if_fail(uri != NULL, -1);
+    SpiceURI *uri;
 
-    if (sscanf(uri, "spice://%127[-.0-9a-zA-Z]%n", host, &len) != 1)
-        goto fail;
-    pos += len;
+    if (!uristr)
+        return TRUE;
 
-    if (uri[pos] == '/')
-        pos++;
+    uri = spice_uri_parse(uristr, error);
+    if (!uri)
+        return FALSE;
 
-    for (;;) {
-        if (uri[pos] == '?' || uri[pos] == ';' || uri[pos] == '&') {
-            pos++;
-            punctuation++;
-            continue;
-        }
-        if (uri[pos] == 0) {
-            break;
-        }
-        if (uri[pos] == ':') {
-            if (punctuation++) {
-                g_warning("colon seen after a previous punctuation (?;&:)");
-                goto fail;
-            }
-            pos++;
-            /* port numbers are 16 bit, fits in five decimal figures. */
-            if (sscanf(uri + pos, "%5[0-9]%n", value, &len) != 1)
-                goto fail;
-            port = g_strdup(value);
-            pos += len;
-            continue;
-        } else {
-            if (sscanf(uri + pos, "%31[-a-zA-Z0-9]=%127[^;&]%n", key, value, &len) != 2)
-                goto fail;
-        }
-        pos += len;
-        target_key = NULL;
-        if (g_str_equal(key, "port")) {
-            target_key = &port;
-        } else if (g_str_equal(key, "tls-port")) {
-            target_key = &tls_port;
-        } else if (g_str_equal(key, "password")) {
-            target_key = &password;
-            g_warning("password may be visible in process listings");
-        } else {
-            g_warning("unknown key in spice URI parsing: %s", key);
-            goto fail;
-        }
-        if (target_key) {
-            if (*target_key) {
-                g_warning("double set of %s", key);
-                goto fail;
-            }
-            *target_key = g_strdup(value);
-        }
-    }
+    if (spice_uri_get_hostname(uri))
+        s->host = g_strdup(spice_uri_get_hostname(uri));
+    if (spice_uri_get_port(uri))
+        s->port = g_strdup(spice_uri_get_port(uri));
+    if (spice_uri_get_tls_port(uri))
+        s->tls_port = g_strdup(spice_uri_get_tls_port(uri));
+    if (spice_uri_get_password(uri))
+        s->password = g_strdup(spice_uri_get_password(uri));
 
-    if (port == NULL && tls_port == NULL) {
-        g_warning("missing port or tls-port in spice URI");
-        goto fail;
-    }
+    spice_uri_free(uri);
 
-    /* parsed ok -> apply */
-    g_free(uri);
-    g_free(s->host);
-    g_free(s->port);
-    g_free(s->tls_port);
-    g_free(s->password);
-    s->host = g_strdup(host);
-    s->port = port;
-    s->tls_port = tls_port;
-    s->password = password;
-    return 0;
-
-fail:
-    g_free(uri);
-    g_free(port);
-    g_free(tls_port);
-    g_free(password);
-    return -1;
+    return TRUE;
 }
 
 static void spice_session_get_property(GObject    *gobject,
@@ -339,8 +277,7 @@  static void spice_session_get_property(GObject    *gobject,
 {
     SpiceSession *session = SPICE_SESSION(gobject);
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
-    char buf[256];
-    int len;
+    char *str;
 
     switch (prop_id) {
     case PROP_HOST:
@@ -365,8 +302,9 @@  static void spice_session_get_property(GObject    *gobject,
         g_value_set_int(value, s->protocol);
 	break;
     case PROP_URI:
-        len = spice_uri_create(session, buf, sizeof(buf));
-        g_value_set_string(value, len ? buf : NULL);
+        str = spice_session_uri_format(session);
+        g_value_set_string(value, str);
+        g_free(str);
         break;
     case PROP_CLIENT_SOCKETS:
         g_value_set_boolean(value, s->client_provided_sockets);
@@ -467,8 +405,7 @@  static void spice_session_set_property(GObject      *gobject,
         break;
     case PROP_URI:
         str = g_value_get_string(value);
-        if (str != NULL)
-            spice_uri_parse(session, str);
+        spice_session_uri_parse(session, str, NULL);
         break;
     case PROP_CLIENT_SOCKETS:
         s->client_provided_sockets = g_value_get_boolean(value);
diff --git a/gtk/spice-uri.c b/gtk/spice-uri.c
new file mode 100644
index 0000000..c26930a
--- /dev/null
+++ b/gtk/spice-uri.c
@@ -0,0 +1,257 @@ 
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2012 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "spice-uri.h"
+
+#include <libxml/uri.h>
+#include <string.h>
+
+struct SpiceURIParam {
+    gchar *name;
+    gchar *value;
+};
+
+struct SpiceURI {
+    gchar *hostname;
+    gchar *port;
+    gchar *tlsPort;
+    gchar *password;
+};
+
+SpiceURI *spice_uri_new(const gchar *hostname,
+			const gchar *port,
+			const gchar *tlsPort,
+			const gchar *password)
+{
+    SpiceURI *uri = g_new0(SpiceURI, 1);
+
+    uri->hostname = hostname ? g_strdup(hostname) : NULL;
+    uri->port = port ? g_strdup(port) : NULL;
+    uri->tlsPort = tlsPort ? g_strdup(tlsPort) : NULL;
+    uri->password = password ? g_strdup(password) : NULL;
+
+    return uri;
+}
+
+
+static void spice_uri_parse_params(const gchar *qparams,
+                                   struct SpiceURIParam **params,
+                                   gsize *nparams)
+{
+    const char *end, *eq;
+
+    *params = NULL;
+    *nparams = 0;
+
+    if (!qparams || qparams[0] == '\0')
+        return;
+
+    while (*qparams) {
+        char *name = NULL, *value = NULL;
+
+        /* Find the next separator, or end of the string. */
+        end = strchr (qparams, '&');
+        if (!end)
+            end = strchr (qparams, ';');
+        if (!end)
+            end = qparams + strlen (qparams);
+
+        /* Find the first '=' character between here and end. */
+        eq = strchr (qparams, '=');
+        if (eq && eq >= end) eq = NULL;
+
+        /* Empty section (eg. "&&"). */
+        if (end == qparams)
+            goto next;
+
+        /* If there is no '=' character, then we have just "name"
+         * and consistent with CGI.pm we assume value is "".
+         */
+        else if (!eq) {
+            if (!(name = xmlURIUnescapeString(qparams, end - qparams, NULL)))
+                g_error("Failed to allocate memory for unescaped URI parameter");
+        }
+        /* Or if we have "name=" here (works around annoying
+         * problem when calling xmlURIUnescapeString with len = 0).
+         */
+        else if (eq+1 == end) {
+            if (!(name = xmlURIUnescapeString(qparams, eq - qparams, NULL)))
+                g_error("Failed to allocate memory for unescaped URI parameter");
+        }
+        /* If the '=' character is at the beginning then we have
+         * "=value" and consistent with CGI.pm we _ignore_ this.
+         */
+        else if (qparams == eq)
+            goto next;
+
+        /* Otherwise it's "name=value". */
+        else {
+            if (!(name = xmlURIUnescapeString(qparams, eq - qparams, NULL)))
+                g_error("Failed to allocate memory for unescaped URI parameter");
+            if (!(value = xmlURIUnescapeString(eq+1, end - (eq+1), NULL)))
+                g_error("Failed to allocate memory for unescaped URI parameter");
+        }
+
+        /* Append to the parameter set. */
+        *params = g_renew(struct SpiceURIParam, *params, (*nparams)+1);
+        (*params)[*nparams].name = g_strdup((const gchar *)name);
+        (*params)[*nparams].value = g_strdup((const gchar *)value);
+        (*nparams)++;
+        xmlFree(name);
+        xmlFree(value);
+
+    next:
+        qparams = end;
+        if (*qparams) qparams ++; /* skip '&' separator */
+    }
+ }
+
+SpiceURI *spice_uri_parse(const gchar *uristr, GError **error)
+{
+    SpiceURI *uri = g_new0(SpiceURI, 1);
+    xmlURIPtr xmluri = NULL;
+    struct SpiceURIParam *params = NULL;
+    gsize nparams = 0;
+    gsize i;
+
+    xmluri = xmlParseURI(uristr);
+    if (!xmluri) {
+        g_set_error(error, 0, 0,
+                    "Unable to parse URI %s", uristr);
+        goto error;
+    }
+
+    spice_uri_parse_params(xmluri->query_raw, &params, &nparams);
+
+    if (xmluri->server) {
+        if (xmluri->server[0] == '[') {
+            gchar *tmp;
+            uri->hostname = g_strdup(xmluri->server + 1);
+            if ((tmp = strchr(uri->hostname, ']')))
+                *tmp = '\0';
+        } else {
+            uri->hostname = g_strdup(xmluri->server);
+        }
+    }
+
+    for (i = 0 ; i < nparams ; i++) {
+        if (g_str_equal(params[i].name, "port")) {
+            uri->port = g_strdup(params[i].name);
+        } else if (g_str_equal(params[i].name, "tls-port")) {
+            uri->tlsPort = g_strdup(params[i].name);
+        } else if (g_str_equal(params[i].name, "password")) {
+            uri->password = g_strdup(params[i].name);
+            g_warning("password may be visible in process listings");
+        } else {
+            g_set_error(error, 0, 0,
+                        "unknown key in spice URI parsing: %s",
+                        params[i].name);
+            goto error;
+        }
+    }
+
+ cleanup:
+    for (i = 0 ; i < nparams ; i++) {
+        g_free(params[i].name);
+        g_free(params[i].value);
+    }
+    g_free(params);
+    xmlFreeURI(xmluri);
+    return uri;
+
+ error:
+    spice_uri_free(uri);
+    uri = NULL;
+    goto cleanup;
+}
+
+
+gchar *spice_uri_format(const SpiceURI *uri)
+{
+    xmlURIPtr xmluri = g_new0(xmlURI, 1);
+    xmlChar * xmluristr = NULL;
+    gchar *uristr;
+    GString *qparams = NULL;
+
+    if (strchr(uri->hostname, ':')) {
+        xmluri->server = g_strdup_printf("[%s]", uri->hostname);
+    } else {
+        xmluri->server = g_strdup(uri->hostname);
+    }
+
+    qparams = g_string_new("");
+    if (uri->port) {
+        xmlChar *p = xmlURIEscape((const xmlChar *)uri->port);
+        g_string_append_printf(qparams, "port=%s", (const gchar *)p);
+        xmlFree(p);
+    }
+    if (uri->tlsPort) {
+        xmlChar *p = xmlURIEscape((const xmlChar *)uri->tlsPort);
+        g_string_append_printf(qparams, "tls-port=%s", (const gchar *)p);
+        xmlFree(p);
+    }
+    if (uri->password) {
+        xmlChar *p = xmlURIEscape((const xmlChar *)uri->password);
+        g_string_append_printf(qparams, "password=%s", (const gchar *)p);
+        xmlFree(p);
+    }
+
+    xmluri->query = g_string_free(qparams, FALSE);
+    xmluri->scheme = g_strdup("spice");
+
+    xmluristr = xmlSaveUri(xmluri);
+    uristr = g_strdup((const gchar *)xmluristr);
+
+    xmlFree(xmluristr);
+    xmlFreeURI(xmluri);
+    return uristr;
+}
+
+const gchar *spice_uri_get_hostname(const SpiceURI *uri)
+{
+    return uri->hostname;
+}
+
+const gchar *spice_uri_get_port(const SpiceURI *uri)
+{
+    return uri->port;
+}
+
+const gchar *spice_uri_get_tls_port(const SpiceURI *uri)
+{
+    return uri->tlsPort;
+}
+
+const gchar *spice_uri_get_password(const SpiceURI *uri)
+{
+    return uri->password;
+}
+
+void spice_uri_free(SpiceURI *uri)
+{
+    if (!uri)
+        return;
+
+    g_free(uri->hostname);
+    g_free(uri->port);
+    g_free(uri->tlsPort);
+    g_free(uri->password);
+    g_free(uri);
+}
+
+G_END_DECLS
diff --git a/gtk/spice-uri.h b/gtk/spice-uri.h
new file mode 100644
index 0000000..02add44
--- /dev/null
+++ b/gtk/spice-uri.h
@@ -0,0 +1,45 @@ 
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2012 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#ifndef __SPICE_CLIENT_URI_H__
+#define __SPICE_CLIENT_URI_H__
+
+#include <glib.h>
+
+G_BEGIN_DECLS
+
+typedef struct SpiceURI SpiceURI;
+
+SpiceURI *spice_uri_new(const gchar *hostname,
+			const gchar *port,
+			const gchar *tlsPort,
+			const gchar *password);
+SpiceURI *spice_uri_parse(const gchar *uristr, GError **error);
+gchar *spice_uri_format(const SpiceURI *uri);
+
+const gchar *spice_uri_get_hostname(const SpiceURI *uri);
+const gchar *spice_uri_get_port(const SpiceURI *uri);
+const gchar *spice_uri_get_tls_port(const SpiceURI *uri);
+const gchar *spice_uri_get_password(const SpiceURI *uri);
+
+void spice_uri_free(SpiceURI *uri);
+
+G_END_DECLS
+
+#endif /* __SPICE_CLIENT_URI_H__ */

Comments

----- Mensaje original -----
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> The SPICE URI parser did not cope with URI containing
> raw IPv6 addresses ie
> 
>    spice://[2001:41c8:1:4fd4::2]/?port=5901
> 
> This replaces the custom written URI parser with code that
> calls out to libxml2's URI APIs which are RFC compliant.

I was waiting for GLib GUri API for that. It is a bit sad to add libxml dependency just for it, isn't it?
On Wed, Apr 18, 2012 at 11:52:25AM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Mensaje original -----
> > From: "Daniel P. Berrange" <berrange@redhat.com>
> > 
> > The SPICE URI parser did not cope with URI containing
> > raw IPv6 addresses ie
> > 
> >    spice://[2001:41c8:1:4fd4::2]/?port=5901
> > 
> > This replaces the custom written URI parser with code that
> > calls out to libxml2's URI APIs which are RFC compliant.
> 
> I was waiting for GLib GUri API for that. It is a bit sad to add libxml dependency just for it, isn't it?

Is that really a burden ?  libxml2 exists on pretty much every system
in the world. Even my  TV came with a copy of the Libxml2 README and
LICENSE :-)  virt-viewer / libvirt will already be using it too.

Daniel
----- Mensaje original -----
> Is that really a burden ?  libxml2 exists on pretty much every system
> in the world. Even my  TV came with a copy of the Libxml2 README and
> LICENSE :-)  virt-viewer / libvirt will already be using it too.

It kinda is sad, none of glib/gtk/spice-gtk uses it, afaik. And having just for parsing ipv6 is a bit lame imho.
Marc-André Lureau píše v St 18. 04. 2012 v 12:01 -0400:
> 
> ----- Mensaje original -----
> > Is that really a burden ?  libxml2 exists on pretty much every system
> > in the world. Even my  TV came with a copy of the Libxml2 README and
> > LICENSE :-)  virt-viewer / libvirt will already be using it too.
> 
> It kinda is sad, none of glib/gtk/spice-gtk uses it, afaik. And having just for parsing ipv6 is a bit lame imho.

In addition, ipv6 address passed to remote-viewer via controller works
in both full form and shortened form (prefix::suffix).

David

> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel