From 3d4428726ea90264d6814b4477c1d32e364a3dde Mon Sep 17 00:00:00 2001
From: Urs Fleisch <ufleisch@users.sourceforge.net>
Date: Tue, 14 May 2024 13:27:17 +0200
Subject: [PATCH] Fix parsing of ID3v2.2 frames (#1228)

---
 taglib/mpeg/id3v2/id3v2tag.cpp |  16 +++++++++--
 tests/data/itunes10.mp3        | Bin 0 -> 12312 bytes
 tests/test_mpeg.cpp            |  51 +++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/itunes10.mp3

diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
index 4f8390ee..f97be59e 100644
--- a/taglib/mpeg/id3v2/id3v2tag.cpp
+++ b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -843,8 +843,10 @@ void ID3v2::Tag::parse(const ByteVector &origData)
       break;
     }
 
-    Frame *frame = d->factory->createFrame(data.mid(frameDataPosition),
-                                           &d->header);
+    const ByteVector origData = data.mid(frameDataPosition);
+    const Header *tagHeader = &d->header;
+    unsigned int headerVersion = tagHeader->majorVersion();
+    Frame *frame = d->factory->createFrame(origData, tagHeader);
 
     if(!frame)
       return;
@@ -856,7 +858,15 @@ void ID3v2::Tag::parse(const ByteVector &origData)
       return;
     }
 
-    frameDataPosition += frame->size() + frame->headerSize();
+    if(frame->header()->version() == headerVersion) {
+      frameDataPosition += frame->size() + frame->headerSize();
+    } else {
+      // The frame was converted to another version, e.g. from 2.2 to 2.4.
+      // We must advance the frame data position according to the original
+      // frame, not the converted frame because its header size might differ.
+      Frame::Header origHeader(origData, headerVersion);
+      frameDataPosition += origHeader.frameSize() + origHeader.size();
+    }
     addFrame(frame);
   }
 
