tdf#128820: use wps namespace for simple text shapes

Without that, simple text shapes inside groups were written in
<pic:wsp> elements, with many child elements also having pic::
prefix.

Change-Id: I114cf3499e03aa5ca042211d7b134aaf5b0e7fbf
Reviewed-on: https://gerrit.libreoffice.org/82980
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>

Also consider saved exceptions when terminating parse

As with previous commit 18ae77a065cb8ae6940d4067f6ab7e99a3f74047, this
will start showing parse errors on invalid files which previously just
opened without warnings, silently losing the invalid stream part. Any
bug bisected to this commit is not a regression from this commit! The
real problem was already before, and was just disclosed by this (which
is the actual goal).

Also simplify unit test data for tdf#128820, which will now be enough
after the change.

A unit test (testN779627) revealed unexpected throws when parsing; this
was fixed.

Change-Id: I5a21b9001874ec6e3b8273c10043ef930bf1cc82
Reviewed-on: https://gerrit.libreoffice.org/82981
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>

tdf#128820: improve unit test to not depend on opening bad file failing

... also test exported XML directly

Change-Id: I50237593dd111e7c7974452769c8d49c22012713
Reviewed-on: https://gerrit.libreoffice.org/83005
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>

Fix UBSan build after 9fdf8c0a5cc036ea9bd1e11dd8f2c1a6e601fae2

The said commit simplified a testdoc to testTdf128820, using a smallest
possible SVG in it. This seems to produce the smallest possible PNG of
size 8, which is passed into GraphicDescriptor::ImpDetectPNG. There its
size is read into nTemp32 past the end of the file without checks,
which keeps last value of the variable (which was the magic number
0x0d0a1a0a), which is then saved into the descriptor. Then that value
is used in ImpGraphic::ImplGetSizePixel, and later multiplying it in
lclConvertScreenPixelToHmm causes UB.

Fix by checking all the reads in GraphicDescriptor::ImpDetectPNG.

Change-Id: Ib4740fac2b87fbef57d5150151129b9852f3ecb8
Reviewed-on: https://gerrit.libreoffice.org/83119
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/83296

7 files changed, 161 insertions, 81 deletions

diff --git a/oox/source/export/shapes.cxx b/oox/source/export/shapes.cxx
index ebab46728bef..9f69e316cbec 100644
--- a/oox/source/export/shapes.cxx
+++ b/oox/source/export/shapes.cxx
@@ -381,6 +381,14 @@ awt::Size ShapeExport::MapSize( const awt::Size& rSize ) const
     return awt::Size( aRetSize.Width(), aRetSize.Height() );
 }
 
+static bool IsNonEmptySimpleText(const Reference<XInterface>& xIface)
+{
+    if (Reference<XSimpleText> xText{ xIface, UNO_QUERY })
+        return xText->getString().getLength();
+
+    return false;
+}
+
 bool ShapeExport::NonEmptyText( const Reference< XInterface >& xIface )
 {
     Reference< XPropertySet > xPropSet( xIface, UNO_QUERY );
@@ -414,12 +422,7 @@ bool ShapeExport::NonEmptyText( const Reference< XInterface >& xIface )
         }
     }
 
-    Reference< XSimpleText > xText( xIface, UNO_QUERY );
-
-    if( xText.is() )
-        return xText->getString().getLength();
-
-    return false;
+    return IsNonEmptySimpleText(xIface);
 }
 
 ShapeExport& ShapeExport::WritePolyPolygonShape( const Reference< XShape >& xShape, const bool bClosed )
