package/qa/cppunit/data/tdf166862.odt |binary package/qa/cppunit/test_zippackage.cxx | 18 ++++++++++++++++++ package/source/zipapi/ZipFile.cxx | 11 ++++++----- 3 files changed, 24 insertions(+), 5 deletions(-)
New commits: commit 90146f70bf3840a7f093cebed4f5857329eea42d Author: Mike Kaganski <[email protected]> AuthorDate: Thu Jun 5 14:35:36 2025 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Jun 9 11:25:08 2025 +0200 tdf#166862: fix reading data for CRC calculation Commit a6ad198d097fb4a503c8d5831d484ff46721134b made the size read from the grabber wrong for the last chunk: it wasn't decreased to the remaining size, but was still equal to nBlockSize. The grabber could read past the chunk, and the CRC was wrong. No idea why did that only hit this file - it seemed that any file larger than 32K and that was stored (i.e., any image) would trigger that path? Change-Id: I980dd701e54c4ed565173b5b90ed301bf9b5f719 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186241 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> (cherry picked from commit 9727cf785945218a4f06562f4edb657551d25436) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186280 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/package/qa/cppunit/data/tdf166862.odt b/package/qa/cppunit/data/tdf166862.odt new file mode 100644 index 000000000000..814bebe670d7 Binary files /dev/null and b/package/qa/cppunit/data/tdf166862.odt differ diff --git a/package/qa/cppunit/test_zippackage.cxx b/package/qa/cppunit/test_zippackage.cxx index f296802e4524..94b8c616400f 100644 --- a/package/qa/cppunit/test_zippackage.cxx +++ b/package/qa/cppunit/test_zippackage.cxx @@ -483,6 +483,24 @@ CPPUNIT_TEST_FIXTURE(ZipPackageTest, testDataDescriptorDirectory) CPPUNIT_ASSERT_EQUAL(sal_Int64(899), xStream->getPropertyValue(u"Size"_ustr).get<sal_Int64>()); } +CPPUNIT_TEST_FIXTURE(ZipPackageTest, testTdf166862) +{ + // This package is broken; but the actual issue was incorrect CRC calculation for an image + OUString aURL = m_directories.getURLFromSrc(u"/package/qa/cppunit/data/tdf166862.odt"); + uno::Sequence<uno::Any> args{ uno::Any(aURL), uno::Any(beans::NamedValue{ u"RepairPackage"_ustr, + uno::Any(true) }) }; + + auto xMgr = m_xContext->getServiceManager(); + + auto xZip(xMgr->createInstanceWithArgumentsAndContext(ZipPackage, args, m_xContext) + .queryThrow<container::XHierarchicalNameAccess>()); + static constexpr OUString picURL = u"Pictures/1000000000000596000000AE0BDDB537.jpg"_ustr; + CPPUNIT_ASSERT(xZip->hasByHierarchicalName(picURL)); + auto aPic = xZip->getByHierarchicalName(picURL).queryThrow<beans::XPropertySet>(); + // Before the fix, this was 0: the size was reset because of unmatched CRC + CPPUNIT_ASSERT_EQUAL(sal_Int64(43886), aPic->getPropertyValue(u"Size"_ustr).get<sal_Int64>()); +} + //CPPUNIT_TEST_SUITE_REGISTRATION(...); //CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 46c8ad11ee0d..934930e943a1 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -1939,12 +1939,13 @@ sal_Int32 ZipFile::getCRC( sal_Int64 nOffset, sal_Int64 nSize ) std::vector<sal_Int8> aBuffer(nBlockSize); aGrabber.seek( nOffset ); - sal_Int32 nRead; - for (sal_Int64 ind = 0; - (nRead = aGrabber.readBytes( aBuffer.data(), nBlockSize )) && ind * nBlockSize < nSize; - ++ind) + sal_Int64 nRead = 0; + while (nRead < nSize) { - aCRC.updateSegment(aBuffer.data(), nRead); + sal_Int64 nToRead = std::min(nSize - nRead, nBlockSize); + sal_Int64 nReadThisTime = aGrabber.readBytes(aBuffer.data(), nToRead); + aCRC.updateSegment(aBuffer.data(), nReadThisTime); + nRead += nReadThisTime; } return aCRC.getValue();
