From ab3526af2e89e3204fdd1092ae764a87c1c59b4d Mon Sep 17 00:00:00 2001
From: Ryan Francesconi <2917795+ryanfrancesconi@users.noreply.github.com>
Date: Mon, 26 Oct 2026 20:59:21 -0700
Subject: [PATCH] Matroska: Render element sizes at a size length limit with a
 valid length (#1469)

EBML::minSize() chose the size length for an element's data size with
<= at each limit, so a size of exactly 2^(7n) - 1 (127, 16383, 2097151,
...) was rendered in n bytes with every value bit set. That is the
reserved "unknown size" value (RFC 8794 section 6.2), so a reader no
longer knows where the element ends. A tag value of exactly 127 bytes,
for example, saves without error but reads back as an empty string.

minSize() also returned 0 for any size from 0x7FFFFFFFF (32 GiB) up, so
renderVINT() wrote a size of zero bytes. Sizes up to 2^56 - 2 now use
size lengths 6 to 8; larger sizes still return 0, as no size length can
hold them.

testElementSizeLength checks minSize() on either side of the limit of
each size length from 1 to 8 bytes. testSaveElementSizeAtLengthLimit
saves titles of 126 to 128 and 16382 to 16384 bytes to no-tags.mka and
reads them back.
---
 taglib/matroska/ebml/ebmlutils.h | 19 +++++++++++----
 tests/test_matroska.cpp          | 42 ++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/taglib/matroska/ebml/ebmlutils.h b/taglib/matroska/ebml/ebmlutils.h
index abbec1a5..61839b86 100644
--- a/taglib/matroska/ebml/ebmlutils.h
+++ b/taglib/matroska/ebml/ebmlutils.h
@@ -59,18 +59,27 @@ namespace TagLib {
 
     unsigned long long randomUID();
 
+    // A size with all value bits set is reserved for "unknown size" (see
+    // https://datatracker.ietf.org/doc/rfc8794/ section 6.2), so a size length
+    // of n bytes can hold sizes up to 2^(7n) - 2.
     constexpr int minSize(uint64_t data)
     {
-      if(data <= 0x7Fu)
+      if(data < 0x7Fu)
         return 1;
-      if(data <= 0x3FFFu)
+      if(data < 0x3FFFu)
         return 2;
-      if(data <= 0x1FFFFFu)
+      if(data < 0x1FFFFFu)
         return 3;
-      if(data <= 0xFFFFFFFu)
+      if(data < 0xFFFFFFFu)
         return 4;
-      if(data <= 0x7FFFFFFFFu)
+      if(data < 0x7FFFFFFFFu)
         return 5;
+      if(data < 0x3FFFFFFFFFFu)
+        return 6;
+      if(data < 0x1FFFFFFFFFFFFu)
+        return 7;
+      if(data < 0xFFFFFFFFFFFFFFu)
+        return 8;
       return 0;
     }
 
diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp
index bf0bc875..783bd6b1 100644
--- a/tests/test_matroska.cpp
+++ b/tests/test_matroska.cpp
@@ -134,6 +134,7 @@
 #include "matroskachapteredition.h"
 #include "matroskachapters.h"
 #include "matroskasimpletag.h"
+#include "ebml/ebmlutils.h"
 #include "plainfile.h"
 #include "tfilestream.h"
 #include <cppunit/extensions/HelperMacros.h>
@@ -186,6 +187,8 @@ class TestMatroska : public CppUnit::TestFixture
   CPPUNIT_TEST(testAttachedFileDataReadOnDemand);
   CPPUNIT_TEST(testSaveUnrequestedAttachedFileData);
   CPPUNIT_TEST(testSegmentTitleWithoutAudioProperties);
+  CPPUNIT_TEST(testElementSizeLength);
+  CPPUNIT_TEST(testSaveElementSizeAtLengthLimit);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -1927,6 +1930,45 @@ public:
     CPPUNIT_ASSERT_EQUAL(String(""), noTitle.tag()->title());
   }
 
+  void testElementSizeLength()
+  {
+    for(int length = 1; length <= 8; ++length) {
+      // 2^(7n) - 1 is the "unknown size" value of a size length of n bytes.
+      const uint64_t unknownSize = (1ULL << (7 * length)) - 1;
+      CPPUNIT_ASSERT_EQUAL(length, EBML::minSize(unknownSize - 1));
+      if(length < 8) {
+        CPPUNIT_ASSERT_EQUAL(length + 1, EBML::minSize(unknownSize));
+      }
+    }
+    CPPUNIT_ASSERT_EQUAL(1, EBML::minSize(0));
+    CPPUNIT_ASSERT_EQUAL(0, EBML::minSize(0xFFFFFFFFFFFFFFULL));
+    CPPUNIT_ASSERT_EQUAL(0, EBML::minSize(0xFFFFFFFFFFFFFFFFULL));
+  }
+
+  void testSaveElementSizeAtLengthLimit()
+  {
+    // A title of 127 or 16383 bytes gives its string element a size equal to
+    // the "unknown size" value of a size length of 1 or 2 bytes.
+    for(const unsigned int length : {126U, 127U, 128U, 16382U, 16383U, 16384U}) {
+      ScopedFileCopy copy("no-tags", ".mka");
+      string newname = copy.fileName();
+      const String title(string(length, 'a'));
+      {
+        Matroska::File f(newname.c_str());
+        CPPUNIT_ASSERT(f.isValid());
+        f.tag(true)->setTitle(title);
+        CPPUNIT_ASSERT(f.save());
+      }
+      {
+        Matroska::File f(newname.c_str(), true, AudioProperties::Accurate);
+        CPPUNIT_ASSERT(f.isValid());
+        CPPUNIT_ASSERT(f.tag(false));
+        CPPUNIT_ASSERT_EQUAL(title, f.tag()->title());
+        CPPUNIT_ASSERT_EQUAL(444, f.audioProperties()->lengthInMilliseconds());
+      }
+    }
+  }
+
   void testUnknownSizeSegment()
   {
     ScopedFileCopy copy("no-tags", ".mka");