You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tika.apache.org by ta...@apache.org on 2026/07/10 14:17:46 UTC

(tika) branch TIKA-4784-id3-utf16-bom created (now b969f2af37)

This is an automated email from the ASF dual-hosted git repository.

tballison pushed a change to branch TIKA-4784-id3-utf16-bom
in repository https://gitbox.apache.org/repos/asf/tika.git


      at b969f2af37 TIKA-4784: fix unicode in id3

This branch includes the following new commits:

     new b969f2af37 TIKA-4784: fix unicode in id3

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(tika) 01/01: TIKA-4784: fix unicode in id3

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tballison pushed a commit to branch TIKA-4784-id3-utf16-bom
in repository https://gitbox.apache.org/repos/asf/tika.git

commit b969f2af37060bf7eaf5ffad2aa3b3386044bdad
Author: tallison <ta...@apache.org>
AuthorDate: Fri Jul 10 10:17:31 2026 -0400

    TIKA-4784: fix unicode in id3
---
 .../org/apache/tika/parser/mp3/ID3v22Handler.java  |  12 +-
 .../org/apache/tika/parser/mp3/ID3v23Handler.java  |  12 +-
 .../org/apache/tika/parser/mp3/ID3v24Handler.java  |  12 +-
 .../org/apache/tika/parser/mp3/ID3v2Frame.java     |  77 ++++++--
 .../org/apache/tika/parser/mp3/ID3v2FrameTest.java | 193 +++++++++++++++++++++
 .../org/apache/tika/parser/mp3/Mp3ParserTest.java  |  63 +++++++
 6 files changed, 356 insertions(+), 13 deletions(-)

diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v22Handler.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v22Handler.java
index 5f0102cd03..eed4d31079 100644
--- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v22Handler.java
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v22Handler.java
@@ -72,7 +72,7 @@ public class ID3v22Handler implements ID3Tags {
                     copyright = getTagString(tag.data, 0, tag.data.length);
                     break;
                 case "COM":
-                    comments.add(getComment(tag.data, 0, tag.data.length));
+                    addComment(getComment(tag.data, 0, tag.data.length));
                     break;
                 case "TRK":
                     trackNumber = getTagString(tag.data, 0, tag.data.length);
@@ -113,6 +113,16 @@ public class ID3v22Handler implements ID3Tags {
         return ID3v2Frame.getComment(data, offset, length);
     }
 
+    /**
+     * Malformed comment frames decode to null and are skipped rather than
+     * being added to the list, where they would trip up the consumers.
+     */
+    private void addComment(ID3Comment comment) {
+        if (comment != null) {
+            comments.add(comment);
+        }
+    }
+
     public boolean getTagsPresent() {
         return true;
     }
diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v23Handler.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v23Handler.java
index 9d10a5ac29..959b6519a6 100644
--- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v23Handler.java
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v23Handler.java
@@ -73,7 +73,7 @@ public class ID3v23Handler implements ID3Tags {
                     copyright = getTagString(tag.data, 0, tag.data.length);
                     break;
                 case "COMM":
-                    comments.add(getComment(tag.data, 0, tag.data.length));
+                    addComment(getComment(tag.data, 0, tag.data.length));
                     break;
                 case "TRCK":
                     trackNumber = getTagString(tag.data, 0, tag.data.length);
@@ -99,6 +99,16 @@ public class ID3v23Handler implements ID3Tags {
         return ID3v2Frame.getComment(data, offset, length);
     }
 
+    /**
+     * Malformed comment frames decode to null and are skipped rather than
+     * being added to the list, where they would trip up the consumers.
+     */
+    private void addComment(ID3Comment comment) {
+        if (comment != null) {
+            comments.add(comment);
+        }
+    }
+
     public boolean getTagsPresent() {
         return true;
     }
diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v24Handler.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v24Handler.java
index f9d7b106a8..5b2de1d052 100644
--- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v24Handler.java
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v24Handler.java
@@ -79,7 +79,7 @@ public class ID3v24Handler implements ID3Tags {
                     copyright = getTagString(tag.data, 0, tag.data.length);
                     break;
                 case "COMM":
-                    comments.add(getComment(tag.data, 0, tag.data.length));
+                    addComment(getComment(tag.data, 0, tag.data.length));
                     break;
                 case "TRCK":
                     trackNumber = getTagString(tag.data, 0, tag.data.length);
@@ -105,6 +105,16 @@ public class ID3v24Handler implements ID3Tags {
         return ID3v2Frame.getComment(data, offset, length);
     }
 
+    /**
+     * Malformed comment frames decode to null and are skipped rather than
+     * being added to the list, where they would trip up the consumers.
+     */
+    private void addComment(ID3Comment comment) {
+        if (comment != null) {
+            comments.add(comment);
+        }
+    }
+
     public boolean getTagsPresent() {
         return true;
     }
diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v2Frame.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v2Frame.java
index 1dddd1410b..d1dee624ca 100644
--- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v2Frame.java
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp3/ID3v2Frame.java
@@ -230,25 +230,80 @@ public class ID3v2Frame implements MP3Frame {
         // (return empty string), because new String(..)
         // gives different results on different JVMs
         if (encoding.encoding.equals("UTF-16") && actualLength == 2 &&
-                ((data[offset] == (byte) 0xff && data[offset + 1] == (byte) 0xfe) ||
-                        (data[offset] == (byte) 0xfe && data[offset + 1] == (byte) 0xff))) {
+                hasBOM(data, offset, actualLength)) {
             return "";
         }
 
         try {
             // Build the base string
-            return new String(data, offset, actualLength, encoding.encoding);
+            return decodeText(data, offset, actualLength, encoding);
         } catch (UnsupportedEncodingException e) {
             throw new RuntimeException("Core encoding " + encoding.encoding + " is not available",
                     e);
         }
     }
 
+    /**
+     * Decodes text in the frame's declared encoding.
+     * <p>
+     * Encoding {@code $01} is defined as UTF-16 <em>with</em> a BOM, but some taggers omit it.
+     * Java's UTF-16 charset then falls back to big-endian, which silently turns little-endian
+     * text into CJK mojibake (the ASCII 'T' {@code 0x54 0x00} decodes to U+5400). When the BOM
+     * is missing, recover the byte order from the position of the NUL bytes instead, and keep
+     * the big-endian fallback when the bytes carry no signal either way.
+     */
+    private static String decodeText(byte[] data, int offset, int length, TextEncoding encoding)
+            throws UnsupportedEncodingException {
+        String charset = encoding.encoding;
+        if ("UTF-16".equals(charset) && !hasBOM(data, offset, length)) {
+            charset = guessUTF16ByteOrder(data, offset, length);
+        }
+        return new String(data, offset, length, charset);
+    }
+
+    /**
+     * Does the text at the given offset start with a UTF-16 byte order mark?
+     */
+    private static boolean hasBOM(byte[] data, int offset, int length) {
+        if (length < 2) {
+            return false;
+        }
+        int first = data[offset] & 0xff;
+        int second = data[offset + 1] & 0xff;
+        return (first == 0xff && second == 0xfe) || (first == 0xfe && second == 0xff);
+    }
+
+    /**
+     * Guesses the byte order of BOM-less UTF-16 text by counting which half of each code
+     * unit is NUL. Characters below U+0100 - which dominate the tags this has to rescue -
+     * encode as {@code 0x00 lo} big-endian and {@code lo 0x00} little-endian. Text with no
+     * NUL bytes at all (eg CJK) gives no signal, so it stays on the big-endian default.
+     */
+    private static String guessUTF16ByteOrder(byte[] data, int offset, int length) {
+        int bigEndian = 0;
+        int littleEndian = 0;
+        for (int i = 0; i + 1 < length; i += 2) {
+            if (data[offset + i] == 0) {
+                bigEndian++;
+            }
+            if (data[offset + i + 1] == 0) {
+                littleEndian++;
+            }
+        }
+        return littleEndian > bigEndian ? "UTF-16LE" : "UTF-16BE";
+    }
+
     /**
      * Builds up the ID3 comment, by parsing and extracting
      * the comment string parts from the given data.
+     * Returns null if the frame is too short or malformed to hold a comment.
      */
     protected static ID3Comment getComment(byte[] data, int offset, int length) {
+        // A comment is at minimum an encoding flag and a 3 byte language
+        if (length < 4) {
+            return null;
+        }
+
         // Comments must have an encoding
         int encodingFlag = data[offset];
         if (encodingFlag >= 0 && encodingFlag < encodings.length) {
@@ -264,6 +319,7 @@ public class ID3v2Frame implements MP3Frame {
         String lang = getString(data, offset + 1, 3);
 
         // After that we have [Desc]\0(\0)[Text]
+        int end = offset + length;
         int descStart = offset + 4;
         int textStart = -1;
         String description = null;
@@ -271,29 +327,30 @@ public class ID3v2Frame implements MP3Frame {
 
         // Find where the description ends
         try {
-            for (int i = descStart; i < offset + length; i++) {
-                if (encoding.doubleByte && data[i] == 0 && data[i + 1] == 0) {
+            for (int i = descStart; i < end; i++) {
+                // A double byte terminator needs two bytes to be present
+                if (encoding.doubleByte && i + 1 < end && data[i] == 0 && data[i + 1] == 0) {
                     // Handle LE vs BE on low byte text
-                    if (i + 2 < offset + length && data[i + 1] == 0 && data[i + 2] == 0) {
+                    if (i + 2 < end && data[i + 2] == 0) {
                         i++;
                     }
                     textStart = i + 2;
-                    description = new String(data, descStart, i - descStart, encoding.encoding);
+                    description = decodeText(data, descStart, i - descStart, encoding);
                     break;
                 }
                 if (!encoding.doubleByte && data[i] == 0) {
                     textStart = i + 1;
-                    description = new String(data, descStart, i - descStart, encoding.encoding);
+                    description = decodeText(data, descStart, i - descStart, encoding);
                     break;
                 }
             }
 
             // Did we find the end?
             if (textStart > -1) {
-                text = new String(data, textStart, offset + length - textStart, encoding.encoding);
+                text = decodeText(data, textStart, end - textStart, encoding);
             } else {
                 // Assume everything is the text
-                text = new String(data, descStart, offset + length - descStart, encoding.encoding);
+                text = decodeText(data, descStart, end - descStart, encoding);
             }
 
             // Return
diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/ID3v2FrameTest.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/ID3v2FrameTest.java
new file mode 100644
index 0000000000..6091d6031c
--- /dev/null
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/ID3v2FrameTest.java
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.mp3;
+
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
+import static java.nio.charset.StandardCharsets.UTF_16BE;
+import static java.nio.charset.StandardCharsets.UTF_16LE;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.Charset;
+
+import org.junit.jupiter.api.Test;
+
+import org.apache.tika.parser.mp3.ID3Tags.ID3Comment;
+
+/**
+ * Tests the shared ID3v2 text decoding used by every ID3v2.2/2.3/2.4 text frame.
+ */
+public class ID3v2FrameTest {
+
+    private static final byte ISO_8859_1_FLAG = 0;
+    private static final byte UTF_16_BOM_FLAG = 1;
+    private static final byte UTF_16BE_FLAG = 2;
+    private static final byte UTF_8_FLAG = 3;
+
+    private static final byte[] BOM_LE = {(byte) 0xff, (byte) 0xfe};
+    private static final byte[] BOM_BE = {(byte) 0xfe, (byte) 0xff};
+
+    /**
+     * Latin text, which encodes to a NUL byte per character in UTF-16 and so carries a
+     * byte-order signal, and CJK text, which does not.
+     */
+    private static final String LATIN = "Test Copyright";
+    private static final String CJK = "日本語";
+
+    private static byte[] frame(byte encodingFlag, byte[]... parts) {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        out.write(encodingFlag);
+        for (byte[] part : parts) {
+            out.write(part, 0, part.length);
+        }
+        return out.toByteArray();
+    }
+
+    private static byte[] bytes(String text, Charset charset) {
+        return text.getBytes(charset);
+    }
+
+    private static String tagString(byte[] data) {
+        return ID3v2Frame.getTagString(data, 0, data.length);
+    }
+
+    @Test
+    public void testSingleByteEncodings() {
+        assertEquals(LATIN, tagString(frame(ISO_8859_1_FLAG, bytes(LATIN, ISO_8859_1))));
+        assertEquals(CJK, tagString(frame(UTF_8_FLAG, bytes(CJK, UTF_8))));
+    }
+
+    @Test
+    public void testUTF16WithBOM() {
+        assertEquals(LATIN, tagString(frame(UTF_16_BOM_FLAG, BOM_LE, bytes(LATIN, UTF_16LE))));
+        assertEquals(LATIN, tagString(frame(UTF_16_BOM_FLAG, BOM_BE, bytes(LATIN, UTF_16BE))));
+        assertEquals(CJK, tagString(frame(UTF_16_BOM_FLAG, BOM_LE, bytes(CJK, UTF_16LE))));
+        assertEquals(CJK, tagString(frame(UTF_16_BOM_FLAG, BOM_BE, bytes(CJK, UTF_16BE))));
+    }
+
+    @Test
+    public void testUTF16BEWithoutBOMFlag() {
+        assertEquals(LATIN, tagString(frame(UTF_16BE_FLAG, bytes(LATIN, UTF_16BE))));
+        assertEquals(CJK, tagString(frame(UTF_16BE_FLAG, bytes(CJK, UTF_16BE))));
+    }
+
+    /**
+     * Encoding $01 promises a BOM. Taggers that omit it used to be decoded as big-endian,
+     * turning little-endian text into CJK mojibake ('T' 0x54 0x00 becomes U+5400).
+     */
+    @Test
+    public void testUTF16WithoutBOMRecoversByteOrder() {
+        assertEquals(LATIN, tagString(frame(UTF_16_BOM_FLAG, bytes(LATIN, UTF_16LE))));
+        assertEquals(LATIN, tagString(frame(UTF_16_BOM_FLAG, bytes(LATIN, UTF_16BE))));
+    }
+
+    /**
+     * A BOM-less frame whose characters are all above U+00FF has no NUL bytes and so gives
+     * no byte-order signal at all. Nothing can rescue that, so the big-endian default of
+     * Java's UTF-16 charset must be preserved rather than guessed away.
+     */
+    @Test
+    public void testUTF16WithoutBOMKeepsBigEndianDefaultWhenNoSignal() {
+        assertEquals(CJK, tagString(frame(UTF_16_BOM_FLAG, bytes(CJK, UTF_16BE))));
+    }
+
+    @Test
+    public void testNullTerminationIsTrimmed() {
+        byte[] doubleNul = {0, 0};
+        byte[] singleNul = {0};
+        assertEquals(LATIN, tagString(frame(UTF_16_BOM_FLAG, bytes(LATIN, UTF_16LE), doubleNul)));
+        assertEquals(LATIN,
+                tagString(frame(UTF_16_BOM_FLAG, BOM_LE, bytes(LATIN, UTF_16LE), doubleNul)));
+        assertEquals(LATIN, tagString(frame(ISO_8859_1_FLAG, bytes(LATIN, ISO_8859_1), singleNul)));
+    }
+
+    /**
+     * TIKA-1024: a frame holding nothing but a BOM decodes to the empty string.
+     */
+    @Test
+    public void testNakedBOM() {
+        assertEquals("", tagString(frame(UTF_16_BOM_FLAG, BOM_LE)));
+        assertEquals("", tagString(frame(UTF_16_BOM_FLAG, BOM_BE)));
+    }
+
+    /**
+     * COMM frames decode a description and a text part separately, so both have to recover
+     * the byte order independently.
+     */
+    @Test
+    public void testCommentWithoutBOMRecoversByteOrder() throws IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        out.write(UTF_16_BOM_FLAG);
+        out.write(bytes("eng", ISO_8859_1));
+        out.write(bytes("Desc", UTF_16LE));
+        out.write(new byte[]{0, 0});
+        out.write(bytes(LATIN, UTF_16LE));
+        byte[] data = out.toByteArray();
+
+        ID3Comment comment = ID3v2Frame.getComment(data, 0, data.length);
+        assertEquals("eng", comment.getLanguage());
+        assertEquals("Desc", comment.getDescription());
+        assertEquals(LATIN, comment.getText());
+    }
+
+    @Test
+    public void testCommentWithBOM() throws IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        out.write(UTF_16_BOM_FLAG);
+        out.write(bytes("eng", ISO_8859_1));
+        out.write(BOM_LE);
+        out.write(bytes("Desc", UTF_16LE));
+        out.write(new byte[]{0, 0});
+        out.write(BOM_LE);
+        out.write(bytes(LATIN, UTF_16LE));
+        byte[] data = out.toByteArray();
+
+        ID3Comment comment = ID3v2Frame.getComment(data, 0, data.length);
+        assertEquals("eng", comment.getLanguage());
+        assertEquals("Desc", comment.getDescription());
+        assertEquals(LATIN, comment.getText());
+    }
+
+    /**
+     * A comment frame too short to hold an encoding flag and a 3 byte language, or one
+     * carrying an unknown encoding flag, decodes to null rather than reading off the end
+     * of the frame.
+     */
+    @Test
+    public void testMalformedCommentsReturnNull() {
+        assertNull(ID3v2Frame.getComment(new byte[0], 0, 0));
+        assertNull(ID3v2Frame.getComment(new byte[]{UTF_16_BOM_FLAG}, 0, 1));
+        assertNull(ID3v2Frame.getComment(new byte[]{ISO_8859_1_FLAG, 'e', 'n'}, 0, 3));
+        // 0x05 is not a defined ID3v2 text encoding
+        byte[] badFlag = {5, 'e', 'n', 'g', 'D', 'e', 's', 'c', 0, 'T'};
+        assertNull(ID3v2Frame.getComment(badFlag, 0, badFlag.length));
+    }
+
+    /**
+     * A double byte comment whose frame ends on a lone NUL has no room for a terminator,
+     * and must not read past the end of the frame looking for one.
+     */
+    @Test
+    public void testCommentTruncatedOnTerminatorDoesNotOverrun() {
+        byte[] data = {UTF_16_BOM_FLAG, 'e', 'n', 'g', 0};
+        ID3Comment comment = ID3v2Frame.getComment(data, 0, data.length);
+        assertEquals("eng", comment.getLanguage());
+    }
+}
diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/Mp3ParserTest.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/Mp3ParserTest.java
index 935b4be893..45af3e7679 100644
--- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/Mp3ParserTest.java
+++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp3/Mp3ParserTest.java
@@ -19,6 +19,9 @@ package org.apache.tika.parser.mp3;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+
 import org.junit.jupiter.api.Test;
 
 import org.apache.tika.TikaTest;
@@ -26,6 +29,8 @@ import org.apache.tika.io.TikaInputStream;
 import org.apache.tika.metadata.Metadata;
 import org.apache.tika.metadata.TikaCoreProperties;
 import org.apache.tika.metadata.XMPDM;
+import org.apache.tika.parser.ParseContext;
+import org.apache.tika.sax.BodyContentHandler;
 
 /**
  * Test case for parsing mp3 files.
@@ -362,4 +367,62 @@ public class Mp3ParserTest extends TikaTest {
         assertEquals("audio/mpeg", metadata.get(Metadata.CONTENT_TYPE));
         assertEquals("", metadata.get(XMPDM.GENRE));
     }
+
+    /**
+     * A malformed COMM frame must not take the whole parse down with it. Each of these
+     * frame bodies used to abort the parse of otherwise readable audio, with an
+     * ArrayIndexOutOfBounds, a StringIndexOutOfBounds or a NullPointerException.
+     */
+    @Test
+    public void testMalformedCommentFrameIsSkipped() throws Exception {
+        byte[][] bodies = {
+                new byte[0],                                  // empty body
+                new byte[]{1},                                // no language
+                new byte[]{0, 'e', 'n'},                      // truncated language
+                // 0x05 is not a defined ID3v2 text encoding
+                new byte[]{5, 'e', 'n', 'g', 'D', 'e', 's', 'c', 0, 'T'},
+                // double byte encoding, but the frame ends before the two byte terminator
+                new byte[]{1, 'e', 'n', 'g', 0}
+        };
+        for (byte[] body : bodies) {
+            String name = "COMM body of length " + body.length;
+            Metadata metadata = new Metadata();
+            try (TikaInputStream tis = TikaInputStream.get(mp3WithFrame("COMM", body))) {
+                new Mp3Parser().parse(tis, new BodyContentHandler(-1), metadata, new ParseContext());
+            }
+            assertEquals("audio/mpeg", metadata.get(Metadata.CONTENT_TYPE), name);
+            // the audio behind the broken tag is still read
+            assertEquals("44100", metadata.get(XMPDM.AUDIO_SAMPLE_RATE), name);
+        }
+    }
+
+    /**
+     * Wraps the audio of testMP3noid3.mp3 in an ID3v2.3 tag holding the single given frame.
+     */
+    private byte[] mp3WithFrame(String frameId, byte[] body) throws Exception {
+        byte[] audio;
+        try (TikaInputStream tis = getResourceAsStream("/test-documents/testMP3noid3.mp3")) {
+            audio = tis.readAllBytes();
+        }
+
+        ByteArrayOutputStream frame = new ByteArrayOutputStream();
+        frame.write(frameId.getBytes(StandardCharsets.US_ASCII));
+        // ID3v2.3 frame sizes are plain 32 bit big endian
+        frame.write(new byte[]{(byte) (body.length >>> 24), (byte) (body.length >>> 16),
+                (byte) (body.length >>> 8), (byte) body.length});
+        frame.write(new byte[]{0, 0});
+        frame.write(body);
+        byte[] frames = frame.toByteArray();
+
+        ByteArrayOutputStream mp3 = new ByteArrayOutputStream();
+        mp3.write("ID3".getBytes(StandardCharsets.US_ASCII));
+        mp3.write(new byte[]{3, 0, 0});
+        // the tag size is synchsafe: seven bits per byte
+        int size = frames.length;
+        mp3.write(new byte[]{(byte) ((size >>> 21) & 0x7f), (byte) ((size >>> 14) & 0x7f),
+                (byte) ((size >>> 7) & 0x7f), (byte) (size & 0x7f)});
+        mp3.write(frames);
+        mp3.write(audio);
+        return mp3.toByteArray();
+    }
 }