From: Tsuda Kageyu Date: Sat, 3 Jan 2015 18:54:46 +0000 (+0900) Subject: Check AIFF/WAV files for duplicate tags. X-Git-Tag: v1.10beta~118^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=30eac7569f095c998e2abb3958398ab57c810e62;p=taglib Check AIFF/WAV files for duplicate tags. AIFF/WAV files can have duplicate tags and it leads to memory leak. --- diff --git a/taglib/riff/aiff/aifffile.cpp b/taglib/riff/aiff/aifffile.cpp index 2f79920c..595b2950 100644 --- a/taglib/riff/aiff/aifffile.cpp +++ b/taglib/riff/aiff/aifffile.cpp @@ -137,16 +137,26 @@ bool RIFF::AIFF::File::hasID3v2Tag() const void RIFF::AIFF::File::read(bool readProperties, Properties::ReadStyle propertiesStyle) { + ByteVector formatData; for(uint i = 0; i < chunkCount(); i++) { - if(chunkName(i) == "ID3 " || chunkName(i) == "id3 ") { - d->tagChunkID = chunkName(i); - d->tag = new ID3v2::Tag(this, chunkOffset(i)); - d->hasID3v2 = true; + const ByteVector name = chunkName(i); + if(name == "ID3 " || name == "id3 ") { + if(!d->tag) { + d->tagChunkID = name; + d->tag = new ID3v2::Tag(this, chunkOffset(i)); + d->hasID3v2 = true; + } + else { + debug("RIFF::AIFF::File::read() - Duplicate ID3v2 tag found."); + } } - else if(chunkName(i) == "COMM" && readProperties) - d->properties = new Properties(chunkData(i), propertiesStyle); + else if(name == "COMM" && readProperties) + formatData = chunkData(i); } if(!d->tag) d->tag = new ID3v2::Tag; + + if(!formatData.isEmpty()) + d->properties = new Properties(formatData, propertiesStyle); } diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp index 6c835e70..4b379fa4 100644 --- a/taglib/riff/wav/wavfile.cpp +++ b/taglib/riff/wav/wavfile.cpp @@ -194,31 +194,39 @@ void RIFF::WAV::File::read(bool readProperties, Properties::ReadStyle properties ByteVector formatData; uint streamLength = 0; for(uint i = 0; i < chunkCount(); i++) { - String name = chunkName(i); + const ByteVector name = chunkName(i); if(name == "ID3 " || name == "id3 ") { - d->tagChunkID = chunkName(i); - d->tag.set(ID3v2Index, new ID3v2::Tag(this, chunkOffset(i))); - d->hasID3v2 = true; + if(!d->tag[ID3v2Index]) { + d->tagChunkID = name; + d->tag.set(ID3v2Index, new ID3v2::Tag(this, chunkOffset(i))); + d->hasID3v2 = true; + } + else { + debug("RIFF::WAV::File::read() - Duplicate ID3v2 tag found."); + } + } + else if(name == "LIST") { + const ByteVector data = chunkData(i); + if(data.mid(0, 4) == "INFO") { + if(!d->tag[InfoIndex]) { + d->tag.set(InfoIndex, new RIFF::Info::Tag(data)); + d->hasInfo = true; + } + else { + debug("RIFF::WAV::File::read() - Duplicate INFO tag found."); + } + } } else if(name == "fmt " && readProperties) formatData = chunkData(i); else if(name == "data" && readProperties) streamLength = chunkDataSize(i); - else if(name == "LIST") { - ByteVector data = chunkData(i); - ByteVector type = data.mid(0, 4); - - if(type == "INFO") { - d->tag.set(InfoIndex, new RIFF::Info::Tag(data)); - d->hasInfo = true; - } - } } - if (!d->tag[ID3v2Index]) + if(!d->tag[ID3v2Index]) d->tag.set(ID3v2Index, new ID3v2::Tag); - if (!d->tag[InfoIndex]) + if(!d->tag[InfoIndex]) d->tag.set(InfoIndex, new RIFF::Info::Tag); if(!formatData.isEmpty()) diff --git a/tests/data/duplicate_id3v2.aiff b/tests/data/duplicate_id3v2.aiff new file mode 100644 index 00000000..6703583f Binary files /dev/null and b/tests/data/duplicate_id3v2.aiff differ diff --git a/tests/data/duplicate_tags.wav b/tests/data/duplicate_tags.wav new file mode 100644 index 00000000..b9865bbd Binary files /dev/null and b/tests/data/duplicate_tags.wav differ diff --git a/tests/test_aiff.cpp b/tests/test_aiff.cpp index 4df8d893..968dbebb 100644 --- a/tests/test_aiff.cpp +++ b/tests/test_aiff.cpp @@ -15,6 +15,7 @@ class TestAIFF : public CppUnit::TestFixture CPPUNIT_TEST(testReading); CPPUNIT_TEST(testSaveID3v2); CPPUNIT_TEST(testAiffCProperties); + CPPUNIT_TEST(testDuplicateID3v2); CPPUNIT_TEST(testFuzzedFile1); CPPUNIT_TEST(testFuzzedFile2); CPPUNIT_TEST_SUITE_END(); @@ -50,8 +51,19 @@ public: { RIFF::AIFF::File f(TEST_FILE_PATH_C("alaw.aifc")); CPPUNIT_ASSERT(f.audioProperties()->isAiffC()); - CPPUNIT_ASSERT(f.audioProperties()->compressionType() == "ALAW"); - CPPUNIT_ASSERT(f.audioProperties()->compressionName() == "SGI CCITT G.711 A-law"); + CPPUNIT_ASSERT_EQUAL(ByteVector("ALAW"), f.audioProperties()->compressionType()); + CPPUNIT_ASSERT_EQUAL(String("SGI CCITT G.711 A-law"), f.audioProperties()->compressionName()); + } + + void testDuplicateID3v2() + { + RIFF::AIFF::File f(TEST_FILE_PATH_C("duplicate_id3v2.aiff")); + + // duplicate_id3v2.aiff has duplicate ID3v2 tags. + // title() returns "Title2" if can't skip the second tag. + + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL(String("Title1"), f.tag()->title()); } void testFuzzedFile1() diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 07b970ee..74440b93 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -96,14 +96,12 @@ public: void testDuplicateID3v2() { - ScopedFileCopy copy("duplicate_id3v2", ".mp3"); - string newname = copy.fileName(); - - MPEG::File f(newname.c_str()); + MPEG::File f(TEST_FILE_PATH_C("duplicate_id3v2.mp3")); // duplicate_id3v2.mp3 has duplicate ID3v2 tags. // Sample rate will be 32000 if can't skip the second tag. + CPPUNIT_ASSERT(f.hasID3v2Tag()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); } diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp index a962ca23..1b71f6ea 100644 --- a/tests/test_wav.cpp +++ b/tests/test_wav.cpp @@ -19,6 +19,7 @@ class TestWAV : public CppUnit::TestFixture CPPUNIT_TEST(testID3v2Tag); CPPUNIT_TEST(testInfoTag); CPPUNIT_TEST(testStripTags); + CPPUNIT_TEST(testDuplicateTags); CPPUNIT_TEST(testFuzzedFile1); CPPUNIT_TEST(testFuzzedFile2); CPPUNIT_TEST_SUITE_END(); @@ -141,6 +142,20 @@ public: delete f; } + void testDuplicateTags() + { + RIFF::WAV::File f(TEST_FILE_PATH_C("duplicate_tags.wav")); + + // duplicate_tags.wav has duplicate ID3v2/INFO tags. + // title() returns "Title2" if can't skip the second tag. + + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL(String("Title1"), f.ID3v2Tag()->title()); + + CPPUNIT_ASSERT(f.hasInfoTag()); + CPPUNIT_ASSERT_EQUAL(String("Title1"), f.InfoTag()->title()); + } + void testFuzzedFile1() { RIFF::WAV::File f1(TEST_FILE_PATH_C("infloop.wav"));