From 365de006d14c50ea7de82330f780d58c10518718 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 17 Aug 2026 14:13:51 +0200 Subject: [PATCH] tdf#173160 Maintain order of arguments to macro The arguments for a macro when invoked by the dispatch provider get converted from a set of PropertyValues to a sequence of Anys containing just the values. Since the commit below, the TransformItems step was changed to store the PropertyValues in an unordered_map which makes the order of the values unpredictable. If the dispatch provider is used to execute a macro then the order of the arguments becomes important because the macro has no other way to determine which argument represents which property. In order to maintain the previous behaviour, the dispatch method now reorders the arguments so that they have the same order as before. The previous order was determined by the order the properties were handled in TransformItems. commit e8d73d53073f1d953c3498bf0ee2bfef996eb4b0 Author: Mike Kaganski Date: Wed Dec 3 10:45:35 2025 +0500 Let TransformItems return comphelper::SequenceAsHashMap Simplifies its usage in all places where the result needs to be modified. Change-Id: I37f8d99827e0f1cb2b6e4f4f7735e26fb87f6703 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195162 Change-Id: Iacf64650842eb62c0a2bd266784aa9c7accf45bd Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209231 Reviewed-by: Neil Roberts Tested-by: Jenkins --- .../CppunitTest_scripting_scripthandler.mk | 43 ++++ scripting/Module_scripting.mk | 1 + .../scripthandler/ScriptHandlerTest.cxx | 217 ++++++++++++++++++ .../source/protocolhandler/scripthandler.cxx | 86 +++++-- 4 files changed, 327 insertions(+), 20 deletions(-) create mode 100644 scripting/CppunitTest_scripting_scripthandler.mk create mode 100644 scripting/qa/cppunit/scripthandler/ScriptHandlerTest.cxx diff --git a/scripting/CppunitTest_scripting_scripthandler.mk b/scripting/CppunitTest_scripting_scripthandler.mk new file mode 100644 index 000000000000..b1fcf4ea2b63 --- /dev/null +++ b/scripting/CppunitTest_scripting_scripthandler.mk @@ -0,0 +1,43 @@ +# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t; fill-column: 100 -*- +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# + +$(eval $(call gb_CppunitTest_CppunitTest,scripting_scripthandler)) + +$(eval $(call gb_CppunitTest_add_exception_objects,scripting_scripthandler, \ + scripting/qa/cppunit/scripthandler/ScriptHandlerTest \ +)) + +$(eval $(call gb_CppunitTest_use_libraries,scripting_scripthandler, \ + cppu \ + cppuhelper \ + sal \ + subsequenttest \ + test \ + unotest \ + utl \ + tl \ + vcl \ +)) + +$(eval $(call gb_CppunitTest_use_externals,scripting_scripthandler,\ + boost_headers \ + libxml2 \ +)) + +$(eval $(call gb_CppunitTest_use_api,scripting_scripthandler,\ + udkapi \ + offapi \ +)) + +$(eval $(call gb_CppunitTest_use_ure,scripting_scripthandler)) +$(eval $(call gb_CppunitTest_use_vcl,scripting_scripthandler)) +$(eval $(call gb_CppunitTest_use_rdb,scripting_scripthandler,services)) +$(eval $(call gb_CppunitTest_use_configuration,scripting_scripthandler)) + +# vim: set noet sw=4 ts=4: diff --git a/scripting/Module_scripting.mk b/scripting/Module_scripting.mk index 4d27b702c1f3..082948a5b182 100644 --- a/scripting/Module_scripting.mk +++ b/scripting/Module_scripting.mk @@ -51,6 +51,7 @@ $(eval $(call gb_Module_add_check_targets,scripting,\ $(if $(ENABLE_QUICKJS),CppunitTest_scripting_jsprov) \ $(if $(ENABLE_SCRIPTING_BEANSHELL),CppunitTest_scripting_beanshell) \ $(if $(DISABLE_PYTHON),,CppunitTest_scripting_pythonscript) \ + CppunitTest_scripting_scripthandler \ )) $(eval $(call gb_Module_add_subsequentcheck_targets,scripting,\ diff --git a/scripting/qa/cppunit/scripthandler/ScriptHandlerTest.cxx b/scripting/qa/cppunit/scripthandler/ScriptHandlerTest.cxx new file mode 100644 index 000000000000..4a4d900645fa --- /dev/null +++ b/scripting/qa/cppunit/scripthandler/ScriptHandlerTest.cxx @@ -0,0 +1,217 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +constexpr OUString DUMMY_PROVIDER_SERVICE + = u"com.sun.star.script.provider.ScriptProviderForDummy"_ustr; +constexpr OUString LANGUAGE_PROVIDER_SERVICE + = u"com.sun.star.script.provider.LanguageScriptProvider"_ustr; +constexpr OUString SCRIPT_URL + = u"vnd.sun.star.script:Whatever?language=Dummy&location=document"_ustr; + +// A Dummy script provider so that we can execute our own function instead of running a script +class DummyProvider + : public cppu::WeakImplHelper +{ +public: + // XScriptProvider + css::uno::Reference + SAL_CALL getScript(const OUString& sScriptURI) override; + + // XScript + css::uno::Any SAL_CALL invoke(const css::uno::Sequence& aParams, + css::uno::Sequence& aOutParamIndex, + css::uno::Sequence& aOutParam) override; + + // XServiceInfo + OUString SAL_CALL getImplementationName() override; + sal_Bool SAL_CALL supportsService(const OUString& sServiceName) override; + css::uno::Sequence SAL_CALL getSupportedServiceNames() override; + + // XSingleComponentFactory + css::uno::Reference SAL_CALL createInstanceWithContext( + const css::uno::Reference& xContext) override; + css::uno::Reference SAL_CALL createInstanceWithArgumentsAndContext( + const css::uno::Sequence& rArguments, + const css::uno::Reference& xContext) override; + + bool m_bInvoked = false; +}; + +css::uno::Reference + SAL_CALL DummyProvider::getScript(const OUString&) +{ + return this; +} + +css::uno::Any SAL_CALL DummyProvider::invoke(const css::uno::Sequence& aParams, + css::uno::Sequence&, + css::uno::Sequence&) +{ + CPPUNIT_ASSERT_GREATEREQUAL(sal_Int32(6), aParams.getLength()); + + // First argument should be the URL + OUString sUrl; + CPPUNIT_ASSERT(aParams[0] >>= sUrl); + CPPUNIT_ASSERT_EQUAL(SCRIPT_URL, sUrl); + + // Second argument should be an XInteractionHandler + css::uno::Reference xInteractionHandler; + CPPUNIT_ASSERT(aParams[1] >>= xInteractionHandler); + + // Third argument is the OpenNewView value + bool bOpenNewView = true; + CPPUNIT_ASSERT(aParams[2] >>= bOpenNewView); + CPPUNIT_ASSERT(!bOpenNewView); + + // Fourth argument is the frame name + OUString sFrameName; + CPPUNIT_ASSERT(aParams[3] >>= sFrameName); + CPPUNIT_ASSERT_EQUAL(u"_self"_ustr, sFrameName); + + // Fifth argument is the macro execution mode + sal_Int16 nMacroExecutionMode = 0; + CPPUNIT_ASSERT(aParams[4] >>= nMacroExecutionMode); + CPPUNIT_ASSERT_EQUAL(css::document::MacroExecMode::USE_CONFIG, nMacroExecutionMode); + + // Sixth argument is the update doc mode + sal_Int16 nUpdateDocMode = 0; + CPPUNIT_ASSERT(aParams[5] >>= nUpdateDocMode); + CPPUNIT_ASSERT_EQUAL(css::document::UpdateDocMode::ACCORDING_TO_CONFIG, nUpdateDocMode); + + m_bInvoked = true; + + return css::uno::Any(); +} + +OUString SAL_CALL DummyProvider::getImplementationName() +{ + return u"com.sun.star.test.ScriptProviderForDummy"_ustr; +} + +sal_Bool SAL_CALL DummyProvider::supportsService(const OUString& sServiceName) +{ + return sServiceName == DUMMY_PROVIDER_SERVICE || sServiceName == LANGUAGE_PROVIDER_SERVICE; +} + +css::uno::Sequence SAL_CALL DummyProvider::getSupportedServiceNames() +{ + return css::uno::Sequence{ DUMMY_PROVIDER_SERVICE, LANGUAGE_PROVIDER_SERVICE }; +} + +css::uno::Reference SAL_CALL +DummyProvider::createInstanceWithContext(const css::uno::Reference&) +{ + return static_cast(this); +} + +css::uno::Reference + SAL_CALL DummyProvider::createInstanceWithArgumentsAndContext( + const css::uno::Sequence&, + const css::uno::Reference&) +{ + return static_cast(this); +} + +class ScriptHandlerTest : public UnoApiTest +{ +public: + ScriptHandlerTest() + : UnoApiTest(u"/scripting/qa/extras"_ustr) + { + } + +private: + void testDispatchArguments(); + + CPPUNIT_TEST_SUITE(ScriptHandlerTest); + CPPUNIT_TEST(testDispatchArguments); + CPPUNIT_TEST_SUITE_END(); +}; + +void ScriptHandlerTest::testDispatchArguments() +{ + // Tests that the first argument to a macro when invoked via a dispatch URL is the URL itself. + // See tdf#173160. + rtl::Reference xDummyProvider = new DummyProvider; + + // Temporarily register our dummy provider with the service manager so that the master script + // provider can find it + css::uno::Reference xServiceSet(m_xFactory, css::uno::UNO_QUERY_THROW); + css::uno::Reference xService(xDummyProvider); + xServiceSet->insert(css::uno::Any(xService)); + comphelper::ScopeGuard g( + [&xServiceSet, &xService] { xServiceSet->remove(css::uno::Any(xService)); }); + + css::uno::Sequence aPropertyValues = { + comphelper::makePropertyValue(u"FileName"_ustr, SCRIPT_URL), + comphelper::makePropertyValue(u"InteractionHandler"_ustr, + css::uno::Reference()), + comphelper::makePropertyValue(u"OpenNewView"_ustr, false), + comphelper::makePropertyValue(u"FrameName"_ustr, u"_self"_ustr), + comphelper::makePropertyValue(u"MacroExecutionMode"_ustr, + css::document::MacroExecMode::USE_CONFIG), + comphelper::makePropertyValue(u"UpdateDocMode"_ustr, + css::document::UpdateDocMode::ACCORDING_TO_CONFIG), + comphelper::makePropertyValue(u"SynchronMode"_ustr, false), + }; + + loadFromURL(u"private:factory/swriter"_ustr); + + // Dispatch the .uno:Open command. We can’t use XDispatchHelper for this because it will set + // SynchronMode to true and there isn’t a synchronous handler for it. + + css::uno::Reference xModel(mxComponent, css::uno::UNO_QUERY_THROW); + css::uno::Reference xDispatchProvider( + xModel->getCurrentController(), css::uno::UNO_QUERY_THROW); + + css::uno::Reference xParser + = css::util::URLTransformer::create(m_xContext); + css::util::URL aURL; + aURL.Complete = ".uno:Open"; + xParser->parseStrict(aURL); + + css::uno::Reference xDispatch + = xDispatchProvider->queryDispatch(aURL, OUString(), 0); + CPPUNIT_ASSERT(xDispatch.is()); + + xDispatch->dispatch(aURL, aPropertyValues); + + Scheduler::ProcessEventsToIdle(); + + // Make sure the script was actually invoked + CPPUNIT_ASSERT(xDummyProvider->m_bInvoked); +} + +CPPUNIT_TEST_SUITE_REGISTRATION(ScriptHandlerTest); + +} // namespace + +CPPUNIT_PLUGIN_IMPLEMENT(); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/scripting/source/protocolhandler/scripthandler.cxx b/scripting/source/protocolhandler/scripthandler.cxx index becf01a89ca6..270935dbbec7 100644 --- a/scripting/source/protocolhandler/scripthandler.cxx +++ b/scripting/source/protocolhandler/scripthandler.cxx @@ -84,6 +84,71 @@ void SAL_CALL ScriptProtocolHandler::initialize( m_bInitialised = true; } +namespace +{ +Sequence lcl_filterAndOrderScriptArgs(const Sequence& rArgs) +{ + // Copy the Sequence to a local array so we can modify the order + Sequence aArgs = rArgs; + sal_Int32 nSize = aArgs.getLength(); + PropertyValue* pArgs = aArgs.getArray(); + + // For some reason the script gets passed only the values from the PropertyValue array and not + // the names so the only way for it to distinguish them is by the order. That means this order + // is set in stone. See tdf#173160. + static const std::array aPropertyOrder = { + u"URL"_ustr, u"InteractionHandler"_ustr, u"OpenNewView"_ustr, + u"FrameName"_ustr, u"MacroExecutionMode"_ustr, u"UpdateDocMode"_ustr, + }; + sal_Int32 nUsedArgs = 0; + + // Reorder the property values according to the preset order + for (const auto& sPropertyName : aPropertyOrder) + { + for (sal_Int32 i = nUsedArgs; i < nSize; ++i) + { + if (sPropertyName == pArgs[i].Name) + { + // Swap this property with the one in the place we want it to be + if (i != nUsedArgs) + std::swap(pArgs[i], pArgs[nUsedArgs]); + ++nUsedArgs; + break; + } + } + } + + // Sometimes we get a propertyval with name = "Referer" or "SynchronMode". These are not actual + // arguments to be passed to script, but flags describing the call, so ignore. Who thought that + // passing such "meta-arguments" mixed in with real arguments was a good idea? + while (nUsedArgs < nSize) + { + if (pArgs[nUsedArgs].Name == "Referer" || pArgs[nUsedArgs].Name == "SynchronMode") + { + // Move the last argument over the place of this one and then decrease the number of + // arguments that we’ll use so that the moved one will get ignored + if (nUsedArgs + 1 < nSize) + pArgs[nUsedArgs] = std::move(pArgs[nSize - 1]); + --nSize; + } + else + { + // Otherwise keep the argument + ++nUsedArgs; + } + } + + Sequence aValues(nUsedArgs); + Any* pValues = aValues.getArray(); + + // Move only the values into the final array + for (sal_Int32 i = 0; i < nUsedArgs; ++i) + pValues[i] = std::move(pArgs[i].Value); + + return aValues; +} +} + Reference< XDispatch > SAL_CALL ScriptProtocolHandler::queryDispatch( const URL& aURL, const OUString&, sal_Int32 ) { @@ -178,29 +243,10 @@ void SAL_CALL ScriptProtocolHandler::dispatchWithNotification( "ScriptProtocolHandler::dispatchWithNotification: validate xFunc - unable to obtain XScript interface" ); - Sequence< Any > inArgs; + Sequence< Any > inArgs = lcl_filterAndOrderScriptArgs(lArgs); Sequence< Any > outArgs; Sequence< sal_Int16 > outIndex; - if ( lArgs.hasElements() ) - { - int argCount = 0; - for ( const auto& rArg : lArgs ) - { - // Sometimes we get a propertyval with name = "Referer" or "SynchronMode". These - // are not actual arguments to be passed to script, but flags describing the - // call, so ignore. Who thought that passing such "meta-arguments" mixed in with - // real arguments was a good idea? - if ( (rArg.Name != "Referer" && - rArg.Name != "SynchronMode") || - rArg.Name.isEmpty() ) //TODO:??? - { - inArgs.realloc( ++argCount ); - inArgs.getArray()[ argCount - 1 ] = rArg.Value; - } - } - } - // attempt to protect the document against the script tampering with its Undo Context std::unique_ptr< ::framework::DocumentUndoGuard > pUndoGuard; if ( bIsDocumentScript )