writerfilter: DOCX import: better error handling than "catch (...) {}"

If there is a SAXParseException, OOXMLDocumentImpl::resolve() should not
ignore it, because if it occurs in a substream some end tag handlers may
not have been run and the DomainMapper may be in an inconsistent state,
so continuing to parse the outer document is probably not a good idea.

Also add some exception mangling so sfx2 can present a useful error
dialog.

Change-Id: I169ba6db25f2ae264af08a64edf76a6bf6757f85

5 files changed, 134 insertions, 7 deletions

diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
index 6d7a87a68896..6a76477c06de 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
@@ -266,11 +266,6 @@ DECLARE_OOXMLEXPORT_TEST(testMathLim, "math-lim.docx")
     CHECK_FORMULA( "lim from {x \xe2\x86\x92 1} {x}", getFormula( getRun( getParagraph( 1 ), 1 )));
 }
 
-DECLARE_OOXMLEXPORT_TEST(testMathMalformedXml, "math-malformed_xml.docx")
-{
-    CPPUNIT_ASSERT_EQUAL( 0, getLength());
-}
-
 DECLARE_OOXMLEXPORT_TEST(testMathMatrix, "math-matrix.docx")
 {
     CHECK_FORMULA( "left [matrix {1 # 2 ## 3 # 4} right ]", getFormula( getRun( getParagraph( 1 ), 1 )));
diff --git a/sw/qa/extras/ooxmlexport/data/math-malformed_xml.docx b/sw/qa/extras/ooxmlimport/data/math-malformed_xml.docx
index 53d64b039142..53d64b039142 100644
--- a/sw/qa/extras/ooxmlexport/data/math-malformed_xml.docx
+++ b/sw/qa/extras/ooxmlimport/data/math-malformed_xml.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 52028993f9b3..a6fb079bdb88 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -89,6 +89,46 @@ public:
     }
 };
 