diff --git a/tests/data/itunes10.mp3 b/tests/data/itunes10.mp3
new file mode 100644
index 0000000000000000000000000000000000000000..1c944a646fb06ee22b343748b19d9d5e6497fa2c
GIT binary patch
literal 12312
zcmeI2c{G%5AIEQE45OGw)?_bPvds{ZbuhzNXAmY!3??SB^pGZ`LiVPT%96FQWF9Kn
zm&q259<<pjk%Xv6it^rimh=AgzUMio^PcmZ_x_!Gxvt;!yMNd3`rOwa-#;!K2E_@0
z=CF@Q)B=F50EHNFlpLn1VPlU1i1wNQ00&G$!zp3mU=Iw~2ABjLj-bl?=`*td06xGh
zm>LosMh*ptCU^kg`8@_862Z^;0L#$eh!DzAe}L#{4FE7eQ(aR7OlATA++bS^AiA1w
zy0tVkH36cj{dc!67-43&Sr3-%eLPCWh8*r41en=@vuy*&U@mY*D*0%57#IeAu?5fs
zobW^d;Qu~|j|!#ugaP(AGk_Nf*xOnHKoa=e$|u1GE_Evoi*W?s8k;v920mRlKPP|-
z9l&8shymR5uDEnboH(t{#qY`R{<}f%q`0KG;*Diw?{lb)X08fJ>V?g88cpi{G<vQ#
zW%{(2nDl!sR%}rGr{bpDV~-C#K4iTh-k4ZXws3dsK#tSngiN@S3Q9#@ZX$WL`4bJv
z7`u0Di|6Ztm`iQe?=}=eM)|+Z#!n=b=fx<em80fr2sv@%`N_^T+A5tsBY6^WB;SJR
zeM@&PA-XuP{Pu!@#M0qTF<WyN+2@y@t(02Gq$DI3^4I$Xbje*=0*4dh`Q7*T%h*sX
zQo3}Ev^h(}Uk*QPdAZOOlc&r}9nU=YtfAebIY}e>jM4hH{+n-Y?#By9BIZ^*`eTxu
z%e7tTSJgRwCWwg!w-3mPy=i0ryfjs_bA4`E=hk{{a?Y*fiS;kF2i-h!b|lqhXWWkX
zWySt^U>Y`uhm!WCAigT*DXaJsDPUU4aP`esA8P$ORvq$@#14%yS$-<F^{Q~tNl$O&
z@zZ^SUQc%p&B64W+#~NaY3}RFm5#F#&A!~I85@0Zz=g%R_}qNU3>%oQTniq$wjvNV
ze6Ffi=oHU3PQQd50k5Q%8<S7PB<5-mokp~OC2^NsJzH5h(psF?G~Pk{6ecTksq6hp
zzU;Y4<XI|)1sC@**?yLrV<gVKinB=;O|{0&8Jr#^;jt)APW3*;-UNCk|83GCOXj3A
zN%vN=G72$q@_f_6Ag;+(ZQ2yLV{_KzwB4QzhYM2eQz_kgM-dG7IKrK!b@Gja3Cgy|
zP$&+iyd+P-jL9$uY=8n+s2?&ZvSF%rB&#%b&5xeVi~Wplq}4=y6?S{UqPL*D%LA=r
z&PNMWu~kb^$c)0gs?i(E#NB{^$D2aT(8q|WTd-tCgq5&_uRh9Ya$#No*|mgM@RMg^
z^uoh^F*cDx?_?LNGiR^(X}%;$-7Ij|8P1WUWzzcyq>Y&b`td;$-p1HUQAC<R<16kF
z=MJwtu5eD(N~6z5EX}xNuu|wrEq_qZ$rOKeDcb`G0@7aZ+w=amig`gVJw*L<EBgF<
zxg0ThL=W5`)v@SY|8e2)my%7G1Q+Y2S)*^dca&@_V@v4K9BqsH`kB%~5$*{$cJ!qN
z-SglqcaB?p#@xmGq<5aLebhp5hgimK%J{Y6iz`S;iQdBCsW*X@rRbajh6T2Y_hHd5
zUO_eRQBCbP&6C>pFJ52|H>Vac4V^Y#&|AC(-(A!x9^fhp;<GH_;kg!iALr|6kyRJP
zWKS{bAK4_ZJ$3KRU3-*jC^g!0;@$x7LfR>oL#n8!y^n#Ee){2$HJS3g;igwl=FSLG
zV98d64mvhwt$Ubd9xAy>CsON>{<0$6XOs^W4mO<{a!+4x+0H>R%zjKOL{UujMK!fD
zdy87rtuQ*XaoSC2c_)7BY=;`Bgjd4LD#DDcW@W0yTJFA6bDI8RJ|dK!qK6Var9rtr
zoot`r_pUFsFhbNNdWUF;cel533?8PkP?KSuB0&PIZvZWoAXJ2}L3P5|<oY%@gn_NI
z6;jYSQ$lpuXvNsVRVi;sXcqQUV`t6njmRv9c345i1*I8mHnJfEFH}jk*jAQF)|_8B
z_$Y<n|HvytJDA54WpiVl?ttAskAJmz$XMuJ#-=uNN#9SfJdhTS7S)P8m!?+huAIbE
z571go&YDmYy!Fv4i8-ue=FN+}Rus-^GhF-Yo2BrmD_kv=<Ma>?`rz>sBU*KPtZo_e
z>Y~$yAD;0XWHzz1IiB)7QpO6gM{nv{`rdjq6TClbw^y>%r4HHklv7^VF^8|Pcqzv+
zJN&gz;T&yKl@mflWwkds?P&bbT4JHH<yC^Zo|YjI_h2_)D&|_S+3K#1OOCQ<_Fh`N
zh8FE1m2F$j)yh-wJ@Q@|6HMRchI1$+bZ)=0M|gBn7%lq6%G{ELV|oqDQWBK%ev60e
z*AP?&bgg$EQ6cGO?J0)C)5e)LTL&7B3|fUO9d<O#&X8jZ={92f+Q03LKl0e_-a}LW
z;?LbqSFxXV9T$Wp)>Ew%X<gmFdVTTUC-P=h(M6`GW;;h6e4_DAMW^Br&->eI{K%WE
z9zN6(Lt!_^XZ**@&fNGMHTLs>S%vmp?s;eAYY{FZ<^lYSLl?c-#^rv6(pBwu=N)rN
z=VLLsKR-ocXy)y9j3lM1s4J46%X(h#Cl7g5XqcDvl{oY>48{F%GWkW@a5uYlHYghC
z$PFWfN=mX=2k3=0Q4YVkPnT}z5KnTstTvf!cLjF=qs#9ta7MpR^<_Oi|3OE#MHp`O
zx`znsDQ_-Y+0tl=mL6^15G^ts&1-7tUQA!}k7zvM(DC491DRP~U+mOftr{+!)5Y!x
zgx{;L)=PwaI1#`5LL;7Gt4i!f^AXD~FU7d(8=O|KxDLm(NwN8rfa|+#ceH*T9EpkH
zF<O`MLGv*BPB~CGaTmPmK1->^gd&W-@b3LWhV9d$m{zN(2@!_TgJ+NNPNmp~%5!F!
zf85iOZq`kWoi0GcjJ1x<mAoaaR6grHuc%tg+bteaXF{f~i*3}ch*qf2MV?GZBl$32
zo=R7BRpD;YIMz+Mu@Lt$R^BjZP*hBs1Va-E1~C;rb5HD+Dub)-Ru0|0C?`{TSy@)H
zt8MtL_-ONoE$^tFZ6`X6*IZMM97@Yq6)7-$c=j11jb7j=H_q4)AsGyp$j7WuNsX-)
z7irat0~145$P6sLq$$-RJ~|i4&U5dgZ=JMSLsSrxn{FuAJ__R;p^D0@i{`#6P)Av*
zKf4+7adAvr@ODdlSYQ@&XzD4xpFm5W%l$~Y+8Qac$_Q9a8?!*9OO1AUeb94TK&Wc3
z(2Sk=Xx4kjU*~qj`x_5LW#vz_4Y(l!0|S?9gicPVANT?m!sT1&z|5}jEVe_x-TT0Y
zr=ekIP|$&C{d~ppGRLJrCX83&QgFzMTfp_1HrI>uos+#V=CKJBO`hIx#Nb{RJ%4ak
z2F_V`pn&~KRMNn@b%6c4&f|l_F<OMt-pJ>lHUN@V{)Ug;%%;=<aOSp{Tc%#89i2hx
z#kWOJ2eeK9cU2|0DV#R7pS`7tJ*cKY@rK%zUl?+jPbeiMoDzH#AQC`@g#ZY_q2V&N
z-c&N^05kFb?)WYXK@X_Lw*KMyLpBmX*(mgHL4RpSf*Ak^{66rPfZQwyGKc^~03rYp
zfCxYYAOa8phyX+YA^;J92tWiN0uTX+07L*H01<!)Km;HH5CMn)L;xZH5%@nOu(8JU
z{cp?09rX(aV+6n(JnYEh0>u7p+xWW%&ZUPpu`x%f>FI=Wur^EF`walltqQPK@DyX~
zW*s9nd)zh$Ap0i~_`c%=-U9#ytQrm;caQ-94)7`hyI>O?3jkby{&_V`rypTIINgsZ
f3Ci)mBe1bn?YoiwpLg7kt_Iq2Kbq)2D&M~VPm!H5

literal 0
HcmV?d00001

diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index 2ebfc407..d6aa3a40 100644
--- a/tests/test_mpeg.cpp
+++ b/tests/test_mpeg.cpp
@@ -71,6 +71,7 @@ class TestMPEG : public CppUnit::TestFixture
   CPPUNIT_TEST(testIgnoreGarbage);
   CPPUNIT_TEST(testExtendedHeader);
   CPPUNIT_TEST(testReadStyleFast);
+  CPPUNIT_TEST(testID3v22Properties);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -618,6 +619,56 @@ public:
     }
   }
 
+  void testID3v22Properties()
+  {
+    ScopedFileCopy copy("itunes10", ".mp3");
+
+    MPEG::File f(copy.fileName().c_str());
+    PropertyMap expectedProperties(SimplePropertyMap{
+      {"ALBUM", {"Album"}},
+      {"ALBUMARTIST", {"Album Artist"}},
+      {"ALBUMARTISTSORT", {"Sort Album Artist"}},
+      {"ALBUMSORT", {"Sort Album"}},
+      {"ARTIST", {"Artist"}},
+      {"ARTISTSORT", {"Sort Artist"}},
+      {"BPM", {"180"}},
+      {"COMMENT", {"Comments"}},
+      {"COMMENT:ITUNPGAP", {"1"}},
+      {"COMPILATION", {"1"}},
+      {"COMPOSER", {"Composer"}},
+      {"COMPOSERSORT", {"Sort Composer"}},
+      {"DATE", {"2011"}},
+      {"DISCNUMBER", {"1/2"}},
+      {"GENRE", {"Heavy Metal"}},
+      {"LYRICS", {"Lyrics"}},
+      {"SUBTITLE", {"Description"}},
+      {"TITLE", {"iTunes10MP3"}},
+      {"TITLESORT", {"Sort Name"}},
+      {"TRACKNUMBER", {"1/10"}},
+      {"WORK", {"Grouping"}}
+    });
+    expectedProperties.addUnsupportedData("APIC");
+    expectedProperties.addUnsupportedData("UNKNOWN/RVA");
+
+    PropertyMap properties = f.properties();
+    if (expectedProperties != properties) {
+      CPPUNIT_ASSERT_EQUAL(expectedProperties.toString(), properties.toString());
+    }
+    CPPUNIT_ASSERT(expectedProperties == properties);
+
+    const String PICTURE_KEY("PICTURE");
+    CPPUNIT_ASSERT_EQUAL(StringList(PICTURE_KEY), f.complexPropertyKeys());
+    auto pictures = f.complexProperties(PICTURE_KEY);
+    CPPUNIT_ASSERT_EQUAL(1U, pictures.size());
+    auto picture = pictures.front();
+    CPPUNIT_ASSERT_EQUAL(String("image/png"), picture.value("mimeType").toString());
+    CPPUNIT_ASSERT(picture.value("description").toString().isEmpty());
+    CPPUNIT_ASSERT_EQUAL(String("Other"), picture.value("pictureType").toString());
+    auto data = picture.value("data").toByteVector();
+    CPPUNIT_ASSERT(data.startsWith("\x89PNG\x0d\x0a\x1a\x0a"));
+    CPPUNIT_ASSERT_EQUAL(2315U, data.size());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);