(related: fdo#79319) writerfilter: RTF import: fix crash on "fake" pict

The document has a \pict with {\sp{\sn shapeType}{\sv 1}}, i.e. it's
actually a rectangle shape; Word seems to ignore the picture data in
this case, so try to do the same.  Also consolidate the shape creation
in a new function RTFSdrImport::initShape().

(regression from ba9b63d8101197d3fd8612193b1ca188271dfc1a)

(cherry picked from commit 2b9e782497cb962d9ca74a851a1389b0e29df49c)

Conflicts:
	writerfilter/source/rtftok/rtfsdrimport.cxx
	writerfilter/source/rtftok/rtfsdrimport.hxx

Change-Id: Iec94852ddc4c1ca3d8284119e6f1818a8dbb4149
Reviewed-on: https://gerrit.libreoffice.org/10387
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

3 files changed, 101 insertions, 52 deletions

diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index bee142c069bc..ef5c241721a6 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -4941,9 +4941,6 @@ int RTFDocumentImpl::popState()
     case DESTINATION_BOOKMARKEND:
         Mapper().props(lcl_getBookmarkProperties(m_aBookmarks[m_aStates.top().aDestinationText.makeStringAndClear()]));
         break;
-    case DESTINATION_PICT:
-        resolvePict(true, m_pSdrImport->getCurrentShape());
-        break;
     case DESTINATION_FORMFIELDNAME:
     {
         RTFValue::Pointer_t pValue(new RTFValue(m_aStates.top().aDestinationText.makeStringAndClear()));
@@ -5231,6 +5228,13 @@ int RTFDocumentImpl::popState()
             Mapper().endShape();
         }
         break;
+    case DESTINATION_PICT:
+        // fdo#79319 ignore picture data if it's really a shape
+        if (!m_pSdrImport->isFakePict())
+        {
+            resolvePict(true, m_pSdrImport->getCurrentShape());
+        }
+        break;
     case DESTINATION_SHAPE:
         m_bNeedCr = m_bNeedCrOrig;
         if (aState.aFrame.inFrame())
diff --git a/writerfilter/source/rtftok/rtfsdrimport.cxx b/writerfilter/source/rtftok/rtfsdrimport.cxx
index 5edf4254c5d7..22e257b078c2 100644
--- a/writerfilter/source/rtftok/rtfsdrimport.cxx
+++ b/writerfilter/source/rtftok/rtfsdrimport.cxx
@@ -49,8 +49,9 @@ namespace rtftok
 
 RTFSdrImport::RTFSdrImport(RTFDocumentImpl& rDocument,
                            uno::Reference<lang::XComponent> const& xDstDoc)
-    : m_rImport(rDocument),
-      m_bTextFrame(false)
+    : m_rImport(rDocument)
+    , m_bTextFrame(false)
+    , m_bFakePict(false)
 {
     uno::Reference<drawing::XDrawPageSupplier> xDrawings(xDstDoc, uno::UNO_QUERY);
     if (xDrawings.is())
@@ -215,21 +216,92 @@ void RTFSdrImport::applyProperty(uno::Reference<drawing::XShape> xShape, const O
     }
 }
 
-void RTFSdrImport::resolve(RTFShape& rShape, bool bClose,
-        ShapeOrPict const shapeOrPict)
+int RTFSdrImport::initShape(
+    uno::Reference<drawing::XShape> & o_xShape,
+    uno::Reference<beans::XPropertySet> & o_xPropSet,
+    bool & o_rIsCustomShape,
+    RTFShape const& rShape, bool const bClose, ShapeOrPict const shapeOrPict)
 {
+    assert(!o_xShape.is());
+    assert(!o_xPropSet.is());
+    o_rIsCustomShape = false;
+    m_bFakePict = false;
+
+    // first, find the shape type
     int nType = -1;
+    std::vector< std::pair<OUString, OUString> >::const_iterator const iter(
+        std::find_if(rShape.aProperties.begin(),
+                     rShape.aProperties.end(),
+                     boost::bind(&OUString::equals,
+                         boost::bind(&std::pair<OUString, OUString>::first, _1),
+                         OUString("shapeType"))));
+
+    if (iter == rShape.aProperties.end())
+    {
+        if (SHAPE == shapeOrPict)
+        {
+            // The spec doesn't state what is the default for shapeType,
+            // Word seems to implement it as a rectangle.
+            nType = ESCHER_ShpInst_Rectangle;
+        }
+        else
+        {   // pict is picture by default but can be a rectangle too fdo#79319
+            nType = ESCHER_ShpInst_PictureFrame;
+        }
+    }
+    else
+    {
+        nType = iter->second.toInt32();
+        if (PICT == shapeOrPict && ESCHER_ShpInst_PictureFrame != nType)
+        {
+            m_bFakePict = true;
+        }
+    }
+
+    switch (nType)
+    {
+    case ESCHER_ShpInst_PictureFrame:
+        createShape("com.sun.star.drawing.GraphicObjectShape", o_xShape, o_xPropSet);
+        break;
+    case ESCHER_ShpInst_Line:
+        createShape("com.sun.star.drawing.LineShape", o_xShape, o_xPropSet);
+        break;
+    case ESCHER_ShpInst_Rectangle:
+    case ESCHER_ShpInst_TextBox:
+        // If we're inside a groupshape, can't use text frames.
+        if (!bClose && m_aParents.size() == 1)
+        {
+            createShape("com.sun.star.text.TextFrame", o_xShape, o_xPropSet);
+            m_bTextFrame = true;
+            std::vector<beans::PropertyValue> aDefaults = getTextFrameDefaults(true);
+            for (size_t j = 0; j < aDefaults.size(); ++j)
+                o_xPropSet->setPropertyValue(aDefaults[j].Name, aDefaults[j].Value);
+            break;
+        }
+        // fall-through intended
+    default:
+        createShape("com.sun.star.drawing.CustomShape", o_xShape, o_xPropSet);
+        o_rIsCustomShape = true;
+        break;
+    }
+
+    // Defaults
+    if (o_xPropSet.is() && !m_bTextFrame)
+    {
+        o_xPropSet->setPropertyValue("FillColor", uno::makeAny(sal_uInt32(
+                        0xffffff))); // White in Word, kind of blue in Writer.
+    }
+
+    return nType;
+}
+
+void RTFSdrImport::resolve(RTFShape& rShape, bool bClose, ShapeOrPict const shapeOrPict)
+{
     bool bPib = false;
-    bool bCustom = false;
     m_bTextFrame = false;
 
     uno::Reference<drawing::XShape> xShape;
     uno::Reference<beans::XPropertySet> xPropertySet;
-    // Create this early, as custom shapes may have properties before the type arrives.
-    if (PICT == shapeOrPict)
-        createShape("com.sun.star.drawing.GraphicObjectShape", xShape, xPropertySet);
-    else
-        createShape("com.sun.star.drawing.CustomShape", xShape, xPropertySet);
     uno::Any aAny;
     beans::PropertyValue aPropertyValue;
     awt::Rectangle aViewBox;
@@ -253,51 +325,16 @@ void RTFSdrImport::resolve(RTFShape& rShape, bool bClose,
     sal_Int16 nRelativeWidthRelation = text::RelOrientation::PAGE_FRAME;
     sal_Int16 nRelativeHeightRelation = text::RelOrientation::PAGE_FRAME;
 
-    // The spec doesn't state what is the default for shapeType, Word seems to implement it as a rectangle.
-    if (SHAPE == shapeOrPict &&
-        std::find_if(rShape.aProperties.begin(),
-                     rShape.aProperties.end(),
-                     boost::bind(&OUString::equals, boost::bind(&std::pair<OUString, OUString>::first, _1), OUString("shapeType")))
-            == rShape.aProperties.end())
-        rShape.aProperties.insert(rShape.aProperties.begin(), std::pair<OUString, OUString>("shapeType", OUString::number(ESCHER_ShpInst_Rectangle)));
+    bool bCustom(false);
+    int const nType =
+        initShape(xShape, xPropertySet, bCustom, rShape, bClose, shapeOrPict);
 
     for (std::vector< std::pair<OUString, OUString> >::iterator i = rShape.aProperties.begin();
             i != rShape.aProperties.end(); ++i)
     {
         if (i->first == "shapeType")
         {
-            nType = i->second.toInt32();
-            switch (nType)
-            {
-            case ESCHER_ShpInst_PictureFrame:
-                createShape("com.sun.star.drawing.GraphicObjectShape", xShape, xPropertySet);
-                break;
-            case ESCHER_ShpInst_Line:
-                createShape("com.sun.star.drawing.LineShape", xShape, xPropertySet);
-                break;
-            case ESCHER_ShpInst_Rectangle:
-            case ESCHER_ShpInst_TextBox:
-                // If we're inside a groupshape, can't use text frames.
-                if (!bClose && m_aParents.size() == 1)
-                {
-                    createShape("com.sun.star.text.TextFrame", xShape, xPropertySet);
-                    m_bTextFrame = true;
-                    std::vector<beans::PropertyValue> aDefaults = getTextFrameDefaults(true);
-                    for (size_t j = 0; j < aDefaults.size(); ++j)
-                        xPropertySet->setPropertyValue(aDefaults[j].Name, aDefaults[j].Value);
-                }
-                else
-                    bCustom = true;
-                break;
-            default:
-                bCustom = true;
-                break;
-            }
-
-            // Defaults
-            aAny <<= (sal_uInt32)0xffffff; // White in Word, kind of blue in Writer.
-            if (xPropertySet.is() && !m_bTextFrame)
-                xPropertySet->setPropertyValue("FillColor", aAny);
+            continue; // ignore: already handled by initShape
         }
         else if (i->first == "wzName")
         {
diff --git a/writerfilter/source/rtftok/rtfsdrimport.hxx b/writerfilter/source/rtftok/rtfsdrimport.hxx
index 0b6958993bb2..927f4505e640 100644
--- a/writerfilter/source/rtftok/rtfsdrimport.hxx
+++ b/writerfilter/source/rtftok/rtfsdrimport.hxx
@@ -45,15 +45,23 @@ public:
     void popParent();
     css::uno::Reference<css::drawing::XShape> const& getCurrentShape()
         { return m_xShape; }
+    bool isFakePict() { return m_bFakePict; }
 private:
     void createShape(const OUString& aService, css::uno::Reference<css::drawing::XShape>& xShape, css::uno::Reference<css::beans::XPropertySet>& xPropertySet);
     void applyProperty(css::uno::Reference<css::drawing::XShape> xShape, const OUString& aKey, const OUString& aValue);
+    int initShape(
+        css::uno::Reference<css::drawing::XShape> & o_xShape,
+        css::uno::Reference<css::beans::XPropertySet> & o_xPropSet,
+        bool & o_rIsCustomShape,
+        RTFShape const& rShape, bool bClose, ShapeOrPict const shapeOrPict);
 
     RTFDocumentImpl& m_rImport;
     std::stack< css::uno::Reference<css::drawing::XShapes> > m_aParents;
     css::uno::Reference<css::drawing::XShape> m_xShape;
     /// If m_xShape is imported as a Writer text frame (instead of a drawinglayer rectangle).
     bool m_bTextFrame;
+    /// if inside \pict, but actually it's a shape (not a picture)
+    bool m_bFakePict;
 };
 } // namespace rtftok
 } // namespace writerfilter