From cf86e1a0a00b5e0d8350b481333f696b3004697c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Sat, 8 Dec 2007 12:54:53 +0000 Subject: [PATCH] gst/gststructure.c: Don't crash in _from_string() if the structure name is not valid (fixes #501560). Allow structur... Original commit message from CVS: * gst/gststructure.c: (gst_structure_validate_name), (gst_structure_new_valist), (gst_structure_parse_value), (gst_structure_from_string): Don't crash in _from_string() if the structure name is not valid (fixes #501560). Allow structure names to start with a number again (this apparently broke the ubuntu codec installer). * tests/check/gst/gststructure.c: (GST_START_TEST), (GST_START_TEST), (GST_START_TEST): Add unit test for the crash; update unit tests for new behaviour. --- ChangeLog | 13 +++++++++++++ gst/gststructure.c | 14 ++++++++++---- tests/check/gst/gststructure.c | 29 ++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9885e0eb62..d487edeeb3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2007-12-08 Tim-Philipp Müller + + * gst/gststructure.c: (gst_structure_validate_name), + (gst_structure_new_valist), (gst_structure_parse_value), + (gst_structure_from_string): + Don't crash in _from_string() if the structure name is not valid + (fixes #501560). Allow structure names to start with a number + again (this apparently broke the ubuntu codec installer). + + * tests/check/gst/gststructure.c: (GST_START_TEST), (GST_START_TEST), + (GST_START_TEST): + Add unit test for the crash; update unit tests for new behaviour. + 2007-12-03 Wim Taymans * gst/gstutils.c: diff --git a/gst/gststructure.c b/gst/gststructure.c index e996ca4f56..537bb4ac5c 100644 --- a/gst/gststructure.c +++ b/gst/gststructure.c @@ -145,7 +145,8 @@ gst_structure_validate_name (const gchar * name) g_return_val_if_fail (name != NULL, FALSE); - if (!g_ascii_isalpha (*name)) { + /* FIXME 0.11: use g_ascii_isalpha() */ + if (!g_ascii_isalnum (*name)) { GST_WARNING ("Invalid character '%c' at offset 0 in structure name: %s", *name, name); return FALSE; @@ -236,7 +237,9 @@ gst_structure_new_valist (const gchar * name, g_return_val_if_fail (name != NULL, NULL); structure = gst_structure_empty_new (name); - gst_structure_set_valist (structure, firstfield, varargs); + + if (structure) + gst_structure_set_valist (structure, firstfield, varargs); return structure; } @@ -1883,7 +1886,7 @@ gst_structure_parse_value (gchar * str, * where parsing ended will be returned. * * Returns: a new #GstStructure or NULL when the string could not - * be parsed. Free after usage. + * be parsed. Free with gst_structure_free() after use. */ GstStructure * gst_structure_from_string (const gchar * string, gchar ** end) @@ -1901,7 +1904,7 @@ gst_structure_from_string (const gchar * string, gchar ** end) copy = g_strdup (string); r = copy; - /* skipe spaces */ + /* skip spaces (FIXME: _isspace treats tabs and newlines as space!) */ while (*r && (g_ascii_isspace (*r) || (r[0] == '\\' && g_ascii_isspace (r[1])))) r++; @@ -1917,6 +1920,9 @@ gst_structure_from_string (const gchar * string, gchar ** end) structure = gst_structure_empty_new (name); *w = save; + if (structure == NULL) + goto error; + do { while (*r && (g_ascii_isspace (*r) || (r[0] == '\\' && g_ascii_isspace (r[1])))) diff --git a/tests/check/gst/gststructure.c b/tests/check/gst/gststructure.c index b0e4239219..6e032d2350 100644 --- a/tests/check/gst/gststructure.c +++ b/tests/check/gst/gststructure.c @@ -115,6 +115,17 @@ GST_START_TEST (test_from_string) fail_unless (G_VALUE_HOLDS_BOOLEAN (val)); fail_unless_equals_int (g_value_get_boolean (val), TRUE); gst_structure_free (structure); + + /* This should still work for now (FIXME: 0.11) */ + s = "0.10:decoder-video/mpeg, abc=(boolean)false"; + structure = gst_structure_from_string (s, NULL); + fail_if (structure == NULL, "Could not get structure from string %s", s); + gst_structure_free (structure); + + /* make sure we bail out correctly in case of an error or if parsing fails */ + s = "***foo***, abc=(boolean)false"; + structure = gst_structure_from_string (s, NULL); + fail_unless (structure == NULL); } GST_END_TEST; @@ -131,14 +142,18 @@ GST_START_TEST (test_to_string) #if 0 ASSERT_CRITICAL (st1 = gst_structure_new ("Foo with whitespace", NULL)); fail_unless (st1 == NULL); + ASSERT_CRITICAL (st1 = gst_structure_new ("1st", NULL)); + fail_unless (st1 == NULL); #else st1 = gst_structure_new ("Foo with whitespace is still allowed", NULL); fail_unless (st1 != NULL); gst_structure_free (st1); -#endif - ASSERT_CRITICAL (st1 = gst_structure_new ("1st", NULL)); - fail_unless (st1 == NULL); + /* structure names starting with a number are also still allowed */ + st1 = gst_structure_new ("1st", NULL); + fail_unless (st1 != NULL); + gst_structure_free (st1); +#endif } GST_END_TEST; @@ -239,6 +254,14 @@ GST_START_TEST (test_structure_new) s = gst_structure_new ("name", "key", GST_TYPE_G_ERROR, e, NULL); g_error_free (e); gst_structure_free (s); + + /* This should still work for now (FIXME 0.11) */ + gst_structure_free (gst_structure_new ("0.10:decoder-video/mpeg", NULL)); + + /* make sure we bail out correctly in case of an error or if parsing fails */ + ASSERT_CRITICAL (s = gst_structure_new ("^joo\nba\ndoo^", + "abc", G_TYPE_BOOLEAN, FALSE, NULL)); + fail_unless (s == NULL); } GST_END_TEST;