Fix parsing of regular 32-bit integers in SynchData::toUInt()

BUG:231075

git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@1115275 283d02a7-25f6-0310-bc7c-ecb5cbfe19da

diff --git a/NEWS b/NEWS
index 3b5e6dbe48bf4021ebde1a392638cd9e84879ba5..f629c77f06dff814a34b4bf94ea835828362e3fd 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,10 +1,12 @@
-TagLib 1.6.3 (Apr 13, 2010)
+TagLib 1.6.3 (Apr 15, 2010)
 ===========================
 
  * Fixed definitions of the TAGLIB_WITH_MP4 and TAGLIB_WITH_ASF macros.
  * Fixed upgrading of ID3v2.3 genre frame with ID3v1 code 0 (Blues).
  * New method `int String::toInt(bool *ok)` which can return whether the
    conversion to a number was successfull.
+ * Fixed parsing of incorrectly written lengths in ID3v2 (affects mainly
+   compressed frames). (BUG:231075)
 
 TagLib 1.6.2 (Apr 9, 2010)
 ==========================

diff --git a/taglib/mpeg/id3v2/id3v2synchdata.cpp b/taglib/mpeg/id3v2/id3v2synchdata.cpp
index 6241751e8665f3696922abf9ce5cfc98c6d49d3d..b150948f848545e520318d3fcb787e1b6e3aa745 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2synchdata.cpp
+++ b/taglib/mpeg/id3v2/id3v2synchdata.cpp
@@ -46,14 +46,10 @@ TagLib::uint SynchData::toUInt(const ByteVector &data)
   }
 
   if(notSynchSafe) {
-    /*
-     * Invalid data; assume this was created by some buggy software that just
-     * put normal integers here rather than syncsafe ones, and try it that
-     * way.
-     */
-    sum = 0;
-    for(int i = 0; i <= last; i++)
-      sum |= data[i] << ((last - i) * 8);
+    // Invalid data; assume this was created by some buggy software that just
+    // put normal integers here rather than syncsafe ones, and try it that
+    // way.
+    sum = (data.size() > 4) ? data.mid(0, 4).toUInt() : data.toUInt();
   }
 
   return sum;

diff --git a/tests/data/compressed_id3_frame.mp3 b/tests/data/compressed_id3_frame.mp3
new file mode 100644 (file)
index 0000000..60bc895
Binary files /dev/null and b/tests/data/compressed_id3_frame.mp3 differ

diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 860e911901f74f9be87b119701ae194fa37862d0..4276963d3567160e8002b635ac049868567c2a17 100644 (file)
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -61,6 +61,7 @@ class TestID3v2 : public CppUnit::TestFixture
   CPPUNIT_TEST(testUpdateGenre24);
   CPPUNIT_TEST(testUpdateDate22);
   // CPPUNIT_TEST(testUpdateFullDate22); TODO TYE+TDA should be upgraded to TDRC together
+  CPPUNIT_TEST(testCompressedFrameWithBrokenLength);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -455,6 +456,19 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("2010-04-03"), f.ID3v2Tag()->frameListMap()["TDRC"].front()->toString());
   }
 
+  void testCompressedFrameWithBrokenLength()
+  {
+    MPEG::File f("data/compressed_id3_frame.mp3", false);
+    CPPUNIT_ASSERT(f.ID3v2Tag()->frameListMap().contains("APIC"));
+    ID3v2::AttachedPictureFrame *frame =
+        static_cast<TagLib::ID3v2::AttachedPictureFrame*>(f.ID3v2Tag()->frameListMap()["APIC"].front());
+    CPPUNIT_ASSERT(frame);
+    CPPUNIT_ASSERT_EQUAL(String("image/bmp"), frame->mimeType());
+    CPPUNIT_ASSERT_EQUAL(ID3v2::AttachedPictureFrame::Other, frame->type());
+    CPPUNIT_ASSERT_EQUAL(String(""), frame->description());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(86414), frame->picture().size());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2);

diff --git a/tests/test_synchdata.cpp b/tests/test_synchdata.cpp
index 04ef5359dcb44f2c7607bcf5802287a35ee964c9..1113ee430817c72dc03a661d3ae2d359588cd468 100644 (file)
--- a/tests/test_synchdata.cpp
+++ b/tests/test_synchdata.cpp
@@ -34,6 +34,8 @@ class TestID3v2SynchData : public CppUnit::TestFixture
   CPPUNIT_TEST(test1);
   CPPUNIT_TEST(test2);
   CPPUNIT_TEST(test3);
+  CPPUNIT_TEST(testToUIntBroken);
+  CPPUNIT_TEST(testToUIntBrokenAndTooLarge);
   CPPUNIT_TEST(testDecode1);
   CPPUNIT_TEST(testDecode2);
   CPPUNIT_TEST_SUITE_END();
@@ -67,6 +69,23 @@ public:
     CPPUNIT_ASSERT_EQUAL(ID3v2::SynchData::fromUInt(129), v);
   }
 
+  void testToUIntBroken()
+  {
+    char data[] = { 0, 0, 0, -1 };
+    char data2[] = { 0, 0, -1, -1 };
+
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(255), ID3v2::SynchData::toUInt(ByteVector(data, 4)));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(65535), ID3v2::SynchData::toUInt(ByteVector(data2, 4)));
+  }
+
+  void testToUIntBrokenAndTooLarge()
+  {
+    char data[] = { 0, 0, 0, -1, 0 };
+    ByteVector v(data, 5);
+
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(255), ID3v2::SynchData::toUInt(v));
+  }
+
   void testDecode1()
   {
     ByteVector a("\xff\x00\x00", 3);