Blob Blame History Raw
From 2639ba68dc1986c9b9bcb452f98c812a5f434f15 Mon Sep 17 00:00:00 2001
From: Michael Stahl <mstahl@redhat.com>
Date: Thu, 23 Jun 2016 11:24:55 +0200
Subject: [PATCH 3/3] 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)

Reviewed-on: https://gerrit.libreoffice.org/26618
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit 60f668ba594c08aa935887ecd0e76d6ba016f166)

Change-Id: I448cc43291754ef20adfa6b65916282fcc365a11
---
 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 88e19bb..876b904 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 d77e13b..42a4282 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());
 
@@ -189,6 +194,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 9c3f991..489d6fc 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 fcb7f84..3105e7c 100644
--- a/sfx2/source/doc/docfile.cxx
+++ b/sfx2/source/doc/docfile.cxx
@@ -1511,10 +1511,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, OUString( OSL_LOG_PREFIX  ) );
                     }
+
+                    if (!GetError())
+                        SetError( ERRCODE_IO_GENERAL, OUString( OSL_LOG_PREFIX  ) );
                 }
                 catch ( const uno::Exception& )
                 {
-- 
2.5.5