commit 1ef9c47bdd357157979412c99e1412eeb4777e1f
author Mike Kaganski <mike.kaganski@collabora.com> Sun Feb 19 13:26:48 2023 +0300
committer Caolán McNamara <caolanm@redhat.com> Wed Feb 22 10:13:48 2023 +0000
tree 96ed25485c2b02f3a7d003b50ab7067595d58501
parent 89f87051ea5117ebded3787d8843fb56f60fd34c

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.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147297
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit bcd6333952d4d1fe2bbe1c08616f8210ee0d3f7e)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147283
Reviewed-by: Eike Rathke <erack@redhat.com>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

basic/source/runtime/runtime.cxx

diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 23e6580..d0f49c8 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -1173,7 +1173,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))
    {

sc/qa/extras/testdocuments/ForEachInSelection.ods

diff --git a/sc/qa/extras/testdocuments/ForEachInSelection.ods b/sc/qa/extras/testdocuments/ForEachInSelection.ods
new file mode 100644
index 0000000..7996c86
--- /dev/null
+++ b/sc/qa/extras/testdocuments/ForEachInSelection.ods
Binary files differ

sc/qa/extras/vba-macro-test.cxx

diff --git a/sc/qa/extras/vba-macro-test.cxx b/sc/qa/extras/vba-macro-test.cxx
index 5c4bb5b..486b400 100644
--- a/sc/qa/extras/vba-macro-test.cxx
+++ b/sc/qa/extras/vba-macro-test.cxx
@@ -65,6 +65,7 @@ public:
    void testTdf149531();
    void testTdf118247();
    void testTdf126457();
    void testForEachInSelection();

    CPPUNIT_TEST_SUITE(VBAMacroTest);
    CPPUNIT_TEST(testSimpleCopyAndPaste);
@@ -88,6 +89,7 @@ public:
    CPPUNIT_TEST(testTdf149531);
    CPPUNIT_TEST(testTdf118247);
    CPPUNIT_TEST(testTdf126457);
    CPPUNIT_TEST(testForEachInSelection);
    CPPUNIT_TEST_SUITE_END();
};

@@ -829,6 +831,34 @@ void VBAMacroTest::testTdf126457()

    pDocSh->DoClose();
}

void VBAMacroTest::testForEachInSelection()
{
    loadFromURL(u"ForEachInSelection.ods");
    SfxObjectShell* pFoundShell = SfxObjectShell::GetShellFromComponent(mxComponent);

    CPPUNIT_ASSERT_MESSAGE("Failed to access document shell", pFoundShell);
    ScDocShell* pDocSh = static_cast<ScDocShell*>(pFoundShell);
    ScDocument& rDoc = pDocSh->GetDocument();

    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.
    executeMacro("vnd.sun.Star.script:Standard.Module1.TestForEachInSelection?"
                 "language=Basic&location=document");

    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();