+class FailTest : public Test
+{
+public:
+    // UGLY: hacky manual override of MacrosTest::loadFromDesktop
+    void executeImportTest(const char* filename)
+    {
+        header();
+        preTest(filename);
+        {
+            if (mxComponent.is())
+                mxComponent->dispose();
+            std::cout << filename << ",";
+            mnStartTime = osl_getGlobalTimer();
+            {
+                OUString aURL(getURLFromSrc(mpTestDocumentPath) + OUString::createFromAscii(filename));
+                CPPUNIT_ASSERT_MESSAGE("no desktop", mxDesktop.is());
+                uno::Reference<frame::XComponentLoader> xLoader = uno::Reference<frame::XComponentLoader>(mxDesktop, uno::UNO_QUERY);
+                CPPUNIT_ASSERT_MESSAGE("no loader", xLoader.is());
+                uno::Sequence<beans::PropertyValue> args(1);
+                args[0].Name = "DocumentService";
+                args[0].Handle = -1;
+                args[0].Value <<= OUString("com.sun.star.text.TextDocument");
+                args[0].State = beans::PropertyState_DIRECT_VALUE;
+
+                uno::Reference<lang::XComponent> xComponent = xLoader->loadComponentFromURL(aURL, OUString("_default"), 0, args);
+                OUString sMessage = "loading succeeded: " + aURL;
+                CPPUNIT_ASSERT_MESSAGE(OUStringToOString(sMessage, RTL_TEXTENCODING_UTF8).getStr(), !xComponent.is());
+            }
+        }
+        postTest(filename);
+        verify();
+        finish();
+    }
+};
+
+DECLARE_SW_IMPORT_TEST(testMathMalformedXml, "math-malformed_xml.docx", FailTest)
+{
+    CPPUNIT_ASSERT(!mxComponent.is());
+}
+
 DECLARE_OOXMLIMPORT_TEST(testN751054, "n751054.docx")
 {
     text::TextContentAnchorType eValue = getProperty<text::TextContentAnchorType>(getShape(1), "AnchorType");
diff --git a/writerfilter/source/filter/ImportFilter.cxx b/writerfilter/source/filter/ImportFilter.cxx
index d14ce08e647c..f67a7ff0020e 100644
--- a/writerfilter/source/filter/ImportFilter.cxx
+++ b/writerfilter/source/filter/ImportFilter.cxx
@@ -23,6 +23,9 @@
 #include <com/sun/star/drawing/XDrawPageSupplier.hpp>
 #include <com/sun/star/io/XInputStream.hpp>
 #include <com/sun/star/lang/XMultiServiceFactory.hpp>
+#include <com/sun/star/lang/WrappedTargetRuntimeException.hpp>
+#include <com/sun/star/io/WrongFormatException.hpp>
+#include <com/sun/star/xml/sax/SAXParseException.hpp>
 #include <unotools/mediadescriptor.hxx>
 #include <cppuhelper/supportsservice.hxx>
 #include <oox/core/filterdetect.hxx>
@@ -37,9 +40,48 @@
 #include <oox/ole/olestorage.hxx>
 #include <oox/ole/vbaproject.hxx>
 #include <oox/helper/graphichelper.hxx>
+
 using namespace ::com::sun::star;
 
 
+static OUString lcl_GetExceptionMessageRec(xml::sax::SAXException const& e);
+
+static OUString lcl_GetExceptionMessage(xml::sax::SAXException const& e)
+{
+    OUString const thisMessage("SAXParseException: "
+        "\"" + e.Message + "\"");
+    OUString const restMessage(lcl_GetExceptionMessageRec(e));
+    return restMessage + "\n" + thisMessage;
+}
+static OUString lcl_GetExceptionMessage(xml::sax::SAXParseException const& e)
+{
+    OUString const thisMessage("SAXParseException: "
+        "\"" + e.Message + "\" "
+        + "stream \"" + e.SystemId + "\""
+        + ", Line " + OUString::number(e.LineNumber)
+        + ", Column " + OUString::number(e.ColumnNumber));
+    OUString const restMessage(lcl_GetExceptionMessageRec(e));
+    return restMessage + "\n" + thisMessage;
+}
+static OUString lcl_GetExceptionMessageRec(xml::sax::SAXException const& e)
+{
+    xml::sax::SAXParseException saxpe;
+    if (e.WrappedException >>= saxpe)
+    {
+        return lcl_GetExceptionMessage(saxpe);
+    }
+    xml::sax::SAXException saxe;
+    if (e.WrappedException >>= saxe)
+    {
+        return lcl_GetExceptionMessage(saxe);
+    }
+    uno::Exception ue;
+    if (e.WrappedException >>= ue)
+    {
+        return ue.Message;
+    }
+    return OUString();
+}
 
 sal_Bool WriterFilter::filter( const uno::Sequence< beans::PropertyValue >& aDescriptor )
    throw (uno::RuntimeException, std::exception)
@@ -93,7 +135,35 @@ sal_Bool WriterFilter::filter( const uno::Sequence< beans::PropertyValue >& aDes
             (xDrawings->getDrawPage(), uno::UNO_SET_THROW);
         pDocument->setDrawPage(xDrawPage);
 
-        pDocument->resolve(*pStream);
+        try
+        {
+            pDocument->resolve(*pStream);
+        }
+        catch (xml::sax::SAXParseException const& e)
+        {
+            // note: SfxObjectShell checks for WrongFormatException
+            io::WrongFormatException wfe(lcl_GetExceptionMessage(e));
+            throw lang::WrappedTargetRuntimeException("",
+                    static_cast<OWeakObject*>(this), uno::makeAny(wfe));
+        }
+        catch (xml::sax::SAXException const& e)
+        {
+            // note: SfxObjectShell checks for WrongFormatException
+            io::WrongFormatException wfe(lcl_GetExceptionMessage(e));
+            throw lang::WrappedTargetRuntimeException("",
+                    static_cast<OWeakObject*>(this), uno::makeAny(wfe));
+        }
+        catch (uno::RuntimeException const& e)
+        {
+            throw;
+        }
+        catch (uno::Exception const& e)
+        {
+            SAL_WARN("writerfilter", "WriterFilter::filter(): "
+                    "failed with exception " << e.Message);
+            throw lang::WrappedTargetRuntimeException("",
+                    static_cast<OWeakObject*>(this), uno::makeAny(e));
+        }
 
         // Adding some properties to the document's grab bag for interoperability purposes:
         comphelper::SequenceAsHashMap aGrabBagProperties;
diff --git a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
index 7f3508e6bea4..0481bd1561de 100644
--- a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
+++ b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx
@@ -21,6 +21,7 @@
 
 #include <com/sun/star/xml/sax/XParser.hpp>
 
+#include <com/sun/star/lang/WrappedTargetRuntimeException.hpp>
 #include <com/sun/star/document/XDocumentPropertiesSupplier.hpp>
 #include <com/sun/star/xml/sax/SAXException.hpp>
 #include <com/sun/star/xml/dom/DocumentBuilder.hpp>
@@ -500,7 +501,28 @@ void OOXMLDocumentImpl::resolve(Stream & rStream)
         {
             xParser->parseStream(aParserInput);
         }
-        catch (...) {
+        catch (xml::sax::SAXException const&)
+        {
+            // don't swallow these - handlers may not have been executed,
+            // and the domain mapper is likely in an inconsistent state
+            throw;
+        }
+        catch (uno::RuntimeException const& e)
+        {
+            throw;
+        }
+        // note: cannot throw anything other than SAXException out of here?
+        catch (uno::Exception const& e)
+        {
+            SAL_WARN("writerfilter.ooxml",
+                "OOXMLDocumentImpl::resolve(): exception: " << e.Message);
+            throw lang::WrappedTargetRuntimeException("", nullptr,
+                    uno::makeAny(e));
+        }
+        catch (...)
+        {
+            SAL_WARN("writerfilter.ooxml",
+                "OOXMLDocumentImpl::resolve(): non-UNO exception");
         }
     }