summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Stahl <michael.stahl@allotropia.de>2021-06-30 13:26:59 +0200
committerMichael Stahl <michael.stahl@allotropia.de>2021-07-01 12:00:14 +0200
commit9622539a8fa1faf485abc0a17ce88d88ba017c14 (patch)
tree3650ce38415c5e07c49224a568e9c7c3c4a4ce50
parent9b120ad2773676c5445d4664e9c7007c575249a4 (diff)
sfx2: try to fix lifecycle of SfxOfficeDispatch
This can be created either from the global SfxApplication, or from a SfxViewFrame. Particularly in the latter case, the SfxDispatcher and SfxBindings members are owned by SfxViewFrame, so in case that is destroyed, the SfxOfficeDispatch must clear its pointers. It looks like the member pointers are checked before access already everywhere, so just listen at the SfxViewFrame. Change-Id: If08825734e94dd54e32cb77546684fd583c336ec Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118162 Tested-by: Michael Stahl <michael.stahl@allotropia.de> Reviewed-by: Michael Stahl <michael.stahl@allotropia.de> (cherry picked from commit 7cbd6d768d282077053c354254315f3dc89bf254) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118195 Tested-by: Jenkins (cherry picked from commit 06b84e45ffbe4f281c5d63da0b385c5faff2ef7b)
-rw-r--r--framework/qa/cppunit/data/empty.fodp2
-rw-r--r--framework/qa/cppunit/dispatchtest.cxx32
-rw-r--r--sfx2/inc/unoctitm.hxx7
-rw-r--r--sfx2/source/control/unoctitm.cxx21
4 files changed, 61 insertions, 1 deletions
diff --git a/framework/qa/cppunit/data/empty.fodp b/framework/qa/cppunit/data/empty.fodp
new file mode 100644
index 000000000000..3c2a4cf2cda5
--- /dev/null
+++ b/framework/qa/cppunit/data/empty.fodp
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.presentation"><office:body><office:presentation><draw:page/></office:presentation></office:body></office:document>
diff --git a/framework/qa/cppunit/dispatchtest.cxx b/framework/qa/cppunit/dispatchtest.cxx
index 0dd6b4cfc33d..2e2efec4693a 100644
--- a/framework/qa/cppunit/dispatchtest.cxx
+++ b/framework/qa/cppunit/dispatchtest.cxx
@@ -15,6 +15,7 @@
#include <com/sun/star/frame/DispatchHelper.hpp>
#include <com/sun/star/frame/XDispatchProviderInterceptor.hpp>
#include <com/sun/star/frame/XInterceptorInfo.hpp>
+#include <com/sun/star/util/URLTransformer.hpp>
#include <comphelper/processfactory.hxx>
#include <rtl/ref.hxx>
@@ -201,6 +202,37 @@ CPPUNIT_TEST_FIXTURE(DispatchTest, testInterception)
// This was 1: MyInterceptor::queryDispatch() was called for .uno:Italic.
CPPUNIT_ASSERT_EQUAL(0, pInterceptor->getUnexpected());
}
+
+constexpr OUStringLiteral DATA_DIRECTORY = u"/framework/qa/cppunit/data/";
+
+CPPUNIT_TEST_FIXTURE(DispatchTest, testSfxOfficeDispatchDispose)
+{
+ // this test doesn't work with a new document because of aURL.Main check in SfxBaseController::dispatch()
+ mxComponent = loadFromDesktop(m_directories.getURLFromSrc(DATA_DIRECTORY) + "empty.fodp",
+ "com.sun.star.presentation.PresentationDocument");
+ uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xModel.is());
+ uno::Reference<frame::XController> xController(xModel->getCurrentController());
+ CPPUNIT_ASSERT(xController.is());
+ uno::Reference<frame::XDispatchProvider> xFrame(xController->getFrame(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xFrame.is());
+
+ uno::Reference<util::XURLTransformer> xParser(util::URLTransformer::create(mxComponentContext));
+ util::URL url;
+ url.Complete = xModel->getURL() + "#dummy";
+ xParser->parseStrict(url);
+
+ uno::Reference<frame::XDispatch> xDisp(xFrame->queryDispatch(url, "", 0));
+ CPPUNIT_ASSERT(xDisp.is());
+
+ mxComponent->dispose();
+
+ util::URL urlSlot;
+ urlSlot.Complete = "slot:5598";
+ xParser->parseStrict(urlSlot);
+ // crashed with UAF
+ xDisp->dispatch(urlSlot, {});
+}
}
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sfx2/inc/unoctitm.hxx b/sfx2/inc/unoctitm.hxx
index c180150325cc..7f6bf815e1f9 100644
--- a/sfx2/inc/unoctitm.hxx
+++ b/sfx2/inc/unoctitm.hxx
@@ -25,6 +25,7 @@
#include <cppuhelper/interfacecontainer.hxx>
#include <cppuhelper/weakref.hxx>
+#include <svl/lstner.hxx>
#include <sfx2/ctrlitem.hxx>
#include <osl/mutex.hxx>
@@ -101,7 +102,9 @@ public:
SfxDispatcher* GetDispatcher_Impl();
};
-class SfxDispatchController_Impl : public SfxControllerItem
+class SfxDispatchController_Impl
+ : public SfxControllerItem
+ , public SfxListener
{
css::util::URL aDispatchURL;
SfxDispatcher* pDispatcher;
@@ -126,6 +129,8 @@ public:
const css::util::URL& rURL );
virtual ~SfxDispatchController_Impl() override;
+ virtual void Notify(SfxBroadcaster& rBC, const SfxHint& rHint) override;
+
static OUString getSlaveCommand( const css::util::URL& rURL );
void StateChanged( sal_uInt16 nSID, SfxItemState eState, const SfxPoolItem* pState, SfxSlotServer const * pServ );
diff --git a/sfx2/source/control/unoctitm.cxx b/sfx2/source/control/unoctitm.cxx
index 00b9c7188d67..ff1029a2b61f 100644
--- a/sfx2/source/control/unoctitm.cxx
+++ b/sfx2/source/control/unoctitm.cxx
@@ -320,6 +320,27 @@ SfxDispatchController_Impl::SfxDispatchController_Impl(
BindInternal_Impl( nSlot, pBindings );
pBindings->LEAVEREGISTRATIONS();
}
+ assert(pDispatcher);
+ assert(SfxApplication::Get()->GetAppDispatcher_Impl() == pDispatcher
+ || pDispatcher->GetFrame() != nullptr);
+ if (pDispatcher->GetFrame())
+ {
+ StartListening(*pDispatcher->GetFrame());
+ }
+ else
+ {
+ StartListening(*SfxApplication::Get());
+ }
+}
+
+void SfxDispatchController_Impl::Notify(SfxBroadcaster& rBC, SfxHint const& rHint)
+{
+ if (rHint.GetId() == SfxHintId::Dying)
+ { // both pBindings and pDispatcher are dead if SfxViewFrame is dead
+ pBindings = nullptr;
+ pDispatcher = nullptr;
+ EndListening(rBC);
+ }
}
SfxDispatchController_Impl::~SfxDispatchController_Impl()