From 30eac7569f095c998e2abb3958398ab57c810e62 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 4 Jan 2015 03:54:46 +0900 Subject: [PATCH] Check AIFF/WAV files for duplicate tags. AIFF/WAV files can have duplicate tags and it leads to memory leak. --- taglib/riff/aiff/aifffile.cpp | 22 +++++++++++++----- taglib/riff/wav/wavfile.cpp | 38 +++++++++++++++++++------------- tests/data/duplicate_id3v2.aiff | Bin 0 -> 8124 bytes tests/data/duplicate_tags.wav | Bin 0 -> 17052 bytes tests/test_aiff.cpp | 16 ++++++++++++-- tests/test_mpeg.cpp | 6 ++--- tests/test_wav.cpp | 15 +++++++++++++ 7 files changed, 70 insertions(+), 27 deletions(-) create mode 100644 tests/data/duplicate_id3v2.aiff create mode 100644 tests/data/duplicate_tags.wav 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 0000000000000000000000000000000000000000..6703583f2126062657b1f854e1165d7c84968893 GIT binary patch literal 8124 zcmeI0&2D5x5{0kZ<;E})LM+%auh0!^M9)A=E%7s&_7fmjv;-Symb}p{c{ZT2O>w^X zB5sypBpv{lRIYpTW<;JiaUwFWpML(|Pq*#eAD=vX`t;wQf12y>#n|M3R9iwNNTi^I+4wl^KlfSvIznPcO>+Lf37x_J}r}*Fw zOcz<@&|kdh*VPY=Ni@A?`@fCVZF%Qvd@uqI#}a>|2;y83(adFSxCjsX6Bqz7AJ@Fz z*}>A8u1d^ua~}JA@)zu|HT-P3yQxSAt5#W|XM$#;?zQ9f5L^#~?+_Aib4Au}8xwr8 z(p{XOTlw3cX6E^cuZ3<<-i>#%2>WtzcLbHxx_L_8YF1(~TQkBaalwL`7+%Jk_{SIQ zmHOgoqJc#^@cro4Lgjm_{pPSiC9b zgBYLM%C+8GM^$5>DyBtN>(Eh3eT8MRG6=w-H&&dEtFA_C><7Ql)8da_C%4T{#)z%z zhgJI?54WlHpHuVLia~9_9y%OPT922C2TzU5-SAa>p|xde71*h(n`eIKC0At2`dEF* z49dmCT@_oFDgw(=9sPz$c}9ec8u9l)?=16S%H3U*`kgM?dB}RJ4p`Uc_8l^2os|xPq*2D^ zR9SeypSsJV(u-r(fb5awlbN|&b%bZL?W$mAu4*zGca!Tx$oU11nyX^1YNC8v6;1 zk&AEGG!ML&akl4tCbO*S8mhvx6nhPdz=x*~wL~u%mGe8S$*IwtR%M=-#nQhx9G$(8 z_cLS5)6Bo9eRe8%ItfViJqS>cs-74skC|si$seo~(x4?a)xs&_GOP5;?N_vPsa;d? zRaMrCmB*k!V|g|miKy%ht;Kpqt;iucEU#sC_Y8xkh>TCIRlH74@LXQvX)2hN+&Mjk zFDpG%<{G)EM!$NRo++lm*4Zqc*cDnc_}R0F{fhOJT{%}1qXuwI)~xA?p!g%;%)sYf zSFOcS=~Vl~HHwq5S|>hyZ<|cZ4*RSpD5_L@{w7bcI92MXY2{S4`VKrbj&1FI?ad6x zk|>Q-(WIJA1JN*OiMw$PG!TqXXB z#p)U5iIZr3XIBB_Yk2f()u6gC5|GHQ3d>!iLZ_aZ==JG4nO7+Ol!LOY+>9cKZkSqE zagdMqDtT1{mSXl)hC=NTMA7j#);;p}HC`rScV_=jW3{P;g$N5yy6NQcVMlo*U1d~Z zR#_h$?Bi&)YM?d9U1Hb~nfuX}yL{;=GnnNUtJ5Ae=g-zh&l|nSH&tsLlV9FsF5`m{ z>YtdzQNLM740~5@TbReMk&Wg?1Nq(a$Ns_p{MKHhPJL5qG`I^|73ceqD)7Cm=XNQIY8&Gqh$cziYxYHV&yH3 zlU!%HYEwo*dAIAKiFJ+xJrVlWvCa@!%eXkF!h@y|O+A&Wh^a>pc~5;k{ypz_A|TdQ zPG%LNVrZ1%xrf#05%MbMTIqchQ$3~`R!23`jPE(XQ!#zY^5|#QaYtP55)&0RXhqdg zZLmzWbw%YF%u!ugy)vQ=c0`Nn%%&=X@v6j%udp=}?z8OtUZG{IRnYz)?eaGjYEHJR zkDp)R>e)x{{d3z6ALKVZZ`&^4zj}7{I2W(wd-dYw*Izxldh*G?df>^|Uwr?~qpSaX z{D`UfKKb_Li|<}?cmKI(;GTiMDFZLQ{HreV_>W!W@t?W~UFZID&%iwce^Um22R5}3 AyZ`_I literal 0 HcmV?d00001 diff --git a/tests/data/duplicate_tags.wav b/tests/data/duplicate_tags.wav new file mode 100644 index 0000000000000000000000000000000000000000..b9865bbd57d28df1f68a9d91cc84fb5dca6eab50 GIT binary patch literal 17052 zcmeI(L2ANK5XSM(D%hPK!7C)X>89|C2|Q3M>7%YKS_mW?ecr?acs!5cq+L`kr3c7= z;3b*lO(4I;Fq;%U&yO$C%DrcwbsP;Pb@ZTfS2_)(o`%}2Rl182MoV(^PiE#_AHBO VlYdVn(%-eIz3-~ly39?jisAiffC()); - 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")); -- 2.40.0