From 0aee235e0aa2a9733ec22b77085961751bdc3b30 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 10 Apr 2017 09:41:54 -0700 Subject: [PATCH] Make ID3 parser more robust (again) I've also removed unnecessary "empty" cases, since to add them everywhere would bloat the code quite a lot. Note that new String(new byte[0], 0, 0, encoding) is valid and will produce and empty string. Issue: #2663 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=152697288 --- .../metadata/id3/Id3DecoderTest.java | 186 +++++++++++++++--- .../exoplayer2/metadata/id3/Id3Decoder.java | 61 +++--- 2 files changed, 202 insertions(+), 45 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/metadata/id3/Id3DecoderTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/metadata/id3/Id3DecoderTest.java index e271108ce4..6b39ed1645 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/metadata/id3/Id3DecoderTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/metadata/id3/Id3DecoderTest.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.metadata.id3; import android.test.MoreAsserts; import com.google.android.exoplayer2.metadata.Metadata; import com.google.android.exoplayer2.metadata.MetadataDecoderException; +import com.google.android.exoplayer2.util.Assertions; import junit.framework.TestCase; /** @@ -25,10 +26,13 @@ import junit.framework.TestCase; */ public final class Id3DecoderTest extends TestCase { + private static final byte[] TAG_HEADER = new byte[] {73, 68, 51, 4, 0, 0, 0, 0, 0, 0}; + private static final int FRAME_HEADER_LENGTH = 10; + private static final int ID3_TEXT_ENCODING_UTF_8 = 3; + public void testDecodeTxxxFrame() throws MetadataDecoderException { - byte[] rawId3 = new byte[] {73, 68, 51, 4, 0, 0, 0, 0, 0, 41, 84, 88, 88, 88, 0, 0, 0, 31, 0, 0, - 3, 0, 109, 100, 105, 97, 108, 111, 103, 95, 86, 73, 78, 68, 73, 67, 79, 49, 53, 50, 55, 54, - 54, 52, 95, 115, 116, 97, 114, 116, 0}; + byte[] rawId3 = buildSingleFrameTag("TXXX", new byte[] {3, 0, 109, 100, 105, 97, 108, 111, 103, + 95, 86, 73, 78, 68, 73, 67, 79, 49, 53, 50, 55, 54, 54, 52, 95, 115, 116, 97, 114, 116, 0}); Id3Decoder decoder = new Id3Decoder(); Metadata metadata = decoder.decode(rawId3, rawId3.length); assertEquals(1, metadata.length()); @@ -36,12 +40,118 @@ public final class Id3DecoderTest extends TestCase { assertEquals("TXXX", textInformationFrame.id); assertEquals("", textInformationFrame.description); assertEquals("mdialog_VINDICO1527664_start", textInformationFrame.value); + + // Test empty. + rawId3 = buildSingleFrameTag("TXXX", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(0, metadata.length()); + + // Test encoding byte only. + rawId3 = buildSingleFrameTag("TXXX", new byte[] {ID3_TEXT_ENCODING_UTF_8}); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + textInformationFrame = (TextInformationFrame) metadata.get(0); + assertEquals("TXXX", textInformationFrame.id); + assertEquals("", textInformationFrame.description); + assertEquals("", textInformationFrame.value); + } + + public void testDecodeTextInformationFrame() throws MetadataDecoderException { + byte[] rawId3 = buildSingleFrameTag("TIT2", new byte[] {3, 72, 101, 108, 108, 111, 32, 87, 111, + 114, 108, 100, 0}); + Id3Decoder decoder = new Id3Decoder(); + Metadata metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + TextInformationFrame textInformationFrame = (TextInformationFrame) metadata.get(0); + assertEquals("TIT2", textInformationFrame.id); + assertNull(textInformationFrame.description); + assertEquals("Hello World", textInformationFrame.value); + + // Test empty. + rawId3 = buildSingleFrameTag("TIT2", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(0, metadata.length()); + + // Test encoding byte only. + rawId3 = buildSingleFrameTag("TIT2", new byte[] {ID3_TEXT_ENCODING_UTF_8}); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + textInformationFrame = (TextInformationFrame) metadata.get(0); + assertEquals("TIT2", textInformationFrame.id); + assertEquals(null, textInformationFrame.description); + assertEquals("", textInformationFrame.value); + } + + public void testDecodeWxxxFrame() throws MetadataDecoderException { + byte[] rawId3 = buildSingleFrameTag("WXXX", new byte[] {ID3_TEXT_ENCODING_UTF_8, 116, 101, 115, + 116, 0, 104, 116, 116, 112, 115, 58, 47, 47, 116, 101, 115, 116, 46, 99, 111, 109, 47, 97, + 98, 99, 63, 100, 101, 102}); + Id3Decoder decoder = new Id3Decoder(); + Metadata metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + UrlLinkFrame urlLinkFrame = (UrlLinkFrame) metadata.get(0); + assertEquals("WXXX", urlLinkFrame.id); + assertEquals("test", urlLinkFrame.description); + assertEquals("https://test.com/abc?def", urlLinkFrame.url); + + // Test empty. + rawId3 = buildSingleFrameTag("WXXX", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(0, metadata.length()); + + // Test encoding byte only. + rawId3 = buildSingleFrameTag("WXXX", new byte[] {ID3_TEXT_ENCODING_UTF_8}); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + urlLinkFrame = (UrlLinkFrame) metadata.get(0); + assertEquals("WXXX", urlLinkFrame.id); + assertEquals("", urlLinkFrame.description); + assertEquals("", urlLinkFrame.url); + } + + public void testDecodeUrlLinkFrame() throws MetadataDecoderException { + byte[] rawId3 = buildSingleFrameTag("WCOM", new byte[] {104, 116, 116, 112, 115, 58, 47, 47, + 116, 101, 115, 116, 46, 99, 111, 109, 47, 97, 98, 99, 63, 100, 101, 102}); + Id3Decoder decoder = new Id3Decoder(); + Metadata metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + UrlLinkFrame urlLinkFrame = (UrlLinkFrame) metadata.get(0); + assertEquals("WCOM", urlLinkFrame.id); + assertEquals(null, urlLinkFrame.description); + assertEquals("https://test.com/abc?def", urlLinkFrame.url); + + // Test empty. + rawId3 = buildSingleFrameTag("WCOM", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + urlLinkFrame = (UrlLinkFrame) metadata.get(0); + assertEquals("WCOM", urlLinkFrame.id); + assertEquals(null, urlLinkFrame.description); + assertEquals("", urlLinkFrame.url); + } + + public void testDecodePrivFrame() throws MetadataDecoderException { + byte[] rawId3 = buildSingleFrameTag("PRIV", new byte[] {116, 101, 115, 116, 0, 1, 2, 3, 4}); + Id3Decoder decoder = new Id3Decoder(); + Metadata metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + PrivFrame privFrame = (PrivFrame) metadata.get(0); + assertEquals("test", privFrame.owner); + MoreAsserts.assertEquals(new byte[] {1, 2, 3, 4}, privFrame.privateData); + + // Test empty. + rawId3 = buildSingleFrameTag("PRIV", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + privFrame = (PrivFrame) metadata.get(0); + assertEquals("", privFrame.owner); + MoreAsserts.assertEquals(new byte[0], privFrame.privateData); } public void testDecodeApicFrame() throws MetadataDecoderException { - byte[] rawId3 = new byte[] {73, 68, 51, 4, 0, 0, 0, 0, 0, 45, 65, 80, 73, 67, 0, 0, 0, 35, 0, 0, - 3, 105, 109, 97, 103, 101, 47, 106, 112, 101, 103, 0, 16, 72, 101, 108, 108, 111, 32, 87, - 111, 114, 108, 100, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0}; + byte[] rawId3 = buildSingleFrameTag("APIC", new byte[] {3, 105, 109, 97, 103, 101, 47, 106, 112, + 101, 103, 0, 16, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 0, 1, 2, 3, 4, 5, 6, 7, + 8, 9, 0}); Id3Decoder decoder = new Id3Decoder(); Metadata metadata = decoder.decode(rawId3, rawId3.length); assertEquals(1, metadata.length()); @@ -53,27 +163,59 @@ public final class Id3DecoderTest extends TestCase { MoreAsserts.assertEquals(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 0}, apicFrame.pictureData); } - public void testDecodeTextInformationFrame() throws MetadataDecoderException { - byte[] rawId3 = new byte[] {73, 68, 51, 4, 0, 0, 0, 0, 0, 23, 84, 73, 84, 50, 0, 0, 0, 13, 0, 0, - 3, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 0}; + public void testDecodeCommentFrame() throws MetadataDecoderException { + byte[] rawId3 = buildSingleFrameTag("COMM", new byte[] {ID3_TEXT_ENCODING_UTF_8, 101, 110, 103, + 100, 101, 115, 99, 114, 105, 112, 116, 105, 111, 110, 0, 116, 101, 120, 116, 0}); Id3Decoder decoder = new Id3Decoder(); Metadata metadata = decoder.decode(rawId3, rawId3.length); assertEquals(1, metadata.length()); - TextInformationFrame textInformationFrame = (TextInformationFrame) metadata.get(0); - assertEquals("TIT2", textInformationFrame.id); - assertNull(textInformationFrame.description); - assertEquals("Hello World", textInformationFrame.value); + CommentFrame commentFrame = (CommentFrame) metadata.get(0); + assertEquals("eng", commentFrame.language); + assertEquals("description", commentFrame.description); + assertEquals("text", commentFrame.text); + + // Test empty. + rawId3 = buildSingleFrameTag("COMM", new byte[0]); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(0, metadata.length()); + + // Test language only. + rawId3 = buildSingleFrameTag("COMM", new byte[] {ID3_TEXT_ENCODING_UTF_8, 101, 110, 103}); + metadata = decoder.decode(rawId3, rawId3.length); + assertEquals(1, metadata.length()); + commentFrame = (CommentFrame) metadata.get(0); + assertEquals("eng", commentFrame.language); + assertEquals("", commentFrame.description); + assertEquals("", commentFrame.text); } - public void testDecodePrivFrame() throws MetadataDecoderException { - byte[] rawId3 = new byte[] {73, 68, 51, 4, 0, 0, 0, 0, 0, 19, 80, 82, 73, 86, 0, 0, 0, 9, 0, 0, - 116, 101, 115, 116, 0, 1, 2, 3, 4}; - Id3Decoder decoder = new Id3Decoder(); - Metadata metadata = decoder.decode(rawId3, rawId3.length); - assertEquals(1, metadata.length()); - PrivFrame privFrame = (PrivFrame) metadata.get(0); - assertEquals("test", privFrame.owner); - MoreAsserts.assertEquals(new byte[] {1, 2, 3, 4}, privFrame.privateData); + private static byte[] buildSingleFrameTag(String frameId, byte[] frameData) { + byte[] frameIdBytes = frameId.getBytes(); + Assertions.checkState(frameIdBytes.length == 4); + + byte[] tagData = new byte[TAG_HEADER.length + FRAME_HEADER_LENGTH + frameData.length]; + System.arraycopy(TAG_HEADER, 0, tagData, 0, TAG_HEADER.length); + // Fill in the size part of the tag header. + int offset = TAG_HEADER.length - 4; + int tagSize = frameData.length + FRAME_HEADER_LENGTH; + tagData[offset++] = (byte) ((tagSize >> 21) & 0x7F); + tagData[offset++] = (byte) ((tagSize >> 14) & 0x7F); + tagData[offset++] = (byte) ((tagSize >> 7) & 0x7F); + tagData[offset++] = (byte) (tagSize & 0x7F); + // Fill in the frame header. + tagData[offset++] = frameIdBytes[0]; + tagData[offset++] = frameIdBytes[1]; + tagData[offset++] = frameIdBytes[2]; + tagData[offset++] = frameIdBytes[3]; + tagData[offset++] = (byte) ((frameData.length >> 24) & 0xFF); + tagData[offset++] = (byte) ((frameData.length >> 16) & 0xFF); + tagData[offset++] = (byte) ((frameData.length >> 8) & 0xFF); + tagData[offset++] = (byte) (frameData.length & 0xFF); + offset += 2; // Frame flags set to 0 + // Fill in the frame data. + System.arraycopy(frameData, 0, tagData, offset, frameData.length); + + return tagData; } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java b/library/core/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java index 4faebd510a..df3353fb18 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java @@ -346,17 +346,13 @@ public final class Id3Decoder implements MetadataDecoder { && (majorVersion == 2 || frameId3 == 'X')) { frame = decodeTxxxFrame(id3Data, frameSize); } else if (frameId0 == 'T') { - String id = majorVersion == 2 - ? String.format(Locale.US, "%c%c%c", frameId0, frameId1, frameId2) - : String.format(Locale.US, "%c%c%c%c", frameId0, frameId1, frameId2, frameId3); + String id = getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3); frame = decodeTextInformationFrame(id3Data, frameSize, id); } else if (frameId0 == 'W' && frameId1 == 'X' && frameId2 == 'X' && (majorVersion == 2 || frameId3 == 'X')) { frame = decodeWxxxFrame(id3Data, frameSize); } else if (frameId0 == 'W') { - String id = majorVersion == 2 - ? String.format(Locale.US, "%c%c%c", frameId0, frameId1, frameId2) - : String.format(Locale.US, "%c%c%c%c", frameId0, frameId1, frameId2, frameId3); + String id = getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3); frame = decodeUrlLinkFrame(id3Data, frameSize, id); } else if (frameId0 == 'P' && frameId1 == 'R' && frameId2 == 'I' && frameId3 == 'V') { frame = decodePrivFrame(id3Data, frameSize); @@ -376,11 +372,14 @@ public final class Id3Decoder implements MetadataDecoder { frame = decodeChapterTOCFrame(id3Data, frameSize, majorVersion, unsignedIntFrameSizeHack, frameHeaderSize, framePredicate); } else { - String id = majorVersion == 2 - ? String.format(Locale.US, "%c%c%c", frameId0, frameId1, frameId2) - : String.format(Locale.US, "%c%c%c%c", frameId0, frameId1, frameId2, frameId3); + String id = getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3); frame = decodeBinaryFrame(id3Data, frameSize, id); } + if (frame == null) { + Log.w(TAG, "Failed to decode frame: id=" + + getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3) + ", frameSize=" + + frameSize); + } return frame; } catch (UnsupportedEncodingException e) { Log.w(TAG, "Unsupported character encoding"); @@ -392,6 +391,11 @@ public final class Id3Decoder implements MetadataDecoder { private static TextInformationFrame decodeTxxxFrame(ParsableByteArray id3Data, int frameSize) throws UnsupportedEncodingException { + if (frameSize < 1) { + // Frame is malformed. + return null; + } + int encoding = id3Data.readUnsignedByte(); String charset = getCharsetName(encoding); @@ -415,9 +419,9 @@ public final class Id3Decoder implements MetadataDecoder { private static TextInformationFrame decodeTextInformationFrame(ParsableByteArray id3Data, int frameSize, String id) throws UnsupportedEncodingException { - if (frameSize <= 1) { - // Frame is empty or contains only the text encoding byte. - return new TextInformationFrame(id, null, ""); + if (frameSize < 1) { + // Frame is malformed. + return null; } int encoding = id3Data.readUnsignedByte(); @@ -434,6 +438,11 @@ public final class Id3Decoder implements MetadataDecoder { private static UrlLinkFrame decodeWxxxFrame(ParsableByteArray id3Data, int frameSize) throws UnsupportedEncodingException { + if (frameSize < 1) { + // Frame is malformed. + return null; + } + int encoding = id3Data.readUnsignedByte(); String charset = getCharsetName(encoding); @@ -457,11 +466,6 @@ public final class Id3Decoder implements MetadataDecoder { private static UrlLinkFrame decodeUrlLinkFrame(ParsableByteArray id3Data, int frameSize, String id) throws UnsupportedEncodingException { - if (frameSize == 0) { - // Frame is empty. - return new UrlLinkFrame(id, null, ""); - } - byte[] data = new byte[frameSize]; id3Data.readBytes(data, 0, frameSize); @@ -473,19 +477,19 @@ public final class Id3Decoder implements MetadataDecoder { private static PrivFrame decodePrivFrame(ParsableByteArray id3Data, int frameSize) throws UnsupportedEncodingException { - if (frameSize == 0) { - // Frame is empty. - return new PrivFrame("", new byte[0]); - } - byte[] data = new byte[frameSize]; id3Data.readBytes(data, 0, frameSize); int ownerEndIndex = indexOfZeroByte(data, 0); String owner = new String(data, 0, ownerEndIndex, "ISO-8859-1"); + byte[] privateData; int privateDataStartIndex = ownerEndIndex + 1; - byte[] privateData = Arrays.copyOfRange(data, privateDataStartIndex, data.length); + if (privateDataStartIndex < data.length) { + privateData = Arrays.copyOfRange(data, privateDataStartIndex, data.length); + } else { + privateData = new byte[0]; + } return new PrivFrame(owner, privateData); } @@ -556,6 +560,11 @@ public final class Id3Decoder implements MetadataDecoder { private static CommentFrame decodeCommentFrame(ParsableByteArray id3Data, int frameSize) throws UnsupportedEncodingException { + if (frameSize < 4) { + // Frame is malformed. + return null; + } + int encoding = id3Data.readUnsignedByte(); String charset = getCharsetName(encoding); @@ -701,6 +710,12 @@ public final class Id3Decoder implements MetadataDecoder { } } + private static String getFrameId(int majorVersion, int frameId0, int frameId1, int frameId2, + int frameId3) { + return majorVersion == 2 ? String.format(Locale.US, "%c%c%c", frameId0, frameId1, frameId2) + : String.format(Locale.US, "%c%c%c%c", frameId0, frameId1, frameId2, frameId3); + } + private static int indexOfEos(byte[] data, int fromIndex, int encoding) { int terminationPos = indexOfZeroByte(data, fromIndex);