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 2f79920c36844ce99de516f8ffb01abffbaf33d2..595b29509ada3960a34378b72aad4ba5ec53c0bc 100644 (file)
--- 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 6c835e702cb323a54753ffcd2a66a95e34608384..4b379fa4b34c5798f47799fd3ef8241d7735a328 100644 (file)
--- 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 (file)
index 0000000..6703583
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 (file)
index 0000000..b9865bb
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 4df8d893d08076e08fbace418b79ed2ec707171d..968dbebbd528cf87795f9427d9456cb42da55c6c 100644 (file)
--- 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 07b970ee0223749e4fd6f29bf3cba111ad1abe88..74440b9338e977462aee0449c84c79adcade6437 100644 (file)
--- 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 a962ca2324d3d0cb6ebdffd02811e1877d1568a7..1b71f6eab2933116f3915c39fb8935d508d44038 100644 (file)
--- 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"));