tdf#57113: store UTF-16 stringpool data after legacy 1-byte data

This allows to correctly store and read Unicode strings used in
password-protected libraries. The additional data stored after all
legacy data of the stringpool record (after a magic number to mark
its presence), and so is invisible for older versions of program:
this allows to keep the version of data and backward compatibility.
Of course, older versions will only see legacy data, with broken
Unicode strings; and password-protected libraries edited and saved
in older versions will not contain Unicode data.

read_uInt16s_ToOUString and write_uInt16s_FromOUString are used
for correct handling of UTF-16 strings on LE/BE systems.

Change-Id: I990bc27b5cc7d499e71c43d45b7f263af41911e7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86065
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

5 files changed, 133 insertions, 54 deletions

diff --git a/basic/source/classes/image.cxx b/basic/source/classes/image.cxx
index fc2cbf0b154c..959681636dcd 100644
--- a/basic/source/classes/image.cxx
+++ b/basic/source/classes/image.cxx
@@ -90,6 +90,27 @@ static void SbiCloseRecord( SvStream& r, sal_uInt64 nOff )
     r.Seek( nPos );
 }
 
+static constexpr sal_uInt32 nUnicodeDataMagicNumber = 0x556E6920; // "Uni " BE
+
+static bool GetToUnicodePoolData(SvStream& r, sal_uInt64 nLen, sal_uInt64 nNext)
+{
+    const auto nPos = r.Tell();
+    // Check space for legacy data, magic number and Unicode data
+    bool bResult = nPos + nLen + sizeof(sal_uInt32) + nLen * sizeof(sal_Unicode) <= nNext;
+    if (bResult)
+    {
+        r.SeekRel(nLen); // Skip legacy data
+        sal_uInt32 nMagic = 0;
+        r.ReadUInt32(nMagic);
+        if (nMagic != nUnicodeDataMagicNumber)
+        {
+            r.Seek(nPos); // return
+            bResult = false;
+        }
+    }
+    return bResult;
+}
+
 bool SbiImage::Load( SvStream& r, sal_uInt32& nVersion )
 {
 
@@ -189,6 +210,11 @@ bool SbiImage::Load( SvStream& r, sal_uInt32& nVersion )
                 break;
             case FileOffset::StringPool:
             {
+                // the data layout is: nCount of 32-bit offsets into both legacy 1-byte char stream
+                // and resulting char buffer (1:1 correspondence assumed; 16 of 32 bits used);
+                // 32-bit length N of following 1-byte char stream (16 bits used); N bytes of 1-byte
+                // char stream; then optional magic number and stream of N sal_Unicode characters.
+
                 if( bBadVer ) break;
                 //assuming an empty string with just the lead 32bit len indicator
                 const sal_uInt64 nMinStringSize = 4;
@@ -211,13 +237,21 @@ bool SbiImage::Load( SvStream& r, sal_uInt32& nVersion )
                     pStrings.reset(new sal_Unicode[ nLen ]);
                     nStringSize = static_cast<sal_uInt16>(nLen);
 
-                    std::unique_ptr<char[]> pByteStrings(new char[ nLen ]);
-                    r.ReadBytes(pByteStrings.get(), nStringSize);
-                    for( size_t j = 0; j < mvStringOffsets.size(); j++ )
+                    if (GetToUnicodePoolData(r, nLen, nNext))
                     {
-                        sal_uInt16 nOff2 = static_cast<sal_uInt16>(mvStringOffsets[ j ]);
-                        OUString aStr( pByteStrings.get() + nOff2, strlen(pByteStrings.get() + nOff2), eCharSet );
-                        memcpy( pStrings.get() + nOff2, aStr.getStr(), (aStr.getLength() + 1) * sizeof( sal_Unicode ) );
+                        OUString s = read_uInt16s_ToOUString(r, nLen);
+                        memcpy(pStrings.get(), s.getStr(), s.getLength() * sizeof(sal_Unicode));
+                    }
+                    else
+                    {
+                        std::unique_ptr<char[]> pByteStrings(new char[nLen]);
+                        r.ReadBytes(pByteStrings.get(), nLen);
+                        for (size_t j = 0; j < mvStringOffsets.size(); j++)
+                        {
+                            sal_uInt16 nOff2 = static_cast<sal_uInt16>(mvStringOffsets[j]);
+                            OUString aStr(pByteStrings.get() + nOff2, strlen(pByteStrings.get() + nOff2), eCharSet);
+                            memcpy(pStrings.get() + nOff2, aStr.getStr(), (aStr.getLength() + 1) * sizeof(sal_Unicode));
+                        }
                     }
                 }
                 break;
@@ -435,8 +469,14 @@ bool SbiImage::Save( SvStream& r, sal_uInt32 nVer )
         }
         r.WriteUInt32( nStringSize );
         r.WriteBytes(pByteStrings.get(), nStringSize);
-
         pByteStrings.reset();
