On Fri, 2016-05-20 at 10:54 +0200, Victor Toso wrote: > Hi, > > On Thu, May 19, 2016 at 06:38:09PM +0200, Pavel Grunt wrote: > > Just basic support - http://user:password@[host]:port > > > > Resolves: rhbz#1335239 > > --- > > src/spice-uri.c | 25 +++++++++++++++++++++---- > > tests/test-spice-uri.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/src/spice-uri.c b/src/spice-uri.c > > index 6a43461..323fbf7 100644 > > --- a/src/spice-uri.c > > +++ b/src/spice-uri.c > > @@ -150,10 +150,29 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar > > *_uri, GError **error) > > uri = next; > > } > > > > - /* max 2 parts, host:port */ > > - gchar **uriv = g_strsplit(uri, ":", 2); > > + gchar **uriv; > > const gchar *uri_port = NULL; > > > > + 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); > > + uri_port = uriv[1]; > > + } > > + > > This seems to leak (I think Frediano said it before) > This should not leak, uriv is initialized by g_strsplit() and in the end g_strfreev(uriv) is called. The leaks come from the patch "spice-uri: Reset SpiceUri before parsing" as you noticed before Thanks! Pavel > Full valgrind in the end. > > > > if (uriv[0] == NULL || strlen(uriv[0]) == 0) { > > g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, > > "Invalid hostname in uri address"); > > @@ -161,8 +180,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 c32a343..9cdcb90 100644 > > --- a/tests/test-spice-uri.c > > +++ b/tests/test-spice-uri.c > > @@ -66,11 +66,53 @@ static void test_spice_uri_ipv4(void) > > g_object_unref(uri); > > } > > > > +static void test_spice_uri_ipv6(void) > > +{ > > + const struct test_case invalid_test_cases[] = { > > + {"http://[]:80";, "http", NULL, 80, NULL, NULL}, > > + {"http://[::1";, "http", NULL, 3128, NULL, NULL}, > > + {"http://[host]1234";, "http", "host", 3128, NULL, NULL}, > > + {"http://[host]foo/";, "http", "host", 3128, NULL, NULL}, > > + {"http://[::1]:port";, "http", "::1", 3128, NULL, NULL}, > > + {"http://[::127.0.0.1]:";, "http", "::127.0.0.1", 3128, NULL, NULL}, > > + {"http://[::127.0.0.1]:-42";, "http", "::127.0.0.1", 3128, NULL, > > NULL}, > > + {"[3ffe:2a00:100:7031::1]:42000000", "http", > > "3ffe:2a00:100:7031::1", 3128, NULL, NULL}, > > + }; > > + const struct test_case valid_test_cases[] = { > > + {"http://user:password@[host]:80/";, "http", "host", 80, "user", > > "password"}, > > + {"http://user@[1080:0:0:0:8:800:200C:4171]:100";, "http", > > "1080:0:0:0:8:800:200C:4171", 100, > > + "user", NULL}, > > + {"https://[1080::8:800:200C:417A]";, "https", > > "1080::8:800:200C:417A", 3129, NULL, NULL}, > > + {"[3ffe:2a00:100:7031::1]", "http", "3ffe:2a00:100:7031::1", 3128, > > NULL, NULL}, > > + }; > > + > > + guint i; > > + > > + SpiceURI *uri = spice_uri_new(); > > + g_assert_nonnull(uri); > > + > > + for (i = 0; i < G_N_ELEMENTS(invalid_test_cases); i++) { > > + g_assert_false(spice_uri_parse(uri, invalid_test_cases[i].uri, > > NULL)); > > + } > > + > > + for (i = 0; i < G_N_ELEMENTS(valid_test_cases); i++) { > > + g_assert_true(spice_uri_parse(uri, valid_test_cases[i].uri, NULL)); > > + g_assert_cmpstr(spice_uri_get_scheme(uri), ==, > > valid_test_cases[i].scheme); > > + g_assert_cmpstr(spice_uri_get_hostname(uri), ==, > > valid_test_cases[i].hostname); > > + g_assert_cmpstr(spice_uri_get_user(uri), ==, > > valid_test_cases[i].user); > > + g_assert_cmpstr(spice_uri_get_password(uri), ==, > > valid_test_cases[i].password); > > + g_assert_cmpuint(spice_uri_get_port(uri), ==, > > valid_test_cases[i].port); > > + } > > + > > + 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(); > > } > > Yeah, I would split the tests with good-uri (should work) and bad-uri > (should fail) > /spice-uri/ipv4/good-uri > /spice-uri/ipv6/good-uri > /spice-uri/ipv4/bad-uri > /spice-uri/ipv6/bad-uri > > Cheers, > toso > > > > -- > > 2.8.2 > > > > _______________________________________________ > > Spice-devel mailing list > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > /spice_uri/ipv6: OK > ==15632== > ==15632== HEAP SUMMARY: > ==15632== in use at exit: 56,764 bytes in 331 blocks > ==15632== total heap usage: 604 allocs, 273 frees, 228,456 bytes > allocated > ==15632== > ==15632== 5 bytes in 1 blocks are definitely lost in loss record 6 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402490: spice_uri_set_user (spice-uri.c:488) > ==15632== by 0x402AFA: spice_uri_parse (spice-uri.c:148) > ==15632== by 0x401985: test_spice_uri_ipv4 (test-spice-uri.c:58) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 5 bytes in 1 blocks are definitely lost in loss record 7 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x402CEF: spice_uri_parse (spice-uri.c:129) > ==15632== by 0x401665: test_spice_uri_ipv6 (test-spice-uri.c:95) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 6 bytes in 1 blocks are definitely lost in loss record 8 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x4029FC: spice_uri_parse (spice-uri.c:126) > ==15632== by 0x401985: test_spice_uri_ipv4 (test-spice-uri.c:58) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 6 bytes in 1 blocks are definitely lost in loss record 9 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x4029FC: spice_uri_parse (spice-uri.c:126) > ==15632== by 0x4016A5: test_spice_uri_ipv6 (test-spice-uri.c:99) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 9 bytes in 1 blocks are definitely lost in loss record 17 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402740: spice_uri_set_password (spice-uri.c:520) > ==15632== by 0x402B05: spice_uri_parse (spice-uri.c:149) > ==15632== by 0x401985: test_spice_uri_ipv4 (test-spice-uri.c:58) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 9 bytes in 1 blocks are definitely lost in loss record 18 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402740: spice_uri_set_password (spice-uri.c:520) > ==15632== by 0x402B05: spice_uri_parse (spice-uri.c:149) > ==15632== by 0x4016A5: test_spice_uri_ipv6 (test-spice-uri.c:99) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 10 bytes in 2 blocks are definitely lost in loss record 20 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x402BA0: spice_uri_parse (spice-uri.c:122) > ==15632== by 0x401985: test_spice_uri_ipv4 (test-spice-uri.c:58) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 10 bytes in 2 blocks are definitely lost in loss record 21 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x402BA0: spice_uri_parse (spice-uri.c:122) > ==15632== by 0x4016A5: test_spice_uri_ipv6 (test-spice-uri.c:99) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 10 bytes in 2 blocks are definitely lost in loss record 22 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402490: spice_uri_set_user (spice-uri.c:488) > ==15632== by 0x402AFA: spice_uri_parse (spice-uri.c:148) > ==15632== by 0x4016A5: test_spice_uri_ipv6 (test-spice-uri.c:99) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 25 bytes in 3 blocks are definitely lost in loss record 128 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402150: spice_uri_set_hostname (spice-uri.c:271) > ==15632== by 0x402C7C: spice_uri_parse (spice-uri.c:182) > ==15632== by 0x401985: test_spice_uri_ipv4 (test-spice-uri.c:58) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 30 bytes in 6 blocks are definitely lost in loss record 129 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x402BA0: spice_uri_parse (spice-uri.c:122) > ==15632== by 0x401945: test_spice_uri_ipv4 (test-spice-uri.c:54) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 35 bytes in 7 blocks are definitely lost in loss record 140 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402020: spice_uri_set_scheme (spice-uri.c:238) > ==15632== by 0x402BA0: spice_uri_parse (spice-uri.c:122) > ==15632== by 0x401665: test_spice_uri_ipv6 (test-spice-uri.c:95) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 40 bytes in 4 blocks are definitely lost in loss record 164 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402150: spice_uri_set_hostname (spice-uri.c:271) > ==15632== by 0x402C7C: spice_uri_parse (spice-uri.c:182) > ==15632== by 0x401945: test_spice_uri_ipv4 (test-spice-uri.c:54) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 50 bytes in 4 blocks are definitely lost in loss record 168 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402150: spice_uri_set_hostname (spice-uri.c:271) > ==15632== by 0x402C7C: spice_uri_parse (spice-uri.c:182) > ==15632== by 0x401665: test_spice_uri_ipv6 (test-spice-uri.c:95) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== 54 bytes in 3 blocks are definitely lost in loss record 169 of > 301 > ==15632== at 0x4C28BF6: malloc (vg_replace_malloc.c:299) > ==15632== by 0x91F9278: g_malloc (gmem.c:94) > ==15632== by 0x9211AEE: g_strdup (gstrfuncs.c:363) > ==15632== by 0x402150: spice_uri_set_hostname (spice-uri.c:271) > ==15632== by 0x402C7C: spice_uri_parse (spice-uri.c:182) > ==15632== by 0x4016A5: test_spice_uri_ipv6 (test-spice-uri.c:99) > ==15632== by 0x9218ADA: test_case_run (gtestutils.c:2158) > ==15632== by 0x9218ADA: g_test_run_suite_internal (gtestutils.c:2241) > ==15632== by 0x9218CA2: g_test_run_suite_internal (gtestutils.c:2253) > ==15632== by 0x9218E9D: g_test_run_suite (gtestutils.c:2328) > ==15632== by 0x9218EC0: g_test_run (gtestutils.c:1596) > ==15632== by 0x4014EF: main (test-spice-uri.c:117) > ==15632== > ==15632== LEAK SUMMARY: > ==15632== definitely lost: 304 bytes in 39 blocks > ==15632== indirectly lost: 0 bytes in 0 blocks > ==15632== possibly lost: 1,440 bytes in 20 blocks > ==15632== still reachable: 54,156 bytes in 266 blocks > ==15632== of which reachable via heuristic: > ==15632== newarray : 1,536 bytes in 16 > blocks > ==15632== suppressed: 0 bytes in 0 blocks > ==15632== Reachable blocks (those to which a pointer was found) are not > shown. > ==15632== To see them, rerun with: --leak-check=full > --show-leak-kinds=all > ==15632== > ==15632== For counts of detected and suppressed errors, rerun with: -v > ==15632== ERROR SUMMARY: 36 errors from 36 contexts (suppressed: 0 from > 0) > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel