From 18b2e7f981d118b911e7858a9711346cf5c7b13c Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Jun 27 2016 20:39:42 +0000 Subject: Related: tdf#91807 fix parallel zip error handling --- diff --git a/0001-package-fix-exception-handling-in-DeflateThread-rela.patch b/0001-package-fix-exception-handling-in-DeflateThread-rela.patch new file mode 100644 index 0000000..277bb8b --- /dev/null +++ b/0001-package-fix-exception-handling-in-DeflateThread-rela.patch @@ -0,0 +1,132 @@ +From 60f668ba594c08aa935887ecd0e76d6ba016f166 Mon Sep 17 00:00:00 2001 +From: Michael Stahl +Date: Thu, 23 Jun 2016 11:24:55 +0200 +Subject: [PATCH] package: fix exception handling in DeflateThread (related + tdf#91807) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In the bugdoc of tdf#91807 there are at least 49 corrupt zip streams +that raise exceptions in the DeflateThreads. Because the maximum +allowed number of threads happens to be 48, this results in an infinite +loop in ZipOutputStream::reduceScheduledThreadsToGivenNumberOrLess(). + +(regression from 7e2ea27e5d56f5cf767a6718a0f5edc28e24af14) + +In case an exception is thrown, don't re-throw it immediately, which +might cause trouble such as leaking all of the ZipOutputEntry instances +in m_aEntries. + +(cherry picked from commit 8d8b9b80b114b94b20b0bf1438d80e925b49e3bf) + +sfx2: exception on storage commit is an error (related: tdf#91807) + +For no good or obvious reason, SfxMedium::StorageCommit_Impl() swallows +embed::UseBackupException if there is a pTempFile, which (as the comment +claims) is "always now". This results in the temp file actually being +copied to the user-visible file and the SaveAs "succeeding", when it +clearly did not. + +Also move the exception throwing to the end of ZipOutputStream::finish() +to avoid more memory leaks. + +(cherry picked from commit 9084c11fb472f2024e609770ce922c911227e7a8) + +Change-Id: I448cc43291754ef20adfa6b65916282fcc365a11 +Reviewed-on: https://gerrit.libreoffice.org/26618 +Tested-by: Jenkins +Reviewed-by: Caolán McNamara +Tested-by: Caolán McNamara +--- + package/inc/ZipOutputStream.hxx | 1 + + package/source/zipapi/ZipOutputStream.cxx | 12 +++++++++++- + package/source/zippackage/ZipPackageStream.cxx | 11 +++++++++++ + sfx2/source/doc/docfile.cxx | 6 +++--- + 4 files changed, 26 insertions(+), 4 deletions(-) + +diff --git a/package/inc/ZipOutputStream.hxx b/package/inc/ZipOutputStream.hxx +index f995469..0f30618 100644 +--- a/package/inc/ZipOutputStream.hxx ++++ b/package/inc/ZipOutputStream.hxx +@@ -40,6 +40,7 @@ class ZipOutputStream + ZipEntry *m_pCurrentEntry; + comphelper::ThreadPool &m_rSharedThreadPool; + std::vector< ZipOutputEntry* > m_aEntries; ++ ::css::uno::Any m_aDeflateException; + + public: + ZipOutputStream( +diff --git a/package/source/zipapi/ZipOutputStream.cxx b/package/source/zipapi/ZipOutputStream.cxx +index 8cd8b90..94413e2 100644 +--- a/package/source/zipapi/ZipOutputStream.cxx ++++ b/package/source/zipapi/ZipOutputStream.cxx +@@ -98,7 +98,12 @@ void ZipOutputStream::consumeScheduledThreadEntry(ZipOutputEntry* pCandidate) + //Any exceptions thrown in the threads were caught and stored for now + ::css::uno::Any aCaughtException(pCandidate->getParallelDeflateException()); + if (aCaughtException.hasValue()) +- ::cppu::throwException(aCaughtException); ++ { ++ m_aDeflateException = aCaughtException; // store it for later throwing ++ // the exception handler in DeflateThread should have cleaned temp file ++ delete pCandidate; ++ return; ++ } + + writeLOC(pCandidate->getZipEntry(), pCandidate->isEncrypt()); + +@@ -187,6 +192,11 @@ void ZipOutputStream::finish() + writeEND( nOffset, static_cast < sal_Int32 > (m_aChucker.GetPosition()) - nOffset); + m_xStream->flush(); + m_aZipList.clear(); ++ ++ if (m_aDeflateException.hasValue()) ++ { // throw once all threads are finished and m_aEntries can be released ++ ::cppu::throwException(m_aDeflateException); ++ } + } + + css::uno::Reference< css::io::XOutputStream > ZipOutputStream::getStream() +diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx +index 0ff190a..3e84d1e 100644 +--- a/package/source/zippackage/ZipPackageStream.cxx ++++ b/package/source/zippackage/ZipPackageStream.cxx +@@ -486,6 +486,17 @@ private: + catch (const uno::Exception&) + { + mpEntry->setParallelDeflateException(::cppu::getCaughtException()); ++ try ++ { ++ if (mpEntry->m_xOutStream.is()) ++ mpEntry->closeBufferFile(); ++ if (!mpEntry->m_aTempURL.isEmpty()) ++ mpEntry->deleteBufferFile(); ++ } ++ catch (uno::Exception const&) ++ { ++ } ++ mpEntry->setFinished(); + } + } + }; +diff --git a/sfx2/source/doc/docfile.cxx b/sfx2/source/doc/docfile.cxx +index e30deb7..fe2078f 100644 +--- a/sfx2/source/doc/docfile.cxx ++++ b/sfx2/source/doc/docfile.cxx +@@ -1620,10 +1620,10 @@ bool SfxMedium::StorageCommit_Impl() + OSL_ENSURE( !pImp->m_aName.isEmpty(), "The exception _must_ contain the temporary URL!\n" ); + } + } +- +- if ( !GetError() ) +- SetError( ERRCODE_IO_GENERAL, OSL_LOG_PREFIX ); + } ++ ++ if (!GetError()) ++ SetError( ERRCODE_IO_GENERAL, OSL_LOG_PREFIX ); + } + catch ( const uno::Exception& ) + { +-- +2.5.5 + diff --git a/libreoffice.spec b/libreoffice.spec index d45d4fa..dd6a50b 100644 --- a/libreoffice.spec +++ b/libreoffice.spec @@ -267,6 +267,7 @@ Patch38: 0001-Resolves-rhbz-1349501-gtk3-smooth-scrolling-events-c.patch Patch39: 0001-Resolves-rhbz-1326304-cannot-detect-loss-of-wayland-.patch Patch40: 0001-gtk3-set-decoration-bits-etc-before-realize.patch Patch41: 0001-Resolves-rhbz-1342823-toolbar-menus-popdown-immediat.patch +Patch42: 0001-package-fix-exception-handling-in-DeflateThread-rela.patch %if ! 0%{?rhel} Patch400: 0001-Update-liborcus-to-0.11.0.patch @@ -2287,6 +2288,7 @@ done %changelog * Mon Jun 27 2016 Caolán McNamara - 1:5.1.4.1-4-UNBUILT - Resolves: rhbz#1342823 toolbar menus popdown immediately +- Related: tdf#91807 fix parallel zip error handling * Fri Jun 24 2016 Caolán McNamara - 1:5.1.4.1-3 - Resolves: rhbz#1349501 gtk3 smooth scrolling events can be user-disabled