From 8c7c9ad6d437620c10575458cdcda4310daa7e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Mon, 21 Aug 2006 18:34:46 +0000 Subject: [PATCH] gst-libs/gst/tag/gstvorbistag.c: Allow id_data_len == 0 (needed for vorbis comments in Speex files). Original commit message from CVS: * gst-libs/gst/tag/gstvorbistag.c: (gst_tag_list_from_vorbiscomment_buffer): Allow id_data_len == 0 (needed for vorbis comments in Speex files). Also add some checks to make sure we don't memcmp() beyond the end of vorbiscomment buffer if the ID to check for is larger than the buffer. * tests/check/libs/tag.c: (GST_START_TEST): Some more tests for gst_tag_list_from_vorbiscomment_buffer(). --- ChangeLog | 11 ++++ gst-libs/gst/tag/gstvorbistag.c | 7 ++- tests/check/libs/tag.c | 89 ++++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index a5d287dc78..1c7e688f17 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2006-08-21 Tim-Philipp Müller + + * gst-libs/gst/tag/gstvorbistag.c: + (gst_tag_list_from_vorbiscomment_buffer): + Allow id_data_len == 0 (needed for vorbis comments in Speex files). + Also add some checks to make sure we don't memcmp() beyond the end of + vorbiscomment buffer if the ID to check for is larger than the buffer. + + * tests/check/libs/tag.c: (GST_START_TEST): + Some more tests for gst_tag_list_from_vorbiscomment_buffer(). + 2006-08-21 Tim-Philipp Müller * ext/vorbis/vorbisenc.c: (gst_vorbis_enc_metadata_set1), diff --git a/gst-libs/gst/tag/gstvorbistag.c b/gst-libs/gst/tag/gstvorbistag.c index 916d9c7fd7..c6b02d3f9f 100644 --- a/gst-libs/gst/tag/gstvorbistag.c +++ b/gst-libs/gst/tag/gstvorbistag.c @@ -296,16 +296,15 @@ gst_tag_list_from_vorbiscomment_buffer (const GstBuffer * buffer, GstTagList *list; g_return_val_if_fail (GST_IS_BUFFER (buffer), NULL); - g_return_val_if_fail (id_data != NULL, NULL); - g_return_val_if_fail (id_data_length > 0, NULL); + g_return_val_if_fail (id_data != NULL || id_data_length == 0, NULL); data = GST_BUFFER_DATA (buffer); size = GST_BUFFER_SIZE (buffer); list = gst_tag_list_new (); - if (size < 11) + if (size < 11 || size <= id_data_length + 4) goto error; - if (memcmp (data, id_data, id_data_length) != 0) + if (id_data_length > 0 && memcmp (data, id_data, id_data_length) != 0) goto error; ADVANCE (id_data_length); if (vendor_string) diff --git a/tests/check/libs/tag.c b/tests/check/libs/tag.c index 4feaac0a2e..ddeb462a94 100644 --- a/tests/check/libs/tag.c +++ b/tests/check/libs/tag.c @@ -380,6 +380,94 @@ GST_START_TEST (test_vorbis_tags) ASSERT_TAG_LIST_HAS_STRING (list, GST_TAG_LANGUAGE_CODE, "English"); gst_tag_list_free (list); + + /* make sure gst_tag_list_from_vorbiscomment_buffer() works with an + * empty ID (for Speex) */ + { + const guint8 speex_comments_buf1[] = { 0x03, 0x00, 0x00, 0x00, 'f', 'o', + 'o', 0x00, 0x00, 0x00, 0x00 + }; + GstBuffer *buf; + gchar *vendor = NULL; + + buf = gst_buffer_new (); + GST_BUFFER_DATA (buf) = (guint8 *) speex_comments_buf1; + GST_BUFFER_SIZE (buf) = sizeof (speex_comments_buf1); + + /* make sure it doesn't memcmp over the end of the buffer */ + fail_unless (gst_tag_list_from_vorbiscomment_buffer (buf, + (const guint8 *) "averylongstringbrownfoxjumpoverthefence", 39, + &vendor) == NULL); + fail_unless (vendor == NULL); + + /* make sure it bails out if the ID doesn't match */ + fail_unless (gst_tag_list_from_vorbiscomment_buffer (buf, + (guint8 *) "short", 4, &vendor) == NULL); + fail_unless (vendor == NULL); + + /* now read properly */ + list = gst_tag_list_from_vorbiscomment_buffer (buf, NULL, 0, &vendor); + fail_unless (vendor != NULL); + fail_unless_equals_string (vendor, "foo"); + fail_unless (list != NULL); + fail_unless (gst_structure_n_fields ((GstStructure *) list) == 0); + g_free (vendor); + gst_tag_list_free (list); + + /* now again without vendor */ + list = gst_tag_list_from_vorbiscomment_buffer (buf, NULL, 0, NULL); + fail_unless (list != NULL); + fail_unless (gst_structure_n_fields ((GstStructure *) list) == 0); + gst_tag_list_free (list); + + gst_buffer_unref (buf); + } + + /* the same with an ID */ + { + const guint8 vorbis_comments_buf[] = { 0x03, 'v', 'o', 'r', 'b', 'i', 's', + 0x03, 0x00, 0x00, 0x00, 'f', 'o', 'o', 0x01, 0x00, 0x00, 0x00, + strlen ("ARTIST=foo bar"), 0x00, 0x00, 0x00, 'A', 'R', 'T', 'I', 'S', + 'T', '=', 'f', 'o', 'o', ' ', 'b', 'a', 'r' + }; + GstBuffer *buf; + gchar *vendor = NULL; + + buf = gst_buffer_new (); + GST_BUFFER_DATA (buf) = (guint8 *) vorbis_comments_buf; + GST_BUFFER_SIZE (buf) = sizeof (vorbis_comments_buf); + + /* make sure it doesn't memcmp over the end of the buffer */ + fail_unless (gst_tag_list_from_vorbiscomment_buffer (buf, + (const guint8 *) "averylongstringbrownfoxjumpoverthefence", 39, + &vendor) == NULL); + fail_unless (vendor == NULL); + + /* make sure it bails out if the ID doesn't match */ + fail_unless (gst_tag_list_from_vorbiscomment_buffer (buf, + (guint8 *) "short", 4, &vendor) == NULL); + fail_unless (vendor == NULL); + + /* now read properly */ + list = gst_tag_list_from_vorbiscomment_buffer (buf, + (guint8 *) "\003vorbis", 7, &vendor); + fail_unless (vendor != NULL); + fail_unless_equals_string (vendor, "foo"); + fail_unless (list != NULL); + fail_unless (gst_structure_n_fields ((GstStructure *) list) == 1); + ASSERT_TAG_LIST_HAS_STRING (list, GST_TAG_ARTIST, "foo bar"); + g_free (vendor); + gst_tag_list_free (list); + + /* now again without vendor */ + list = gst_tag_list_from_vorbiscomment_buffer (buf, "\003vorbis", 7, NULL); + fail_unless (list != NULL); + fail_unless (gst_structure_n_fields ((GstStructure *) list) == 1); + ASSERT_TAG_LIST_HAS_STRING (list, GST_TAG_ARTIST, "foo bar"); + gst_tag_list_free (list); + + gst_buffer_unref (buf); + } } GST_END_TEST; @@ -423,7 +511,6 @@ GST_START_TEST (test_id3_tags) GST_END_TEST; - static Suite * tag_suite (void) {