From b98a984b66f69cacfcbf259ac3b2d77ea3c9415f Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 11 Nov 2016 00:07:32 +0900 Subject: [PATCH] Fix handling of lowercase 'metadata_block_picture' fields in Vorbis comments. Also refactored some redundant code for parsing pictures. --- taglib/ogg/xiphcomment.cpp | 94 +++++++++++++------------------- tests/data/lowercase-fields.ogg | Bin 0 -> 4477 bytes tests/test_xiphcomment.cpp | 18 ++++++ 3 files changed, 57 insertions(+), 55 deletions(-) create mode 100644 tests/data/lowercase-fields.ogg diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index d6b8e11c..f56bf810 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -447,85 +447,69 @@ void Ogg::XiphComment::parse(const ByteVector &data) const unsigned int commentLength = data.toUInt(pos, false); pos += 4; - ByteVector entry = data.mid(pos, commentLength); - + const ByteVector entry = data.mid(pos, commentLength); pos += commentLength; // Don't go past data end + if(pos > data.size()) break; - // Handle Pictures separately - if(entry.startsWith("METADATA_BLOCK_PICTURE=")) { + // Check for field separator - // We need base64 encoded data including padding - if((entry.size() - 23) > 3 && ((entry.size() - 23) % 4) == 0) { + const int sep = entry.find('='); + if(sep < 1) { + debug("Ogg::XiphComment::parse() - Discarding a field. Separator not found."); + continue; + } - // Decode base64 picture data - ByteVector picturedata = ByteVector::fromBase64(entry.mid(23)); - if(picturedata.size()) { + // Parse the key - // Decode Flac Picture - FLAC::Picture * picture = new FLAC::Picture(); - if(picture->parse(picturedata)) { + const String key = String(entry.mid(0, sep), String::UTF8).upper(); + if(!checkKey(key)) { + debug("Ogg::XiphComment::parse() - Discarding a field. Invalid key."); + continue; + } - d->pictureList.append(picture); + if(key == "METADATA_BLOCK_PICTURE" || key == "COVERART") { - // continue to next field - continue; - } - else { - delete picture; - debug("Failed to decode FlacPicture block"); - } - } - else { - debug("Failed to decode base64 encoded data"); - } - } - else { - debug("Invalid base64 encoded data"); - } - } + // Handle Pictures separately - // Handle old picture standard - if(entry.startsWith("COVERART=")) { + const ByteVector picturedata = ByteVector::fromBase64(entry.mid(sep + 1)); + if(picturedata.isEmpty()) { + debug("Ogg::XiphComment::parse() - Discarding a field. Invalid base64 data"); + continue; + } - if((entry.size() - 9) > 3 && ((entry.size() - 9) % 4) == 0) { + if(key[0] == L'M') { - // Decode base64 picture data - ByteVector picturedata = ByteVector::fromBase64(entry.mid(9)); - if (picturedata.size()) { + // Decode FLAC Picture - // Assume it's some type of image file - FLAC::Picture * picture = new FLAC::Picture(); - picture->setData(picturedata); - picture->setMimeType("image/"); - picture->setType(FLAC::Picture::Other); + FLAC::Picture * picture = new FLAC::Picture(); + if(picture->parse(picturedata)) { d->pictureList.append(picture); - - // continue to next field - continue; } else { - debug("Failed to decode base64 encoded data"); + delete picture; + debug("Ogg::XiphComment::parse() - Failed to decode FLAC Picture block"); } } else { - debug("Invalid base64 encoded data"); + + // Assume it's some type of image file + + FLAC::Picture * picture = new FLAC::Picture(); + picture->setData(picturedata); + picture->setMimeType("image/"); + picture->setType(FLAC::Picture::Other); + d->pictureList.append(picture); } } + else { - // Check for field separator - int sep = entry.find('='); - if(sep < 1) { - debug("Discarding invalid comment field."); - continue; - } + // Parse the text - // Parse key and value - String key = String(entry.mid(0, sep), String::UTF8); - String value = String(entry.mid(sep + 1), String::UTF8); - addField(key, value, false); + addField(key, String(entry.mid(sep + 1), String::UTF8), false); + } } } diff --git a/tests/data/lowercase-fields.ogg b/tests/data/lowercase-fields.ogg new file mode 100644 index 0000000000000000000000000000000000000000..0ddd49357a906c70d6f3e5f222f0fa5404e4e2cb GIT binary patch literal 4477 zcmeHLeNatHl+?=l0qu3&|>lZIIRhca`BP{0# z{ezHojK%z(U=`3a9~62fJ@Qrimjm3>|NMSg5gyXC76eMnjddFenrej&=Bi>wzfdF0 zkjXM-8)fov5Qu}PvZ>i@Y2Iv5>kUE`WzgsiwZPqDYObuQY_5Evs^O*T`WG6_)y*wU zrp+oSB;BYQ8>_bGzF4ML?5)l#ev#&E*`C=@(?*8)MKs4qH+giUrW;i?-DvdZ4 z)|Zx0br_gxHa9nzLiPV0XbQJJCWSR zZ5L#0E1wWsdPKfH(ovqef2zlb=n1c=T6uP-iX*a%H(lf!TAxvO3td%2!vRO19JIJUw-$u`7XTF zC1&cvt3m0+C{r_->G=p17R9Ld@>VeOY7pY|WjXyNPJf+q(b3JZA2+eu^IEOp-6`;JTa2-11!VJb;{0I?}KdRYhwqZh817+ z&i9qZdbJ~pwY~XcZLYVSA1M}lEtBULdebL?8t5&J@q*l9@0q^R=n$1M(l+Il(7e5q zH+J@N$AMZu+}7iLZ49W0VRN^4>j+T!AGJ|$Z>4z_S{2jWJbOdX8&7u!fB%u^?Wwas z6^!5bvKMx8-7s-%n?s_&L$QAxI5^5*nXozK0ABR;s7 z(x!v}4?{|hsDnr4-*2iMq67S<3&gv9?OMsu;6PW#g!KK3Otb0sk@sVti%CC&ZFy$+ z45ih{ z>?wNIG@DtWf40I{yl1;-PxYyXUi9xW*+myK37!gKv3nymd$tWJC}~i_paxXmt*?ItN?3mmIFTw>*PG z-e)cZZl64Q>xb$}=78f6P4K)XJdeWF6c7x$))*RH!N=*ix=A`QBM3)zN;B%yE>r5udgBij-K<;}55PvD9v14s^l541-l$HzYG(I) zebq74ZU0EVq;~AX3+WyFv7aZs;x58|J%m>~RTtBIQ^fk=IRn|j^SxOuNuXjWa9#(( zysYbu0_CU_DuWC{J@Qpk*{hGqS1+tLp@sdW9 zks}^8=EvbtF7~J4`JM=!E$~M<%Vko{_L*h6hgje+pqdprkp>2)Tn9g*td)h?geo8rVZv)Khavv;xWkool8Y#1}yZT=5l5`%Mwy#{R>=!)~W=%!t6hFYzUy!XA`LLvM+|DD{i~TcHQ0Sv5BEfFZ zO2miV)NaCvQN)ltnv{qSQV3n$9}Z7PVcj_`60Sy8SuXw2em>d9tDaP(a4Bc{V(;tP z^aV(RG(;kN%@=)cCgq{DG*3bJnY_nMX(3a3UQ3AWld2TIdj|5Oc!UfL+K10Au-B;= zxG5Hj>8_^X2BbVVP0V7(^ffZ+N2cpd~d($EJbR}vsE&>4*tqf!RTnqX^w$uRwiaPZsB85lJhh@)|Myy;SmT zJ&=-{Ffv+XcT*gAD;_!iaVQ^-eAmbvolwmwYoP(kdXecqY$$s4VfR0x(jYlwGPFmU z2=otQ7P|MdtH(B-)=Zb|1dp<;aNJMv{d`LdN%##M@IbSntaF5Pq%Y#W&&nFkS|(Mg zqJwi)(nFS5k|53&R~Hd>iqfVXFX`0wA9hjN;majm=|jG`vd-c0sxA%j!DnTi>1)hp0*60`767+ug4lG8TAb40%(~ zvW8A^8eDt0K}U~O3X=4iNxE#j!8v3*vypttek_?KEwuX=upkhBOvFG8!( z9a!KF#N|$3tgNYPXl!nM*=l!m^+14xl89c0o2yuvlOwnA6O*14tbOWfp(uGB4q-g> sCxFn3*z8b7MMrQpo4Mt)PyH$s&*@t}P3`*azVd$7kG-Ji*dKTK7v71uJOBUy literal 0 HcmV?d00001 diff --git a/tests/test_xiphcomment.cpp b/tests/test_xiphcomment.cpp index d2a28a1f..64ae0b21 100644 --- a/tests/test_xiphcomment.cpp +++ b/tests/test_xiphcomment.cpp @@ -47,6 +47,7 @@ class TestXiphComment : public CppUnit::TestFixture CPPUNIT_TEST(testClearComment); CPPUNIT_TEST(testRemoveFields); CPPUNIT_TEST(testPicture); + CPPUNIT_TEST(testLowercaseFields); CPPUNIT_TEST_SUITE_END(); public: @@ -192,6 +193,23 @@ public: } } + void testLowercaseFields() + { + const ScopedFileCopy copy("lowercase-fields", ".ogg"); + { + Vorbis::File f(copy.fileName().c_str()); + List lst = f.tag()->pictureList(); + CPPUNIT_ASSERT_EQUAL(String("TEST TITLE"), f.tag()->title()); + CPPUNIT_ASSERT_EQUAL(String("TEST ARTIST"), f.tag()->artist()); + CPPUNIT_ASSERT_EQUAL((unsigned int)1, lst.size()); + f.save(); + } + { + Vorbis::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.find("METADATA_BLOCK_PICTURE") > 0); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestXiphComment); -- 2.40.0