diff --git a/Makefile.am b/Makefile.am
index 2aa0614f5..ffa92e851 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1199,6 +1199,7 @@ C_TESTS = \
test/test_util \
test/test_byte_reverse \
test/test_mixramp \
+ test/test_icy_parser \
test/test_pcm \
test/test_translate_song \
test/test_queue_priority
@@ -1666,6 +1667,17 @@ test_test_mixramp_LDADD = \
$(GLIB_LIBS) \
$(CPPUNIT_LIBS)
+test_test_icy_parser_SOURCES = \
+ src/Log.cxx src/LogBackend.cxx \
+ test/test_icy_parser.cxx
+test_test_icy_parser_CPPFLAGS = $(AM_CPPFLAGS) $(CPPUNIT_CFLAGS) -DCPPUNIT_HAVE_RTTI=0
+test_test_icy_parser_CXXFLAGS = $(AM_CXXFLAGS) -Wno-error=deprecated-declarations
+test_test_icy_parser_LDADD = \
+ libtag.a \
+ libutil.a \
+ $(GLIB_LIBS) \
+ $(CPPUNIT_LIBS)
+
test_test_pcm_SOURCES = \
src/AudioFormat.cxx \
test/test_pcm_util.hxx \
diff --git a/NEWS b/NEWS
index 2c22aae99..628d50ba3 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,7 @@ ver 0.19 (not yet released)
ver 0.18.8 (not yet released)
* decoder
- ffmpeg: support libav v10_alpha1
+* more robust Icy-Metadata parser
* fix Solaris build failure
ver 0.18.7 (2013/01/13)
diff --git a/doc/developer.xml b/doc/developer.xml
index e9022172c..729e6a513 100644
--- a/doc/developer.xml
+++ b/doc/developer.xml
@@ -155,7 +155,7 @@ foo(const char *abc, int xyz)
<para>
Send your patches to the mailing list:
- musicpd-dev-team@lists.sourceforge.net
+ mpd-devel@musicpd.org
</para>
</chapter>
</book>
diff --git a/src/IcyMetaDataParser.cxx b/src/IcyMetaDataParser.cxx
index d8c12079f..febdb385a 100644
--- a/src/IcyMetaDataParser.cxx
+++ b/src/IcyMetaDataParser.cxx
@@ -82,32 +82,85 @@ icy_add_item(TagBuilder &tag, TagType type, const char *value)
}
static void
-icy_parse_tag_item(TagBuilder &tag, const char *item)
+icy_parse_tag_item(TagBuilder &tag, const char *name, const char *value)
{
- gchar **p = g_strsplit(item, "=", 0);
+ if (strcmp(name, "StreamTitle") == 0)
+ icy_add_item(tag, TAG_TITLE, value);
+ else
+ FormatDebug(icy_metadata_domain,
+ "unknown icy-tag: '%s'", name);
+}
- if (p[0] != nullptr && p[1] != nullptr) {
- if (strcmp(p[0], "StreamTitle") == 0)
- icy_add_item(tag, TAG_TITLE, p[1]);
- else
- FormatDebug(icy_metadata_domain,
- "unknown icy-tag: '%s'", p[0]);
+/**
+ * Find a single quote that is followed by a semicolon (or by the end
+ * of the string). If that fails, return the first single quote. If
+ * that also fails, return #end.
+ */
+static char *
+find_end_quote(char *p, char *const end)
+{
+ char *fallback = std::find(p, end, '\'');
+ if (fallback >= end - 1 || fallback[1] == ';')
+ return fallback;
+
+ p = fallback + 1;
+ while (true) {
+ p = std::find(p, end, '\'');
+ if (p == end)
+ return fallback;
+
+ if (p == end - 1 || p[1] == ';')
+ return p;
+
+ ++p;
}
-
- g_strfreev(p);
}
static Tag *
-icy_parse_tag(const char *p)
+icy_parse_tag(char *p, char *const end)
{
+ assert(p != nullptr);
+ assert(end != nullptr);
+ assert(p <= end);
+
TagBuilder tag;
- gchar **items = g_strsplit(p, ";", 0);
+ while (p != end) {
+ const char *const name = p;
+ char *eq = std::find(p, end, '=');
+ if (eq == end)
+ break;
- for (unsigned i = 0; items[i] != nullptr; ++i)
- icy_parse_tag_item(tag, items[i]);
+ *eq = 0;
+ p = eq + 1;
- g_strfreev(items);
+ if (*p != '\'') {
+ /* syntax error; skip to the next semicolon,
+ try to recover */
+ char *semicolon = std::find(p, end, ';');
+ if (semicolon == end)
+ break;
+ p = semicolon + 1;
+ continue;
+ }
+
+ ++p;
+
+ const char *const value = p;
+ char *quote = find_end_quote(p, end);
+ if (quote == end)
+ break;
+
+ *quote = 0;
+ p = quote + 1;
+
+ icy_parse_tag_item(tag, name, value);
+
+ char *semicolon = std::find(p, end, ';');
+ if (semicolon == end)
+ break;
+ p = semicolon + 1;
+ }
return tag.CommitNew();
}
@@ -154,15 +207,11 @@ IcyMetaDataParser::Meta(const void *data, size_t length)
++length;
if (meta_position == meta_size) {
- /* null-terminate the string */
-
- meta_data[meta_size] = 0;
-
/* parse */
delete tag;
- tag = icy_parse_tag(meta_data);
+ tag = icy_parse_tag(meta_data, meta_data + meta_size);
delete[] meta_data;
/* change back to normal data mode */
diff --git a/test/test_icy_parser.cxx b/test/test_icy_parser.cxx
new file mode 100644
index 000000000..a996df134
--- /dev/null
+++ b/test/test_icy_parser.cxx
@@ -0,0 +1,85 @@
+/*
+ * Unit tests for class IcyMetaDataParser.
+ */
+
+#include "config.h"
+
+/* include the .cxx file to get access to internal functions */
+#include "IcyMetaDataParser.cxx"
+
+#include <cppunit/TestFixture.h>
+#include <cppunit/extensions/TestFactoryRegistry.h>
+#include <cppunit/ui/text/TestRunner.h>
+#include <cppunit/extensions/HelperMacros.h>
+
+#include <string>
+
+#include <string.h>
+
+static Tag *
+icy_parse_tag(const char *p)
+{
+ char *q = strdup(p);
+ Tag *tag = icy_parse_tag(q, q + strlen(q));
+ free(q);
+ return tag;
+}
+
+static void
+CompareTagTitle(const Tag &tag, const std::string &title)
+{
+ CPPUNIT_ASSERT_EQUAL(uint16_t(1), tag.num_items);
+
+ const TagItem &item = *tag.items[0];
+ CPPUNIT_ASSERT_EQUAL(TAG_TITLE, item.type);
+ CPPUNIT_ASSERT_EQUAL(title, std::string(item.value));
+}
+
+static void
+TestIcyParserTitle(const char *input, const char *title)
+{
+ Tag *tag = icy_parse_tag(input);
+ CompareTagTitle(*tag, title);
+ delete tag;
+}
+
+static void
+TestIcyParserEmpty(const char *input)
+{
+ Tag *tag = icy_parse_tag(input);
+ CPPUNIT_ASSERT_EQUAL(uint16_t(0), tag->num_items);
+ delete tag;
+}
+
+class IcyTest : public CppUnit::TestFixture {
+ CPPUNIT_TEST_SUITE(IcyTest);
+ CPPUNIT_TEST(TestIcyMetadataParser);
+ CPPUNIT_TEST_SUITE_END();
+
+public:
+ void TestIcyMetadataParser() {
+ TestIcyParserEmpty("foo=bar;");
+ TestIcyParserTitle("StreamTitle='foo bar'", "foo bar");
+ TestIcyParserTitle("StreamTitle='foo bar';", "foo bar");
+ TestIcyParserTitle("StreamTitle='foo\"bar';", "foo\"bar");
+ TestIcyParserTitle("StreamTitle='foo=bar';", "foo=bar");
+ TestIcyParserTitle("a=b;StreamTitle='foo';", "foo");
+ TestIcyParserTitle("a=;StreamTitle='foo';", "foo");
+ TestIcyParserTitle("a=b;StreamTitle='foo';c=d", "foo");
+ TestIcyParserTitle("a=b;StreamTitle='foo'", "foo");
+ TestIcyParserTitle("a='b;c';StreamTitle='foo;bar'", "foo;bar");
+ TestIcyParserTitle("a='b'c';StreamTitle='foo'bar'", "foo'bar");
+ TestIcyParserTitle("StreamTitle='fo'o'b'ar';a='b'c'd'", "fo'o'b'ar");
+ }
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(IcyTest);
+
+int
+main(gcc_unused int argc, gcc_unused char **argv)
+{
+ CppUnit::TextUi::TestRunner runner;
+ auto ®istry = CppUnit::TestFactoryRegistry::getRegistry();
+ runner.addTest(registry.makeTest());
+ return runner.run() ? EXIT_SUCCESS : EXIT_FAILURE;
+}