From c32f7c7f86ccecd588c705a5b73c80b9efd37c11 Mon Sep 17 00:00:00 2001 From: Ryan Francesconi Date: Sat, 16 May 2026 09:18:11 -0700 Subject: [PATCH] [FLAC] Track iXML/BEXT block presence with explicit flags hasiXMLData() / hasBEXTData() were implemented as !data.isEmpty() checks, which conflated in-memory payload with on-disk block presence. That caused two wrong answers: * setiXMLData("foo") on a file with no iXML block made hasiXMLData() return true immediately, before save(). * A FLAC file carrying an iXML APPLICATION block with empty payload round-tripped fine, but hasiXMLData() reported false. Switch to the same model RIFF::WAV::File already uses: explicit hasiXML / hasBEXT bool flags on FilePrivate, set during scan() when the APPLICATION block is recognised, updated during save() after the block is (re)written or omitted, and returned verbatim by the accessors. New regression test pins down the before/after-save and empty-block cases. Refs: https://github.com/taglib/taglib/issues/1362 --- taglib/flac/flacfile.cpp | 22 ++++++++++++++++++---- tests/test_flac.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index 017df40f..99acb62e 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -72,6 +72,8 @@ public: ByteVector xiphCommentData; String iXMLData; ByteVector bextData; + bool hasiXML { false }; + bool hasBEXT { false }; List blocks; offset_t flacStart { 0 }; @@ -279,6 +281,10 @@ bool FLAC::File::save() payload.append(ByteVector::fromUInt(xml.size(), false)); payload.append(xml); d->blocks.append(new UnknownMetadataBlock(MetadataBlock::Application, payload)); + d->hasiXML = true; + } + else { + d->hasiXML = false; } if(!d->bextData.isEmpty()) { ByteVector payload; @@ -287,6 +293,10 @@ bool FLAC::File::save() payload.append(ByteVector::fromUInt(d->bextData.size(), false)); payload.append(d->bextData); d->blocks.append(new UnknownMetadataBlock(MetadataBlock::Application, payload)); + d->hasBEXT = true; + } + else { + d->hasBEXT = false; } // Replace metadata blocks @@ -532,12 +542,12 @@ bool FLAC::File::hasID3v2Tag() const bool FLAC::File::hasiXMLData() const { - return !d->iXMLData.isEmpty(); + return d->hasiXML; } bool FLAC::File::hasBEXTData() const { - return !d->bextData.isEmpty(); + return d->hasBEXT; } //////////////////////////////////////////////////////////////////////////////// @@ -719,14 +729,18 @@ void FLAC::File::scan() } if(innerId == "iXML") { - if(d->iXMLData.isEmpty()) + if(!d->hasiXML) { + d->hasiXML = true; d->iXMLData = String(innerData, String::UTF8); + } else debug("FLAC::File::scan() -- multiple iXML blocks found, discarding"); } else if(innerId == "bext") { - if(d->bextData.isEmpty()) + if(!d->hasBEXT) { + d->hasBEXT = true; d->bextData = innerData; + } else debug("FLAC::File::scan() -- multiple BEXT blocks found, discarding"); } diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp index 3541b3f5..791fc439 100644 --- a/tests/test_flac.cpp +++ b/tests/test_flac.cpp @@ -74,6 +74,7 @@ class TestFLAC : public CppUnit::TestFixture CPPUNIT_TEST(testReadBEXTRiffWrapped); CPPUNIT_TEST(testWriteiXMLAndBEXT); CPPUNIT_TEST(testWriteEmptyClearsiXMLAndBEXT); + CPPUNIT_TEST(testHasiXMLAndBEXTReflectFileState); CPPUNIT_TEST(testRoundTripPreservesUnknownApplicationBlock); CPPUNIT_TEST_SUITE_END(); @@ -833,6 +834,45 @@ public: } } + void testHasiXMLAndBEXTReflectFileState() + { + // hasiXMLData() / hasBEXTData() must report whether the *file* carries an + // iXML / bext APPLICATION block, not whether in-memory payload happens to + // be non-empty. Regression test for an issue where the accessors were + // implemented as !data.isEmpty() and so flipped on as soon as set*Data() + // was called, before save(), and missed a real-but-empty block. + ScopedFileCopy copy("silence-44-s", ".flac"); + const string newname = copy.fileName(); + + { + FLAC::File f(newname.c_str()); + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + f.setiXMLData(""); + f.setBEXTData(ByteVector("bext")); + // File hasn't been saved yet — file still has no blocks. + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + f.save(); + // After save the blocks are on disk. + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + } + { + FLAC::File f(newname.c_str()); + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + f.setiXMLData(String()); + f.setBEXTData(ByteVector()); + // In-memory payload now empty but the file still carries both blocks. + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + f.save(); + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + } + } + void testRoundTripPreservesUnknownApplicationBlock() { // Source: silence-44-s with an extra APPLICATION/"SMED" block injected