+
+        // Now write magic number and store the same data in UTF-16; this is backward compatible:
+        // old readers will not read this data after having read legacy data, and will proceed
+        // straight to the end of the record. So no version restriction here.
+        r.WriteUInt32(nUnicodeDataMagicNumber);
+        write_uInt16s_FromOUString(r, OUString(pStrings.get(), nStringSize));
+
         SbiCloseRecord( r, nPos );
     }
     // User defined types
diff --git a/basic/source/inc/filefmt.hxx b/basic/source/inc/filefmt.hxx
index c4126907a671..eb1990087d9e 100644
--- a/basic/source/inc/filefmt.hxx
+++ b/basic/source/inc/filefmt.hxx
@@ -41,6 +41,8 @@ class SvStream;
 // Version 12: aoo#64377 increase code size that basic can handle
 //             tdf#75973 support user defined types B_USERTYPES in password protected macros
 // Version 13: tdf#94617 store methods nStart information greater than sal_Int16 limit
+//             tdf#57113 store UTF-16 strings after legacy 1-byte-encoded strings in pool (no
+//                       version number bump for backward compatibility; relies on magic number)
 //
 
 #define B_LEGACYVERSION 0x00000011
diff --git a/sc/CppunitTest_sc_macros_test.mk b/sc/CppunitTest_sc_macros_test.mk
index 90f4930e2016..e0852b653598 100644
--- a/sc/CppunitTest_sc_macros_test.mk
+++ b/sc/CppunitTest_sc_macros_test.mk
@@ -70,52 +70,7 @@ $(eval $(call gb_CppunitTest_use_api,sc_macros_test,\
 $(eval $(call gb_CppunitTest_use_ure,sc_macros_test))
 $(eval $(call gb_CppunitTest_use_vcl,sc_macros_test))
 
-$(eval $(call gb_CppunitTest_use_components,sc_macros_test,\
-    basic/util/sb \
-    comphelper/util/comphelp \
-    configmgr/source/configmgr \
-    dbaccess/util/dba \
-    emfio/emfio \
-    eventattacher/source/evtatt \
-    filter/source/config/cache/filterconfig1 \
-    filter/source/storagefilterdetect/storagefd \
-    forms/util/frm \
-    framework/util/fwk \
-    framework/util/fwl \
-    i18npool/source/search/i18nsearch \
-    i18npool/util/i18npool \
-    linguistic/source/lng \
-    oox/util/oox \
-    package/source/xstor/xstor \
-    package/util/package2 \
-    sax/source/expatwrap/expwrap \
-    scripting/source/basprov/basprov \
-    scripting/source/dlgprov/dlgprov \
-    scripting/source/vbaevents/vbaevents \
-    scripting/util/scriptframe \
-    sc/util/sc \
-    sc/util/scd \
-    sc/util/scfilt \
-    $(call gb_Helper_optional,SCRIPTING, \
-	    sc/util/vbaobj) \
-    sfx2/util/sfx \
-    sot/util/sot \
-    svl/source/fsstor/fsstorage \
-    svtools/util/svt \
-    svx/util/svx \
-    svx/util/svxcore \
-    toolkit/util/tk \
-    ucb/source/core/ucb1 \
-    ucb/source/ucp/file/ucpfile1 \
-    ucb/source/ucp/tdoc/ucptdoc1 \
-    unotools/util/utl \
-    unoxml/source/rdf/unordf \
-    unoxml/source/service/unoxml \
-    uui/util/uui \
-    vcl/vcl.common \
-    vbahelper/util/msforms \
-    xmloff/util/xo \
-))
+$(eval $(call gb_CppunitTest_use_rdb,sc_macros_test,services))
 
 $(eval $(call gb_CppunitTest_use_configuration,sc_macros_test))
 
diff --git a/sc/qa/extras/macros-test.cxx b/sc/qa/extras/macros-test.cxx
index 046a0d131b0c..f887a95f7223 100644
--- a/sc/qa/extras/macros-test.cxx
+++ b/sc/qa/extras/macros-test.cxx
@@ -11,12 +11,14 @@
 #include <test/unoapi_test.hxx>
 #include <osl/file.hxx>
 #include <sal/log.hxx>
-
+#include <unotools/tempfile.hxx>
 #include <vcl/svapp.hxx>
 
 #include <docsh.hxx>
 #include <document.hxx>
 
+#include <com/sun/star/script/XLibraryContainerPassword.hpp>
+
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
 
@@ -26,12 +28,15 @@ class ScMacrosTest : public UnoApiTest
 {
 public:
     ScMacrosTest();
+    void saveAndReload(css::uno::Reference<css::lang::XComponent>& xComponent,
+                       const OUString& rFilter);
 
     void testStarBasic();
     void testVba();
     void testMSP();
     void testPasswordProtectedStarBasic();
     void testRowColumn();
+    void testPasswordProtectedUnicodeString();
 
     CPPUNIT_TEST_SUITE(ScMacrosTest);
     CPPUNIT_TEST(testStarBasic);
@@ -39,10 +44,27 @@ public:
     CPPUNIT_TEST(testVba);
     CPPUNIT_TEST(testPasswordProtectedStarBasic);
     CPPUNIT_TEST(testRowColumn);
+    CPPUNIT_TEST(testPasswordProtectedUnicodeString);
 
     CPPUNIT_TEST_SUITE_END();
 };
 
+void ScMacrosTest::saveAndReload(css::uno::Reference<css::lang::XComponent>& xComponent,
+                                 const OUString& rFilter)
+{
+    utl::TempFile aTempFile;
+    aTempFile.EnableKillingFile();
+    css::uno::Sequence<css::beans::PropertyValue> aArgs(1);
+    aArgs[0].Name = "FilterName";
+    aArgs[0].Value <<= rFilter;
+    css::uno::Reference<css::frame::XStorable> xStorable(xComponent, css::uno::UNO_QUERY_THROW);
+    xStorable->storeAsURL(aTempFile.GetURL(), aArgs);
+    css::uno::Reference<css::util::XCloseable> xCloseable(xComponent, css::uno::UNO_QUERY_THROW);
+    xCloseable->close(true);
+
+    xComponent = loadFromDesktop(aTempFile.GetURL(), "com.sun.star.sheet.SpreadsheetDocument");
+}
+
 // I suppose you could say this test doesn't really belong here, OTOH
 // we need a full document to run the test ( it related originally to an
 // imported Excel VBA macro ) It's convenient and fast to unit test
@@ -371,6 +393,66 @@ void ScMacrosTest::testRowColumn()
     pDocSh->DoClose();
 }
 
+void ScMacrosTest::testPasswordProtectedUnicodeString()
+{
+    const OUString sCorrectString(u"English Русский 中文");
+    const OUString sMacroURL(
+        "vnd.sun.Star.script:Protected.Module1.TestUnicodeString?language=Basic&location=document");
+    const OUString sLibName("Protected");
+
+    OUString aFileName;
+    createFileURL("tdf57113.ods", aFileName);
+    auto xComponent = loadFromDesktop(aFileName, "com.sun.star.sheet.SpreadsheetDocument");
+    CPPUNIT_ASSERT(xComponent);
+
+    // Check that loading password-protected macro image correctly loads Unicode strings
+    {
+        Any aRet;
+        Sequence<sal_Int16> aOutParamIndex;
+        Sequence<Any> aOutParam;
+        Sequence<uno::Any> aParams;
+
+        SfxObjectShell::CallXScript(xComponent, sMacroURL, aParams, aRet, aOutParamIndex,
+                                    aOutParam);
+
+        OUString aReturnValue;
+        aRet >>= aReturnValue;
+        CPPUNIT_ASSERT_EQUAL(sCorrectString, aReturnValue);
+    }
+
+    // Unlock and load the library, to regenerate the image on save
+    css::uno::Reference<css::document::XEmbeddedScripts> xES(xComponent, UNO_QUERY_THROW);
+    css::uno::Reference<css::script::XLibraryContainer> xLC(xES->getBasicLibraries(),
+                                                            UNO_QUERY_THROW);
+    css::uno::Reference<css::script::XLibraryContainerPassword> xPasswd(xLC, UNO_QUERY_THROW);
+    CPPUNIT_ASSERT(xPasswd->isLibraryPasswordProtected(sLibName));
+    CPPUNIT_ASSERT(!xPasswd->isLibraryPasswordVerified(sLibName));
+    CPPUNIT_ASSERT(xPasswd->verifyLibraryPassword(sLibName, "password"));
+    xLC->loadLibrary(sLibName);
+    CPPUNIT_ASSERT(xLC->isLibraryLoaded(sLibName));
+
+    // Now check that saving stores Unicode data correctly in image's string pool
+    saveAndReload(xComponent, "calc8");
+    CPPUNIT_ASSERT(xComponent);
+
+    {
+        Any aRet;
+        Sequence<sal_Int16> aOutParamIndex;
+        Sequence<Any> aOutParam;
+        Sequence<uno::Any> aParams;
+
+        SfxObjectShell::CallXScript(xComponent, sMacroURL, aParams, aRet, aOutParamIndex,
+                                    aOutParam);
+
+        OUString aReturnValue;
+        aRet >>= aReturnValue;
+        CPPUNIT_ASSERT_EQUAL(sCorrectString, aReturnValue);
+    }
+
+    css::uno::Reference<css::util::XCloseable> xCloseable(xComponent, css::uno::UNO_QUERY_THROW);
+    xCloseable->close(true);
+}
+
 ScMacrosTest::ScMacrosTest()
       : UnoApiTest("/sc/qa/extras/testdocuments")
 {
diff --git a/sc/qa/extras/testdocuments/tdf57113.ods b/sc/qa/extras/testdocuments/tdf57113.ods
new file mode 100644
index 000000000000..9768e0de076e
--- /dev/null
+++ b/sc/qa/extras/testdocuments/tdf57113.ods