tdf#158703: return updated position from FnAddNonBrkSpace

The paragraph text may become shorter after the function succeeds,
because it may remove arbitrary number of preceding spaces; since
the position may then be used to access data in the string in the
caller, we need to update it, to avoid use of old position (which
may point beyond the string, and produce a crash; or it may point
to a wrong position in it).

Change-Id: Ib1b4b63cbd7150e0f69c97032e3410db7dadd4dc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160924
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
(cherry picked from commit 1325d66a7f8657aaf9951a0664fbf14cca86905b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160934
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160978

5 files changed, 49 insertions, 13 deletions

diff --git a/editeng/source/misc/svxacorr.cxx b/editeng/source/misc/svxacorr.cxx
index 318da6536df9..56228b2fbf05 100644
--- a/editeng/source/misc/svxacorr.cxx
+++ b/editeng/source/misc/svxacorr.cxx
@@ -669,12 +669,12 @@ bool SvxAutoCorrect::FnChgToEnEmDash(
 }
 
 // Add non-breaking space before specific punctuation marks in French text
-bool SvxAutoCorrect::FnAddNonBrkSpace(
+sal_Int32 SvxAutoCorrect::FnAddNonBrkSpace(
                                 SvxAutoCorrDoc& rDoc, std::u16string_view rTxt,
                                 sal_Int32 nEndPos,
                                 LanguageType eLang, bool& io_bNbspRunNext )
 {
-    bool bRet = false;
+    sal_Int32 nRet = -1;
 
     CharClass& rCC = GetCharClass( eLang );
 
@@ -708,7 +708,7 @@ bool SvxAutoCorrect::FnAddNonBrkSpace(
             if (nIndex + nProtocolLen <= rTxt.size())
             {
                 if (INetURLObject::CompareProtocolScheme(rTxt.substr(nIndex, nProtocolLen)) != INetProtocol::NotValid)
-                    return false;
+                    return -1;
             }
 
             // Check the presence of "://" in the word
@@ -736,7 +736,7 @@ bool SvxAutoCorrect::FnAddNonBrkSpace(
                     if ( bHasSpace )
                         rDoc.Insert( nPos, OUString(cNonBreakingSpace) );
                     io_bNbspRunNext = true;
-                    bRet = true;
+                    nRet = nPos;
                 }
                 else if ( chars.indexOf( cPrevChar ) != -1 )
                     io_bNbspRunNext = true;
@@ -750,12 +750,12 @@ bool SvxAutoCorrect::FnAddNonBrkSpace(
             if ( cPrevChar == ':' && cMaybeSpaceChar == cNonBreakingSpace )
             {
                 rDoc.Delete( nEndPos - 2, nEndPos - 1 );
-                bRet = true;
+                nRet = nEndPos - 1;
             }
         }
     }
 
-    return bRet;
+    return nRet;
 }
 
 // URL recognition
@@ -1502,10 +1502,14 @@ void SvxAutoCorrect::DoAutoCorrect( SvxAutoCorrDoc& rDoc, const OUString& rTxt,
             // Hardspaces autocorrection
             if ( IsAutoCorrFlag( ACFlags::AddNonBrkSpace ) )
             {
-                if ( NeedsHardspaceAutocorr( cChar ) &&
-                    FnAddNonBrkSpace( rDoc, rTxt, nInsPos, GetDocLanguage( rDoc, nInsPos ), io_bNbspRunNext ) )
+                // WARNING ATTENTION: rTxt is an alias of the text node's OUString
+                // and its length may change (even become shorter) if FnAddNonBrkSpace succeeds!
+                sal_Int32 nUpdatedPos = -1;
+                if (NeedsHardspaceAutocorr(cChar))
+                    nUpdatedPos = FnAddNonBrkSpace( rDoc, rTxt, nInsPos, GetDocLanguage( rDoc, nInsPos ), io_bNbspRunNext );
+                if (nUpdatedPos >= 0)
                 {
-                    ;
+                    nInsPos = nUpdatedPos;
                 }
                 else if ( bIsNextRun && !IsAutoCorrectChar( cChar ) )
                 {
diff --git a/include/editeng/svxacorr.hxx b/include/editeng/svxacorr.hxx
index a5e43032a78f..356bdb29fd7b 100644
--- a/include/editeng/svxacorr.hxx
+++ b/include/editeng/svxacorr.hxx
@@ -411,7 +411,9 @@ public:
     bool FnChgToEnEmDash( SvxAutoCorrDoc&, const OUString&,
                                 sal_Int32 nSttPos, sal_Int32 nEndPos,
                                 LanguageType eLang );
-    bool FnAddNonBrkSpace( SvxAutoCorrDoc&, std::u16string_view,
+    // Returns an updated position, at which the insertion/removal happened. It may be
+    // a smaller value, if leading spaces were removed. If unsuccessful, returns -1.
+    sal_Int32 FnAddNonBrkSpace( SvxAutoCorrDoc&, std::u16string_view,
                                 sal_Int32 nEndPos,
                                 LanguageType eLang, bool& io_bNbspRunNext );
     bool FnSetINetAttr( SvxAutoCorrDoc&, const OUString&,
diff --git a/sw/qa/extras/uiwriter/data/tdf158703.fodt b/sw/qa/extras/uiwriter/data/tdf158703.fodt
new file mode 100644
index 000000000000..5fc03d015f7e
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data/tdf158703.fodt
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:automatic-styles>
+  <style:style style:name="P1" style:family="paragraph">
+   <style:text-properties fo:language="fr" fo:country="FR"/>
+  </style:style>
+ </office:automatic-styles>
+ <office:body>
+  <office:text>
+   <text:p text:style-name="P1">Foo <text:s text:c="5"/></text:p>
+  </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/uiwriter/uiwriter8.cxx b/sw/qa/extras/uiwriter/uiwriter8.cxx
index 456852a85ecf..04655535c2d5 100644
--- a/sw/qa/extras/uiwriter/uiwriter8.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter8.cxx
@@ -2794,6 +2794,21 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf158459)
     CPPUNIT_ASSERT_EQUAL(OUString("abdf"), pTextNode->GetText());
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf158703)
+{
+    // Given a document with French text, consisting of a word and several spaces:
+    createSwDoc("tdf158703.fodt");
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+    CPPUNIT_ASSERT(pTextDoc);
+
+    dispatchCommand(mxComponent, ".uno:GoToEndOfDoc", {});
+
+    // Typing ":" after the spaces should start auto-correction, which is expected to
+    // remove the spaces, and insert an NBSP instead. It must not crash.
+    emulateTyping(*pTextDoc, u":");
+    CPPUNIT_ASSERT_EQUAL(OUString(u"Foo\u00A0:"), getParagraph(1)->getString());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/edit/autofmt.cxx b/sw/source/core/edit/autofmt.cxx
index 010e0b20b80e..7d68a66b9e14 100644
--- a/sw/source/core/edit/autofmt.cxx
+++ b/sw/source/core/edit/autofmt.cxx
@@ -2098,8 +2098,8 @@ void SwAutoFormat::AutoCorrect(TextFrameIndex nPos)
                         : LANGUAGE_SYSTEM;
 
                     SetRedlineText( STR_AUTOFMTREDL_NON_BREAK_SPACE );
-                    if (pATst->FnAddNonBrkSpace(aACorrDoc, *pText, sal_Int32(nPos), eLang, bNbspRunNext))
-                        --nPos;
+                    if (sal_Int32 nUpdatedPos = pATst->FnAddNonBrkSpace(aACorrDoc, *pText, sal_Int32(nPos), eLang, bNbspRunNext); nUpdatedPos >= 0)
+                        nPos = TextFrameIndex(nUpdatedPos);
                 }
                 break;
 
@@ -2176,7 +2176,8 @@ void SwAutoFormat::AutoCorrect(TextFrameIndex nPos)
             if ( m_aFlags.bAddNonBrkSpace && nPos < TextFrameIndex(pText->getLength()) )
             {
                 SetRedlineText( STR_AUTOFMTREDL_NON_BREAK_SPACE );
-                pATst->FnAddNonBrkSpace(aACorrDoc, *pText, sal_Int32(nPos), eLang, bNbspRunNext);
+                if (sal_Int32 nUpdatedPos = pATst->FnAddNonBrkSpace(aACorrDoc, *pText, sal_Int32(nPos), eLang, bNbspRunNext); nUpdatedPos >= 0)
+                    nPos = TextFrameIndex(nUpdatedPos);
             }
 
             if( ( m_aFlags.bChgOrdinalNumber &&