[Spice-devel] [PATCH spice-gtk 4/4] spice-uri: Add ipv6 support

[Pavel Grunt]

On Mon, 2016-05-16 at 10:35 -0400, Frediano Ziglio wrote:
> > 
> > 
> > Resolves: rhbz#1335239
> > ---
> >  src/spice-uri.c        | 25 +++++++++++++++++++------
> >  tests/test-spice-uri.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 62 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/spice-uri.c b/src/spice-uri.c
> > index 35d773e..36e75bb 100644
> > --- a/src/spice-uri.c
> > +++ b/src/spice-uri.c
> > @@ -101,7 +101,9 @@ G_GNUC_INTERNAL
> >  gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, GError **error)
> >  {
> >      gchar *dup, *uri;
> > +    gchar **uriv = NULL;
> >      gboolean success = FALSE;
> > +    gboolean ipv6;
> >      size_t len;
> >  
> >      g_return_val_if_fail(self != NULL, FALSE);
> > @@ -130,6 +132,11 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar
> > *_uri, GError **error)
> >          else
> >              break;
> >  
> > +    ipv6 = *uri == '[';
> > +    if (ipv6) {
> > +        uriv = g_strsplit(uri + 1, "]", 2);
> > +        uri = uriv[0];
> > +    }
> >  
> >      /* yes, that parser is bad, we need GUri... */
> >      if (strstr(uri, "@")) {
> > @@ -146,19 +153,25 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar
> > *_uri, GError **error)
> >          spice_uri_set_password(self, NULL);
> >      }
> >  
> > -    /* max 2 parts, host:port */
> > -    gchar **uriv = g_strsplit(uri, ":", 2);
> >      const gchar *uri_port = NULL;
> > +    const gchar *uri_host = NULL;
> > +    if (ipv6) {
> > +        uri_host = uri;
> > +        uri_port = (*uriv[1] == ':') ? uriv[1] + 1 : NULL;
> > +    } else {
> > +        /* max 2 parts, host:port */
> > +        uriv = g_strsplit(uri, ":", 2);
> > +        uri_host = uriv[0];
> > +        uri_port = uriv[1];
> > +    }
> >  
> > -    if (uriv[0] == NULL || strlen(uriv[0]) == 0) {
> > +    if (uri_host == NULL || strlen(uri_host) == 0) {
> >          g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> >                      "Invalid hostname in uri address");
> >          goto end;
> >      }
> >  
> > -    spice_uri_set_hostname(self, uriv[0]);
> > -    if (uriv[0] != NULL)
> > -        uri_port = uriv[1];
> > +    spice_uri_set_hostname(self, uri_host);
> >  
> >      if (uri_port != NULL) {
> >          char *endptr;
> > diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
> > index d556e01..fc379cd 100644
> > --- a/tests/test-spice-uri.c
> > +++ b/tests/test-spice-uri.c
> > @@ -62,11 +62,54 @@ static void test_spice_uri_ipv4(void)
> >      g_object_unref(uri);
> >  }
> >  
> > +static void test_spice_uri_ipv6(void)
> > +{
> > +    SpiceURI *uri = spice_uri_new();
> > +    g_assert_nonnull(uri);
> > +
> > +    /* missing hostname */
> > +    g_assert_false(spice_uri_parse(uri, "http://[]:80", NULL));
> > +    /* invalid port */
> > +    g_assert_false(spice_uri_parse(uri, "http://[::1]:port", NULL));
> > +
> > +    g_assert_true(spice_uri_parse(uri, "http://[::192.9.5.5]/", NULL));
> > +    g_assert_cmpstr(spice_uri_get_scheme(uri), ==, "http");
> > +    g_assert_cmpstr(spice_uri_get_hostname(uri), ==, "::192.9.5.5");
> > +    g_assert_cmpuint(spice_uri_get_port(uri), ==, 3128);
> > +
> > +    g_assert_true(spice_uri_parse(uri, "https://[1080::8:800:200C:417A]",
> > NULL));
> > +    g_assert_cmpstr(spice_uri_get_scheme(uri), ==, "https");
> > +    g_assert_cmpstr(spice_uri_get_hostname(uri), ==,
> > "1080::8:800:200C:417A");
> > +    g_assert_cmpuint(spice_uri_get_port(uri), ==, 3129);
> > +
> > +    g_assert_true(spice_uri_parse(uri, "[3ffe:2a00:100:7031::1]", NULL));
> > +    g_assert_cmpstr(spice_uri_get_scheme(uri), ==, "http");
> > +    g_assert_cmpstr(spice_uri_get_hostname(uri), ==,
> > "3ffe:2a00:100:7031::1");
> > +    g_assert_cmpuint(spice_uri_get_port(uri), ==, 3128);
> > +
> > +    g_assert_true(spice_uri_parse(uri, "http://[user:password@host]:80",
> > NULL));
> 
> http://www.ietf.org/rfc/rfc3986.txt
> 
> The syntax here is http://user:password@[host]:80 (or without []).
> 
> Are we following the standards?

I will update it to follow. Just how to get the port if [] are not used ? But
considering that it is a new thing for spice proxy I would require to use [].

Also I notice that there is another uri parser in spice-session, it would be
worth to merge them.

Thanks,
Pavel

> 
> > +    g_assert_cmpstr(spice_uri_get_scheme(uri), ==, "http");
> > +    g_assert_cmpstr(spice_uri_get_hostname(uri), ==, "host");
> > +    g_assert_cmpstr(spice_uri_get_user(uri), ==, "user");
> > +    g_assert_cmpstr(spice_uri_get_password(uri), ==, "password");
> > +    g_assert_cmpuint(spice_uri_get_port(uri), ==, 80);
> > +
> > +    g_assert_true(spice_uri_parse(uri,
> > "http://[user@1080:0:0:0:8:800:200C:4171]:100", NULL));
> > +    g_assert_cmpstr(spice_uri_get_scheme(uri), ==, "http");
> > +    g_assert_cmpstr(spice_uri_get_hostname(uri), ==,
> > "1080:0:0:0:8:800:200C:4171");
> > +    g_assert_cmpstr(spice_uri_get_user(uri), ==, "user");
> > +    g_assert_cmpstr(spice_uri_get_password(uri), ==, NULL);
> > +    g_assert_cmpuint(spice_uri_get_port(uri), ==, 100);
> > +
> > +    g_object_unref(uri);
> > +}
> > +
> >  int main(int argc, char* argv[])
> >  {
> >      g_test_init(&argc, &argv, NULL);
> >  
> >      g_test_add_func("/spice_uri/ipv4", test_spice_uri_ipv4);
> > +    g_test_add_func("/spice_uri/ipv6", test_spice_uri_ipv6);
> >  
> >      return g_test_run();
> >  }
> > --
> > 2.8.2
> > 
> 
> Frediano