basic/source/runtime/runtime.cxx                  |   10 +++++
 sc/qa/extras/testdocuments/ForEachInSelection.ods |binary
 sc/qa/extras/vba-macro-test.cxx                   |   39 ++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

New commits:
commit acd44070a5fae1bbeec063acc8a68463d914529d
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Feb 19 13:26:48 2023 +0300
Commit:     Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Mon Feb 20 07:52:03 2023 +0000

    tdf#153724: make sure to retrieve the variable value before checking the 
type
    
    Commit 5760c94b8847164f9a7a181f031c7c86643944af tried to avoid all cases
    which could set an error in SbiRuntime::PushForEach. To do that, it checked
    the type of xObjVar before trying to get an object from it, which otherwise
    could set an error.
    
    But the type of the contained value can be not known until it is retrieved
    (which can happen inside SbxValue::Get in a call to SbxValue::Broadcast with
    SfxHintId::BasicDataWanted). This happens e.g. when the container passed to
    'for each' is a call to some special function, like VBA's 'Selection'. Then
    SbxValue::GetFullType would return SbxEMPTY prior to SbxValue::Get.
    
    Let's make sure to call SbxValue::Get first (asking for a Variant, to avoid
    errors on type mismatch), and only then, check the actual result data type.
    
    Change-Id: Iaa697f38285505e50504ae09f9307fbd29e09a53
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147273
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147298
    Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>

diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 52efd4aea837..729bf652a21e 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -1170,7 +1170,15 @@ void SbiRuntime::PushForEach()
     pForStk = p;
 
     SbxVariableRef xObjVar = PopVar();
-    SbxBase* pObj = xObjVar && xObjVar->GetFullType() == SbxOBJECT ? 
xObjVar->GetObject() : nullptr;
+    SbxBase* pObj(nullptr);
+    if (xObjVar)
+    {
+        SbxValues v(SbxVARIANT);
+        // Here it may retrieve the value, and change the type from SbxEMPTY 
to SbxOBJECT
+        xObjVar->Get(v);
+        if (v.eType == SbxOBJECT)
+            pObj = v.pObj;
+    }
 
     if (SbxDimArray* pArray = dynamic_cast<SbxDimArray*>(pObj))
     {
diff --git a/sc/qa/extras/testdocuments/ForEachInSelection.ods 
b/sc/qa/extras/testdocuments/ForEachInSelection.ods
new file mode 100644
index 000000000000..7996c86eb953
Binary files /dev/null and b/sc/qa/extras/testdocuments/ForEachInSelection.ods 
differ
diff --git a/sc/qa/extras/vba-macro-test.cxx b/sc/qa/extras/vba-macro-test.cxx
index 261ee3943a0a..15dfe9c874c0 100644
--- a/sc/qa/extras/vba-macro-test.cxx
+++ b/sc/qa/extras/vba-macro-test.cxx
@@ -55,6 +55,7 @@ public:
     void testTdf131562();
     void testTdf107902();
     void testTdf90278();
+    void testForEachInSelection();
 
     CPPUNIT_TEST_SUITE(VBAMacroTest);
     CPPUNIT_TEST(testSimpleCopyAndPaste);
@@ -72,6 +73,7 @@ public:
     CPPUNIT_TEST(testTdf131562);
     CPPUNIT_TEST(testTdf107902);
     CPPUNIT_TEST(testTdf90278);
+    CPPUNIT_TEST(testForEachInSelection);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -760,6 +762,43 @@ void VBAMacroTest::testTdf90278()
     xCloseable->close(true);
 }
 
+void VBAMacroTest::testForEachInSelection()
+{
+    OUString aFileName;
+    createFileURL(u"ForEachInSelection.ods", aFileName);
+    mxComponent = loadFromDesktop(aFileName, 
"com.sun.star.sheet.SpreadsheetDocument");
+
+    SfxObjectShell* pFoundShell = 
SfxObjectShell::GetShellFromComponent(mxComponent);
+
+    CPPUNIT_ASSERT_MESSAGE("Failed to access document shell", pFoundShell);
+    ScDocShell* pDocSh = static_cast<ScDocShell*>(pFoundShell);
+    ScDocument& rDoc = pDocSh->GetDocument();
+
+    uno::Any aRet;
+    uno::Sequence<sal_Int16> aOutParamIndex;
+    uno::Sequence<uno::Any> aOutParam;
+    uno::Sequence<uno::Any> aParams;
+
+    CPPUNIT_ASSERT_EQUAL(OUString("foo"), rDoc.GetString(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_EQUAL(OUString("bar"), rDoc.GetString(ScAddress(0, 1, 0)));
+    CPPUNIT_ASSERT_EQUAL(OUString("baz"), rDoc.GetString(ScAddress(0, 2, 0)));
+
+    // tdf#153724: without the fix, this would fail with
+    // assertion failed
+    // - Expression: false
+    // - Unexpected dialog:  Error: BASIC runtime error.
+    // '13'
+    // Data type mismatch.
+    SfxObjectShell::CallXScript(mxComponent,
+                                
"vnd.sun.Star.script:Standard.Module1.TestForEachInSelection?"
+                                "language=Basic&location=document",
+                                aParams, aRet, aOutParamIndex, aOutParam);
+
+    CPPUNIT_ASSERT_EQUAL(OUString("oof"), rDoc.GetString(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_EQUAL(OUString("rab"), rDoc.GetString(ScAddress(0, 1, 0)));
+    CPPUNIT_ASSERT_EQUAL(OUString("zab"), rDoc.GetString(ScAddress(0, 2, 0)));
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(VBAMacroTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();

Reply via email to