[Spice-devel] [PATCH spice-gtk v4 8/8] spice-uri: Add ipv6 support
Victor Toso
lists at victortoso.com
Tue May 31 08:57:52 UTC 2016
Hi,
On Mon, May 30, 2016 at 05:46:50PM +0200, Pavel Grunt wrote:
> Just basic support - http://user:password@[host]:port
>
> Resolves: rhbz#1335239
> ---
> src/spice-uri.c | 24 +++++++++++---
> tests/test-spice-uri.c | 90 +++++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 87 insertions(+), 27 deletions(-)
>
> diff --git a/src/spice-uri.c b/src/spice-uri.c
> index 83ebe79..0376cd8 100644
> --- a/src/spice-uri.c
> +++ b/src/spice-uri.c
> @@ -161,8 +161,26 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, GError **error)
> uri = next;
> }
>
> - /* max 2 parts, host:port */
> - uriv = g_strsplit(uri, ":", 2);
> + if (*uri == '[') { /* ipv6 address */
> + uriv = g_strsplit(uri + 1, "]", 2);
> + if (uriv[1] == NULL) {
> + g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> + "Missing ']' in ipv6 uri");
> + goto end;
> + }
> + if (*uriv[1] == ':') {
> + uri_port = uriv[1] + 1;
> + } else if (strlen(uriv[1]) > 0) { /* invalid string after the hostname */
> + g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> + "Invalid uri address");
> + goto end;
> + }
> + } else {
> + /* max 2 parts, host:port */
> + uriv = g_strsplit(uri, ":", 2);
> + if (uriv[0] != NULL)
> + uri_port = uriv[1];
> + }
>
Looks fine
> if (uriv[0] == NULL || strlen(uriv[0]) == 0) {
> g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> @@ -171,8 +189,6 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, GError **error)
> }
>
> spice_uri_set_hostname(self, uriv[0]);
> - if (uriv[0] != NULL)
> - uri_port = uriv[1];
>
> if (uri_port != NULL) {
> gchar *endptr;
> diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
> index 51f652c..d7aee09 100644
> --- a/tests/test-spice-uri.c
> +++ b/tests/test-spice-uri.c
> @@ -29,26 +29,14 @@ struct test_case {
> gchar *error_msg;
> };
>
> -static void test_spice_uri_ipv4_bad(void)
> +static void test_spice_uri_bad(const struct test_case invalid_test_cases[], const guint cases_cnt)
> {
> - const struct test_case invalid_test_cases[] = {
> - {"http://:80", "http", NULL, 80, NULL, NULL, "Invalid hostname in uri address"},
> - {"http://", "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri address"},
> - {"http://127.0.0.1:port", "http", "127.0.0.1", 3128, NULL, NULL,
> - "Invalid uri port: port"},
> - {"http://127.0.0.1:", "http", "127.0.0.1", 3128, NULL, NULL, "Missing uri port"},
> - {"http://127.0.0.1:-80", "http", "127.0.0.1", 3128, NULL, NULL, "Port out of range"},
> - {"http://127.0.0.1:8000000", "http", "127.0.0.1", 3128, NULL, NULL, "Port out of range"},
> - {"scheme://192.168.1.1:3128", "http", "127.0.0.1", 3128, NULL, NULL,
> - "Invalid uri scheme for proxy: scheme"},
> - };
> -
> guint i;
>
> SpiceURI *uri = spice_uri_new();
> g_assert_nonnull(uri);
>
> - for (i = 0; i < G_N_ELEMENTS(invalid_test_cases); i++) {
> + for (i = 0; i < cases_cnt; i++) {
If you are touching the tests again I would say that you could have the
helper functions on the first patch and here only include the ipv6 ones.
But as this is just tests I'm don't think this effort is really
necessary. Thanks for writing those tests!
Acked-by: Victor Toso <victortoso at redhat.com>
> GError *error = NULL;
> g_assert_false(spice_uri_parse(uri, invalid_test_cases[i].uri, &error));
> g_assert_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED);
> @@ -59,21 +47,14 @@ static void test_spice_uri_ipv4_bad(void)
> g_object_unref(uri);
> }
>
> -static void test_spice_uri_ipv4_good(void)
> +static void test_spice_uri_good(const struct test_case valid_test_cases[], const guint cases_cnt)
> {
> - const struct test_case valid_test_cases[] = {
> - {"http://user:password@host:80", "http", "host", 80, "user", "password", NULL},
> - {"http://127.0.0.1/", "http", "127.0.0.1", 3128, NULL, NULL, NULL}, /* reset user & password */
> - {"https://127.0.0.1", "https", "127.0.0.1", 3129, NULL, NULL, NULL},
> - {"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL, NULL},
> - };
> -
> guint i;
>
> SpiceURI *uri = spice_uri_new();
> g_assert_nonnull(uri);
>
> - for (i = 0; i < G_N_ELEMENTS(valid_test_cases); i++) {
> + for (i = 0; i < cases_cnt; i++) {
> GError *error = NULL;
> g_assert_true(spice_uri_parse(uri, valid_test_cases[i].uri, &error));
> g_assert_cmpstr(spice_uri_get_scheme(uri), ==, valid_test_cases[i].scheme);
> @@ -87,12 +68,75 @@ static void test_spice_uri_ipv4_good(void)
> g_object_unref(uri);
> }
>
> +
> +static void test_spice_uri_ipv4_bad(void)
> +{
> + const struct test_case invalid_test_cases[] = {
> + {"http://:80", "http", NULL, 80, NULL, NULL, "Invalid hostname in uri address"},
> + {"http://", "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri address"},
> + {"http://127.0.0.1:port", "http", "127.0.0.1", 3128, NULL, NULL,
> + "Invalid uri port: port"},
> + {"http://127.0.0.1:", "http", "127.0.0.1", 3128, NULL, NULL, "Missing uri port"},
> + {"http://127.0.0.1:-80", "http", "127.0.0.1", 3128, NULL, NULL, "Port out of range"},
> + {"http://127.0.0.1:8000000", "http", "127.0.0.1", 3128, NULL, NULL, "Port out of range"},
> + {"scheme://192.168.1.1:3128", "http", "127.0.0.1", 3128, NULL, NULL,
> + "Invalid uri scheme for proxy: scheme"},
> + };
> +
> + test_spice_uri_bad(invalid_test_cases, G_N_ELEMENTS(invalid_test_cases));
> +}
> +
> +static void test_spice_uri_ipv4_good(void)
> +{
> + const struct test_case valid_test_cases[] = {
> + {"http://user:password@host:80", "http", "host", 80, "user", "password", NULL},
> + {"http://127.0.0.1/", "http", "127.0.0.1", 3128, NULL, NULL, NULL}, /* reset user & password */
> + {"https://127.0.0.1", "https", "127.0.0.1", 3129, NULL, NULL, NULL},
> + {"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL, NULL},
> + };
> +
> + test_spice_uri_good(valid_test_cases, G_N_ELEMENTS(valid_test_cases));
> +}
> +
> +static void test_spice_uri_ipv6_bad(void)
> +{
> + const struct test_case invalid_test_cases[] = {
> + {"http://[]:80", "http", NULL, 80, NULL, NULL, "Invalid hostname in uri address"},
> + {"http://[::1", "http", NULL, 3128, NULL, NULL, "Missing ']' in ipv6 uri"},
> + {"http://[host]1234", "http", "host", 3128, NULL, NULL, "Invalid uri address"},
> + {"http://[host]foo/", "http", "host", 3128, NULL, NULL, "Invalid uri address"},
> + {"http://[::1]:port", "http", "::1", 3128, NULL, NULL, "Invalid uri port: port"},
> + {"http://[::127.0.0.1]:", "http", "::127.0.0.1", 3128, NULL, NULL, "Missing uri port"},
> + {"http://[::127.0.0.1]:-42", "http", "::127.0.0.1", 3128, NULL, NULL, "Port out of range"},
> + {"[3ffe:2a00:100:7031::1]:42000000", "http", "3ffe:2a00:100:7031::1", 3128, NULL, NULL, "Port out of range"},
> + {"scheme://[3ffe::192.168.1.1]:3128", "http", "3ffe::192.168.1.1", 3128, NULL, NULL,
> + "Invalid uri scheme for proxy: scheme"},
> + };
> +
> + test_spice_uri_bad(invalid_test_cases, G_N_ELEMENTS(invalid_test_cases));
> +}
> +
> +static void test_spice_uri_ipv6_good(void)
> +{
> + const struct test_case valid_test_cases[] = {
> + {"http://user:password@[host]:80/", "http", "host", 80, "user", "password", NULL},
> + {"http://user@[1080:0:0:0:8:800:200C:4171]:100", "http", "1080:0:0:0:8:800:200C:4171", 100,
> + "user", NULL, NULL},
> + {"https://[1080::8:800:200C:417A]", "https", "1080::8:800:200C:417A", 3129, NULL, NULL, NULL},
> + {"[3ffe:2a00:100:7031::1]", "http", "3ffe:2a00:100:7031::1", 3128, NULL, NULL, NULL},
> + };
> +
> + test_spice_uri_good(valid_test_cases, G_N_ELEMENTS(valid_test_cases));
> +}
> +
> int main(int argc, char* argv[])
> {
> g_test_init(&argc, &argv, NULL);
>
> g_test_add_func("/spice_uri/ipv4/bad-uri", test_spice_uri_ipv4_bad);
> g_test_add_func("/spice_uri/ipv4/good-uri", test_spice_uri_ipv4_good);
> + g_test_add_func("/spice_uri/ipv6/bad-uri", test_spice_uri_ipv6_bad);
> + g_test_add_func("/spice_uri/ipv6/good-uri", test_spice_uri_ipv6_good);
>
> return g_test_run();
> }
> --
> 2.8.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list