xmloff: Prevent gutter margin xml property to be deallocated

Previously we stored a pointer to the gutter margin XMLPropertyState
so we could read it later. The problem with this is that in between
when we storing and reading the property state, we add elements
to the rProperties vector, where the XML properties are allocated.

Adding new elements to the vector can cause that the internal
array is reallocated to a bigger size and elements copied to the
new array, so our pointer shows to a invalid (deallocated) memory
address.

This issue is fixed by moving the code up to before we add new
elements to rProperties vector and a big warning is written to not
use XMLPropertyState* pointers after the code adds to the
rProperties vector.

Change-Id: I24b0285d49e678fcb3b333bf4054f5cef4207003
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124572
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>

1 files changed, 71 insertions, 63 deletions

diff --git a/xmloff/source/style/PageMasterImportPropMapper.cxx b/xmloff/source/style/PageMasterImportPropMapper.cxx
index b9850eaec618..8ee711804333 100644
--- a/xmloff/source/style/PageMasterImportPropMapper.cxx
+++ b/xmloff/source/style/PageMasterImportPropMapper.cxx
@@ -390,6 +390,74 @@ void PageMasterImportPropertyMapper::finished(std::vector< XMLPropertyState >& r
         pAllFooterMarginProperty->mnIndex = -1;
     }
 
+    if (pMarginGutter)
+    {
+        sal_Int32 nGutterMargin{};
+        pMarginGutter->maValue >>= nGutterMargin;
+
+        bool bGutterAtTop{};
+        uno::Reference<lang::XServiceInfo> xSI(GetImport().GetModel(), uno::UNO_QUERY);
+        if (xSI.is() && xSI->supportsService("com.sun.star.text.TextDocument"))
+        {
+            uno::Reference<lang::XMultiServiceFactory> xFac(GetImport().GetModel(), uno::UNO_QUERY);
+            if (xFac.is())
+            {
+                uno::Reference<beans::XPropertySet> xProps(
+                    xFac->createInstance("com.sun.star.document.Settings"), uno::UNO_QUERY);
+                if (xProps.is())
+                {
+                    xProps->getPropertyValue("GutterAtTop") >>= bGutterAtTop;
+                }
+            }
+        }
+        if (bGutterAtTop)
+        {
+            if (nGutterMargin && pMargins[XML_LINE_TOP])
+            {
+                // Decrease top margin to not include gutter.
+                sal_Int32 nTopMargin{};
+                pMargins[XML_LINE_TOP]->maValue >>= nTopMargin;
+                nTopMargin -= nGutterMargin;
+                pMargins[XML_LINE_TOP]->maValue <<= nTopMargin;
+            }
+        }
+        else
+        {
+            bool bRtlGutter{};
+            if (nGutterMargin && pRtlGutter)
+            {
+                pRtlGutter->maValue >>= bRtlGutter;
+            }
+            if (bRtlGutter)
+            {
+                if (nGutterMargin && pMargins[XML_LINE_RIGHT])
+                {
+                    // Decrease right margin to not include gutter.
+                    sal_Int32 nRightMargin{};
+                    pMargins[XML_LINE_RIGHT]->maValue >>= nRightMargin;
+                    nRightMargin -= nGutterMargin;
+                    pMargins[XML_LINE_RIGHT]->maValue <<= nRightMargin;
+                }
+            }
+            else
+            {
+                if (nGutterMargin && pMargins[XML_LINE_LEFT])
+                {
+                    // Decrease left margin to not include gutter.
+                    sal_Int32 nLeftMargin{};
+                    pMargins[XML_LINE_LEFT]->maValue >>= nLeftMargin;
+                    nLeftMargin -= nGutterMargin;
+                    pMargins[XML_LINE_LEFT]->maValue <<= nLeftMargin;
+                }
+            }
+        }
+    }
+
+    // CAUTION!
+    // The following code adds into the rProperties vector, so all the
+    // XMLPropertyState* pointers that are pointing to the rProperties
+    // elements could potentially be deallocated, so don't use them after
+    // this!
     for (sal_uInt16 i = 0; i < 4; i++)
     {
         if (pNewMargins[i])
@@ -435,6 +503,9 @@ void PageMasterImportPropertyMapper::finished(std::vector< XMLPropertyState >& r
             delete pFooterNewBorders[i];
         }
     }
+    // CAUTION - do not use XMLPropertyState* pointers (like pMargins,
+    // pMarginGutter) after this.
+
     if(xHeaderDynamic)
     {
         rProperties.push_back(*xHeaderDynamic);
@@ -446,68 +517,5 @@ void PageMasterImportPropertyMapper::finished(std::vector< XMLPropertyState >& r
         xFooterDynamic.reset();
     }
 
-    if (!pMarginGutter)
-        return;
-
-    sal_Int32 nGutterMargin{};
-    pMarginGutter->maValue >>= nGutterMargin;
-
-    bool bGutterAtTop{};
-    uno::Reference<lang::XServiceInfo> xSI(GetImport().GetModel(), uno::UNO_QUERY);
-    if (xSI.is() && xSI->supportsService("com.sun.star.text.TextDocument"))
-    {
-        uno::Reference<lang::XMultiServiceFactory> xFac(GetImport().GetModel(), uno::UNO_QUERY);
-        if (xFac.is())
-        {
-            uno::Reference<beans::XPropertySet> xProps(
-                xFac->createInstance("com.sun.star.document.Settings"), uno::UNO_QUERY);
-            if (xProps.is())
-            {
-                xProps->getPropertyValue("GutterAtTop") >>= bGutterAtTop;
-            }
-        }
-    }
-    if (bGutterAtTop)
-    {
-        if (nGutterMargin && pMargins[XML_LINE_TOP])
-        {
-            // Decrease top margin to not include gutter.
-            sal_Int32 nTopMargin{};
-            pMargins[XML_LINE_TOP]->maValue >>= nTopMargin;
-            nTopMargin -= nGutterMargin;
-            pMargins[XML_LINE_TOP]->maValue <<= nTopMargin;
-        }
-    }
-    else
-    {
-        bool bRtlGutter{};
-        if (nGutterMargin && pRtlGutter)
-        {
-            pRtlGutter->maValue >>= bRtlGutter;
-        }
-        if (bRtlGutter)
-        {
-            if (nGutterMargin && pMargins[XML_LINE_RIGHT])
-            {
-                // Decrease right margin to not include gutter.
-                sal_Int32 nRightMargin{};
-                pMargins[XML_LINE_RIGHT]->maValue >>= nRightMargin;
-                nRightMargin -= nGutterMargin;
-                pMargins[XML_LINE_RIGHT]->maValue <<= nRightMargin;
-            }
-        }
-        else
-        {
-            if (nGutterMargin && pMargins[XML_LINE_LEFT])
-            {
-                // Decrease left margin to not include gutter.
-                sal_Int32 nLeftMargin{};
-                pMargins[XML_LINE_LEFT]->maValue >>= nLeftMargin;
-                nLeftMargin -= nGutterMargin;
-                pMargins[XML_LINE_LEFT]->maValue <<= nLeftMargin;
-            }
-        }
-    }
 }
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */