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.
This commit is contained in:
Ryan Francesconi
2026-10-27 04:59:21 +01:00
committed by GitHub
parent 5fb109989d
commit ab3526af2e
2 changed files with 56 additions and 5 deletions
+42
View File
@@ -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");