@@ -529,7 +532,11 @@ ShapeExport& ShapeExport::WriteGroupShape(const uno::Reference<drawing::XShape>&
         uno::Reference<lang::XServiceInfo> xServiceInfo(xChild, uno::UNO_QUERY_THROW);
         if (GetDocumentType() == DOCUMENT_DOCX)
         {
-            if (xServiceInfo->supportsService("com.sun.star.drawing.GraphicObjectShape"))
+            // tdf#128820: WriteGraphicObjectShapePart calls WriteTextShape for non-empty simple
+            // text objects, which needs writing into wps::wsp element, so make sure to use wps
+            // namespace for those objects
+            if (xServiceInfo->supportsService("com.sun.star.drawing.GraphicObjectShape")
+                && !IsNonEmptySimpleText(xChild))
                 mnXmlNamespace = XML_pic;
             else
                 mnXmlNamespace = XML_wps;
@@ -1165,19 +1172,13 @@ void ShapeExport::WriteGraphicObjectShapePart( const Reference< XShape >& xShape
 {
     SAL_INFO("oox.shape", "write graphic object shape");
 
-    if( NonEmptyText( xShape ) )
+    if (IsNonEmptySimpleText(xShape))
     {
-        // avoid treating all 'IsPresentationObject' objects as having text.
-        Reference< XSimpleText > xText( xShape, UNO_QUERY );
+        SAL_INFO("oox.shape", "graphicObject: wrote only text");
 
-        if( xText.is() && !xText->getString().isEmpty() )
-        {
-            SAL_INFO("oox.shape", "graphicObject: wrote only text");
+        WriteTextShape(xShape);
 
-            WriteTextShape( xShape );
-
-            return;
-        }
+        return;
     }
 
     SAL_INFO("oox.shape", "graphicObject without text");
diff --git a/sax/source/fastparser/fastparser.cxx b/sax/source/fastparser/fastparser.cxx
index 7383b8c339ea..9b35c1682be5 100644
--- a/sax/source/fastparser/fastparser.cxx
+++ b/sax/source/fastparser/fastparser.cxx
@@ -175,6 +175,8 @@ struct Entity : public ParserData
     css::uno::Any                           maSavedException;
     osl::Mutex maSavedExceptionMutex;
     void saveException( const Any & e );
+    // Thread-safe check if maSavedException has value
+    bool hasException();
     void throwException( const ::rtl::Reference< FastLocatorImpl > &xDocumentLocator,
                          bool mbDuringParse );
 
@@ -622,6 +624,12 @@ void Entity::saveException( const Any & e )
     }
 }
 
+bool Entity::hasException()
+{
+    osl::MutexGuard g(maSavedExceptionMutex);
+    return maSavedException.hasValue();
+}
+
 } // namespace
 
 namespace sax_fastparser {
@@ -1040,6 +1048,8 @@ void FastSaxParserImpl::parse()
             {
                 if( xmlParseChunk( rEntity.mpParser, reinterpret_cast<const char*>(seqOut.getConstArray()), 0, 1 ) != XML_ERR_OK )
                     rEntity.throwException( mxDocumentLocator, true );
+                if (rEntity.hasException())
+                    rEntity.throwException(mxDocumentLocator, true);
             }
             break;
         }
@@ -1068,10 +1078,8 @@ void FastSaxParserImpl::parse()
         {
             rEntity.throwException( mxDocumentLocator, true );
         }
-        osl::ClearableMutexGuard g(rEntity.maSavedExceptionMutex);
-        if (rEntity.maSavedException.hasValue())
+        if (rEntity.hasException())
         {
-            g.clear();
             rEntity.throwException( mxDocumentLocator, true );
         }
     } while( nRead > 0 );
diff --git a/sw/qa/extras/ooxmlexport/data/tdf128820.fodt b/sw/qa/extras/ooxmlexport/data/tdf128820.fodt
new file mode 100644
index 000000000000..576fd966e28d
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf128820.fodt
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:body>
+  <office:text>
+   <text:p><draw:g text:anchor-type="as-char" draw:name="A">
+     <draw:frame svg:width="1cm" svg:height="1cm" svg:x="1cm" svg:y="1cm">
+      <draw:image>
+       <office:binary-data>PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciLz4</office:binary-data>
+       <text:p>json</text:p>
+      </draw:image>
+     </draw:frame>
+   </draw:g></text:p>
+  </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index 98ee2ab45c48..c8c95d1cd132 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -143,6 +143,31 @@ DECLARE_OOXMLEXPORT_TEST(testTdf124367, "tdf124367.docx")
                              .Position);
 }
 
+DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128820, "tdf128820.fodt")
+{
+    // Import of exported DOCX failed because of wrong namespase used for wsp element
+    // Now test the exported XML, in case we stop failing opening invalid files
+    xmlDocPtr pXml = parseExport("word/document.xml");
+    CPPUNIT_ASSERT(pXml);
+    // The parent wpg:wgp element has three children: wpg:cNvGrpSpPr, wpg:grpSpPr, and wpg:wsp
+    // (if we start legitimately exporting additional children, this needs to be adjusted to check
+    // all those, to make sure we don't export wrong elements).
+    assertXPathChildren(pXml,
+                        "/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+                        "wp:inline/a:graphic/a:graphicData/wpg:wgp",
+                        3);
+    assertXPath(pXml,
+                "/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+                "a:graphic/a:graphicData/wpg:wgp/wpg:cNvGrpSpPr");
+    assertXPath(pXml,
+                "/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+                "a:graphic/a:graphicData/wpg:wgp/wpg:grpSpPr");
+    // This one was pic:wsp instead of wps:wsp
+    assertXPath(pXml,
+                "/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+                "a:graphic/a:graphicData/wpg:wgp/wps:wsp");
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/filter/graphicfilter2.cxx b/vcl/source/filter/graphicfilter2.cxx
index 5640e7c96ed2..0faaaeb81997 100644
--- a/vcl/source/filter/graphicfilter2.cxx
+++ b/vcl/source/filter/graphicfilter2.cxx
@@ -533,83 +533,93 @@ bool GraphicDescriptor::ImpDetectPNG( SvStream& rStm, bool bExtendedInfo )
 
             if ( bExtendedInfo )
             {
-                sal_uInt8 cByte = 0;
+                do {
+                    sal_uInt8 cByte = 0;
 
-                // IHDR-Chunk
-                rStm.SeekRel( 8 );
+                    // IHDR-Chunk
+                    rStm.SeekRel( 8 );
 
-                // width
-                rStm.ReadUInt32( nTemp32 );
-                aPixSize.setWidth( nTemp32 );
+                    // width
+                    rStm.ReadUInt32( nTemp32 );
+                    if (!rStm.good())
+                        break;
+                    aPixSize.setWidth( nTemp32 );
 
-                // height
-                rStm.ReadUInt32( nTemp32 );
-                aPixSize.setHeight( nTemp32 );
+                    // height
+                    rStm.ReadUInt32( nTemp32 );
+                    if (!rStm.good())
+                        break;
+                    aPixSize.setHeight( nTemp32 );
 
-                // Bits/Pixel
-                rStm.ReadUChar( cByte );
-                nBitsPerPixel = cByte;
+                    // Bits/Pixel
+                    rStm.ReadUChar( cByte );
+                    if (!rStm.good())
+                        break;
+                    nBitsPerPixel = cByte;
 
-                // Colour type - check whether it supports alpha values
-                sal_uInt8 cColType = 0;
-                rStm.ReadUChar( cColType );
-                bIsAlpha = bIsTransparent = ( cColType == 4 || cColType == 6 );
+                    // Colour type - check whether it supports alpha values
+                    sal_uInt8 cColType = 0;
+                    rStm.ReadUChar( cColType );
+                    if (!rStm.good())
+                         break;
+                    bIsAlpha = bIsTransparent = ( cColType == 4 || cColType == 6 );
 
-                // Planes always 1;
-                // compression always
-                nPlanes = 1;
+                    // Planes always 1;
+                    // compression always
+                    nPlanes = 1;
 
-                sal_uInt32  nLen32 = 0;
-                nTemp32 = 0;
+                    sal_uInt32  nLen32 = 0;
+                    nTemp32 = 0;
 
-                rStm.SeekRel( 7 );
+                    rStm.SeekRel( 7 );
 
-                // read up to the start of the image
-                rStm.ReadUInt32( nLen32 );
-                rStm.ReadUInt32( nTemp32 );
-                while( ( nTemp32 != 0x49444154 ) && rStm.good() )
-                {
-                    if ( nTemp32 == 0x70485973 ) // physical pixel dimensions
+                    // read up to the start of the image
+                    rStm.ReadUInt32( nLen32 );
+                    rStm.ReadUInt32( nTemp32 );
+                    while( ( nTemp32 != 0x49444154 ) && rStm.good() )
                     {
-                        sal_uLong   nXRes;
-                        sal_uLong   nYRes;
+                        if ( nTemp32 == 0x70485973 ) // physical pixel dimensions
+                        {
+                            sal_uLong   nXRes;
+                            sal_uLong   nYRes;
 
-                        // horizontal resolution
-                        nTemp32 = 0;
-                        rStm.ReadUInt32( nTemp32 );
-                        nXRes = nTemp32;
+                            // horizontal resolution
+                            nTemp32 = 0;
+                            rStm.ReadUInt32( nTemp32 );
+                            nXRes = nTemp32;
 
-                        // vertical resolution
-                        nTemp32 = 0;
-                        rStm.ReadUInt32( nTemp32 );
-                        nYRes = nTemp32;
+                            // vertical resolution
+                            nTemp32 = 0;
+                            rStm.ReadUInt32( nTemp32 );
+                            nYRes = nTemp32;
 
-                        // unit
-                        cByte = 0;
-                        rStm.ReadUChar( cByte );
+                            // unit
+                            cByte = 0;
+                            rStm.ReadUChar( cByte );
 
-                        if ( cByte )
-                        {
-                            if ( nXRes )
-                                aLogSize.setWidth( (aPixSize.Width() * 100000) / nXRes );
+                            if ( cByte )
+                            {
+                                if ( nXRes )
+                                    aLogSize.setWidth( (aPixSize.Width() * 100000) / nXRes );
+
+                                if ( nYRes )
+                                    aLogSize.setHeight( (aPixSize.Height() * 100000) / nYRes );
+                            }
 
-                            if ( nYRes )
-                                aLogSize.setHeight( (aPixSize.Height() * 100000) / nYRes );
+                            nLen32 -= 9;
+                        }
+                        else if ( nTemp32 == 0x74524e53 ) // transparency
+                        {
+                            bIsTransparent = true;
+                            bIsAlpha = ( cColType != 0 && cColType != 2 );
                         }
 
-                        nLen32 -= 9;
-                    }
-                    else if ( nTemp32 == 0x74524e53 ) // transparency
-                    {
-                        bIsTransparent = true;
-                        bIsAlpha = ( cColType != 0 && cColType != 2 );
+                        // skip forward to next chunk
+                        rStm.SeekRel( 4 + nLen32 );
+                        rStm.ReadUInt32( nLen32 );
+                        rStm.ReadUInt32( nTemp32 );
                     }
-
-                    // skip forward to next chunk
-                    rStm.SeekRel( 4 + nLen32 );
-                    rStm.ReadUInt32( nLen32 );
-                    rStm.ReadUInt32( nTemp32 );
-                }
+                } while (false);
             }
         }
     }
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 52a0418abd92..c2feb59199bc 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -2783,7 +2783,18 @@ void DomainMapper_Impl::PopShapeContext()
 
         // Relative width calculations deferred until section's margins are defined.
         // Being cautious: only deferring undefined/minimum-width shapes in order to avoid causing potential regressions
-        if( xShape->getSize().Width <= 2 )
+        css::awt::Size aShapeSize;
+        try
+        {
+            aShapeSize = xShape->getSize();
+        }
+        catch (const css::uno::RuntimeException& e)
+        {
+            // May happen e.g. when text frame has no frame format
+            // See sw/qa/extras/ooxmlimport/data/n779627.docx
+            SAL_WARN("writerfilter.dmapper", "getSize failed. " << e.Message);
+        }
+        if( aShapeSize.Width <= 2 )
         {
             const uno::Reference<beans::XPropertySet> xShapePropertySet( xShape, uno::UNO_QUERY );
             SectionPropertyMap* pSectionContext = GetSectionContext();
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx
index 86fee3a9a193..188b95634d62 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -1638,7 +1638,16 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl )
             if ( xShapePropertySet->getPropertySetInfo()->hasPropertyByName(sPropRelativeWidth) )
             {
                 sal_uInt16 nPercent = 0;
-                xShapePropertySet->getPropertyValue( sPropRelativeWidth ) >>= nPercent;
+                try
+                {
+                    xShapePropertySet->getPropertyValue(sPropRelativeWidth) >>= nPercent;
+                }
+                catch (const css::uno::RuntimeException& e)
+                {
+                    // May happen e.g. when text frame has no frame format
+                    // See sw/qa/extras/ooxmlimport/data/n779627.docx
+                    SAL_WARN("writerfilter", "Getting relative width failed. " << e.Message);
+                }
                 if ( nPercent )
                 {
                     const sal_Int32 nWidth = nParagraphWidth * nPercent / 100;