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

[Frediano Ziglio]

> 
> 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);

I think if you pass something like "http://[123:" will crash due to uriv[1] being NULL.
Also "[host]1234" or "[host]foo" should not be accepted.

Note that this uri syntax is not standard. The [] quoting is not compulsory.

> +        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;

   uri_port = (uriv[1] != NULL && *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;

There is a bit of g_strsplit and no g_strfreev. Looks like there are some leaks.

> 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));

add "http://[::1", "[host]1234", "[host]foo"

> +    /* 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));
> +    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