xmloff/source/style/PageMasterImportPropMapper.cxx |  134 +++++++++++----------
 1 file changed, 71 insertions(+), 63 deletions(-)

New commits:
commit b0e574a8be2a080f5039e968f4f32ce4b8fa2b26
Author:     Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Fri Oct 29 17:23:02 2021 +0200
Commit:     Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Tue Nov 2 19:35:49 2021 +0100

    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 <qui...@gmail.com>

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: */

Reply via email to