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@xxxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel