id3: fix not taking frame names into acount for detecting broken iTun…

…es files under Python 3; add tests (Fixes issue #194)

the only real change here is the name decode.

mutagen/id3/__init__.py

@@ -291,47 +291,11 @@ def _load_header(self):
             else:
                 self.__extdata = b""
 
-    def __determine_bpi(self, data, frames, EMPTY=b"\x00" * 10):
+    def __determine_bpi(self, data, frames):
         if self.version < self._V24:
             return int
-        # have to special case whether to use bitpaddedints here
-        # spec says to use them, but iTunes has it wrong
-
-        # count number of tags found as BitPaddedInt and how far past
-        o = 0
-        asbpi = 0
-        while o < len(data) - 10:
-            part = data[o:o + 10]
-            if part == EMPTY:
-                bpioff = -((len(data) - o) % 10)
-                break
-            name, size, flags = unpack('>4sLH', part)
-            size = BitPaddedInt(size)
-            o += 10 + size
-            if name in frames:
-                asbpi += 1
-        else:
-            bpioff = o - len(data)
-
-        # count number of tags found as int and how far past
-        o = 0
-        asint = 0
-        while o < len(data) - 10:
-            part = data[o:o + 10]
-            if part == EMPTY:
-                intoff = -((len(data) - o) % 10)
-                break
-            name, size, flags = unpack('>4sLH', part)
-            o += 10 + size
-            if name in frames:
-                asint += 1
-        else:
-            intoff = o - len(data)
 
-        # if more tags as int, or equal and bpi is past and int is not
-        if asint > asbpi or (asint == asbpi and (bpioff >= 1 and intoff <= 1)):
-            return int
-        return BitPaddedInt
+        return _determine_bpi(data, frames)
 
     def __read_frames(self, data, frames):
         if self.version < self._V24 and self.f_unsynch:
@@ -783,6 +747,59 @@ def delete(filename, delete_v1=True, delete_v2=True):
 Open = ID3
 
 
+def _determine_bpi(data, frames, EMPTY=b"\x00" * 10):
+    """Takes id3v2.4 frame data and determines if ints or bitpaddedints
+    should be used for parsing. Needed because iTunes used to write
+    normal ints for frame sizes.
+    """
+
+    # count number of tags found as BitPaddedInt and how far past
+    o = 0
+    asbpi = 0
+    while o < len(data) - 10:
+        part = data[o:o + 10]
+        if part == EMPTY:
+            bpioff = -((len(data) - o) % 10)
+            break
+        name, size, flags = unpack('>4sLH', part)
+        size = BitPaddedInt(size)
+        o += 10 + size
+        if PY3:
+            try:
+                name = name.decode("ascii")
+            except UnicodeDecodeError:
+                continue
+        if name in frames:
+            asbpi += 1
+    else:
+        bpioff = o - len(data)
+
+    # count number of tags found as int and how far past
+    o = 0
+    asint = 0
+    while o < len(data) - 10:
+        part = data[o:o + 10]
+        if part == EMPTY:
+            intoff = -((len(data) - o) % 10)
+            break
+        name, size, flags = unpack('>4sLH', part)
+        o += 10 + size
+        if PY3:
+            try:
+                name = name.decode("ascii")
+            except UnicodeDecodeError:
+                continue
+        if name in frames:
+            asint += 1
+    else:
+        intoff = o - len(data)
+
+    # if more tags as int, or equal and bpi is past and int is not
+    if asint > asbpi or (asint == asbpi and (bpioff >= 1 and intoff <= 1)):
+        return int
+    return BitPaddedInt
+
+
 def _find_id3v1(fileobj):
     """Returns a tuple of (id3tag, offset_to_end) or (None, 0)
 

tests/test_id3.py

@@ -7,6 +7,7 @@
 from mutagen.apev2 import APEv2
 from mutagen.id3 import ID3, COMR, Frames, Frames_2_2, ID3Warning, \
     ID3JunkFrameError
+from mutagen.id3._util import BitPaddedInt
 from mutagen._compat import cBytesIO, PY2, iteritems, integer_types
 import warnings
 warnings.simplefilter('error', ID3Warning)
@@ -1749,6 +1750,34 @@ def test_main(self):
         self.assertEqual(id3.Encoding.UTF8, 3)
         self.assertEqual(id3.ID3v1SaveOptions.UPDATE, 1)
 
+    def test_determine_bpi(self):
+        determine_bpi = id3._determine_bpi
+        # default to BitPaddedInt
+        self.assertTrue(determine_bpi("", {}) is BitPaddedInt)
+
+        def get_frame_data(name, size, bpi=True):
+            data = name
+            if bpi:
+                data += BitPaddedInt.to_str(size)
+            else:
+                data += BitPaddedInt.to_str(size, bits=8)
+            data += b"\x00\x00" + b"\x01" * size
+            return data
+
+        data = get_frame_data(b"TPE2", 1000, True)
+        self.assertTrue(determine_bpi(data, Frames) is BitPaddedInt)
+        self.assertTrue(
+            determine_bpi(data + b"\x00" * 1000, Frames) is BitPaddedInt)
+
+        data = get_frame_data(b"TPE2", 1000, False)
+        self.assertTrue(determine_bpi(data, Frames) is int)
+        self.assertTrue(determine_bpi(data + b"\x00" * 1000, Frames) is int)
+
+        # in this case it helps that we know the frame name
+        d = get_frame_data(b"TPE2", 1000) + get_frame_data(b"TPE2", 10) + \
+                b"\x01" * 875
+        self.assertTrue(determine_bpi(d, Frames) is BitPaddedInt)
+
 
 add(TID3Misc)
 add(ID3V1_vs_APEv2)