From c322abacbf48144b5db656afdfcf2f3ecbd6406d Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Fri, 29 Dec 2023 14:06:28 +1100 Subject: [PATCH 01/30] WW-5381 Introduce extension point for MethodAccessor --- .../xwork2/config/impl/DefaultConfiguration.java | 3 +++ .../providers/StrutsDefaultConfigurationProvider.java | 4 ---- .../xwork2/ognl/OgnlValueStackFactory.java | 11 +++++------ .../main/java/org/apache/struts2/StrutsConstants.java | 3 +++ .../struts2/config/StrutsBeanSelectionProvider.java | 2 ++ core/src/main/resources/struts-beans.xml | 2 +- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 842d59c242..ec8fe7e654 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -90,6 +90,7 @@ import com.opensymphony.xwork2.ognl.SecurityMemberAccess; import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor; import com.opensymphony.xwork2.ognl.accessor.RootAccessor; +import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor; import com.opensymphony.xwork2.util.OgnlTextParser; import com.opensymphony.xwork2.util.PatternMatcher; import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider; @@ -100,6 +101,7 @@ import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory; import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.util.reflection.ReflectionProvider; +import ognl.MethodAccessor; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -389,6 +391,7 @@ public static ContainerBuilder bootstrapFactories(ContainerBuilder builder) { .factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON) .factory(RootAccessor.class, CompoundRootAccessor.class, Scope.SINGLETON) + .factory(MethodAccessor.class, XWorkMethodAccessor.class, Scope.SINGLETON) .factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) .factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index bf537899a1..6e66dd7a37 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -47,7 +47,6 @@ import com.opensymphony.xwork2.ognl.accessor.XWorkIteratorPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.XWorkListPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor; -import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor; import com.opensymphony.xwork2.security.AcceptedPatternsChecker; import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker; import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; @@ -66,7 +65,6 @@ import com.opensymphony.xwork2.validator.DefaultValidatorFileParser; import com.opensymphony.xwork2.validator.ValidatorFactory; import com.opensymphony.xwork2.validator.ValidatorFileParser; -import ognl.MethodAccessor; import ognl.PropertyAccessor; import org.apache.struts2.dispatcher.HttpParameters; import org.apache.struts2.dispatcher.Parameter; @@ -142,8 +140,6 @@ public void register(ContainerBuilder builder, LocatableProperties props) throws .factory(PropertyAccessor.class, HttpParameters.class.getName(), HttpParametersPropertyAccessor.class, Scope.SINGLETON) .factory(PropertyAccessor.class, Parameter.class.getName(), ParameterPropertyAccessor.class, Scope.SINGLETON) - .factory(MethodAccessor.class, Object.class.getName(), XWorkMethodAccessor.class, Scope.SINGLETON) - .factory(NullHandler.class, Object.class.getName(), InstantiatingNullHandler.class, Scope.SINGLETON) .factory(ActionValidatorManager.class, AnnotationActionValidatorManager.class, Scope.SINGLETON) .factory(ActionValidatorManager.class, "no-annotations", DefaultActionValidatorManager.class, Scope.SINGLETON) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java index 088caa0010..b1d6ea4998 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java @@ -57,6 +57,11 @@ protected void setCompoundRootAccessor(RootAccessor compoundRootAccessor) { OgnlRuntime.setMethodAccessor(CompoundRoot.class, compoundRootAccessor); } + @Inject + protected void setMethodAccessor(MethodAccessor methodAccessor) { + OgnlRuntime.setMethodAccessor(Object.class, methodAccessor); + } + @Inject("system") protected void setTextProvider(TextProvider textProvider) { this.textProvider = textProvider; @@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException { OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name)); } - names = container.getInstanceNames(MethodAccessor.class); - for (String name : names) { - Class cls = Class.forName(name); - OgnlRuntime.setMethodAccessor(cls, container.getInstance(MethodAccessor.class, name)); - } - names = container.getInstanceNames(NullHandler.class); for (String name : names) { Class cls = Class.forName(name); diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index f39258613e..939b3bddb0 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -223,6 +223,9 @@ public final class StrutsConstants { /** Extension point for the Struts CompoundRootAccessor */ public static final String STRUTS_COMPOUND_ROOT_ACCESSOR = "struts.compoundRootAccessor"; + /** Extension point for the Struts MethodAccessor */ + public static final String STRUTS_METHOD_ACCESSOR = "struts.methodAccessor"; + /** The name of the xwork converter implementation */ public static final String STRUTS_XWORKCONVERTER = "struts.xworkConverter"; diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index 4895fc6f83..a52a67749f 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -61,6 +61,7 @@ import com.opensymphony.xwork2.util.reflection.ReflectionContextFactory; import com.opensymphony.xwork2.util.reflection.ReflectionProvider; import com.opensymphony.xwork2.validator.ActionValidatorManager; +import ognl.MethodAccessor; import org.apache.struts2.StrutsConstants; import org.apache.struts2.components.UrlRenderer; import org.apache.struts2.components.date.DateFormatter; @@ -389,6 +390,7 @@ public void register(ContainerBuilder builder, LocatableProperties props) { alias(FileManagerFactory.class, StrutsConstants.STRUTS_FILE_MANAGER_FACTORY, builder, props, Scope.SINGLETON); alias(RootAccessor.class, StrutsConstants.STRUTS_COMPOUND_ROOT_ACCESSOR, builder, props); + alias(MethodAccessor.class, StrutsConstants.STRUTS_METHOD_ACCESSOR, builder, props); alias(XWorkConverter.class, StrutsConstants.STRUTS_XWORKCONVERTER, builder, props); alias(CollectionConverter.class, StrutsConstants.STRUTS_CONVERTER_COLLECTION, builder, props); diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 506f9d1221..3343b63864 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -198,7 +198,7 @@ - Date: Sat, 30 Dec 2023 23:55:12 +1100 Subject: [PATCH 02/30] WW-5382 Fix StrutsInternalTestCase --- .../struts2/StrutsInternalTestCase.java | 21 ++++++++++--------- .../apache/struts2/config/SettingsTest.java | 2 +- core/src/test/resources/struts.properties | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java b/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java index 30616303fa..5aad829694 100644 --- a/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java +++ b/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java @@ -18,12 +18,14 @@ */ package org.apache.struts2; +import com.opensymphony.xwork2.ActionProxyFactory; import com.opensymphony.xwork2.XWorkTestCase; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.struts2.dispatcher.Dispatcher; import org.apache.struts2.dispatcher.PrepareOperations; import org.apache.struts2.util.StrutsTestCaseHelper; import org.apache.struts2.views.jsp.StrutsMockServletContext; + import java.util.HashMap; import java.util.Map; @@ -38,22 +40,22 @@ public abstract class StrutsInternalTestCase extends XWorkTestCase { /** * Sets up the configuration settings, XWork configuration, and * message resources - * + * * @throws java.lang.Exception */ @Override protected void setUp() throws Exception { - super.setUp(); PrepareOperations.clearDevModeOverride(); // Clear DevMode override every time (consistent ThreadLocal state for tests). initDispatcher(null); } - + protected Dispatcher initDispatcher(Map params) { servletContext = new StrutsMockServletContext(); dispatcher = StrutsTestCaseHelper.initDispatcher(servletContext, params); configurationManager = dispatcher.getConfigurationManager(); configuration = configurationManager.getConfiguration(); container = configuration.getContainer(); + actionProxyFactory = container.getInstance(ActionProxyFactory.class); container.inject(dispatcher); return dispatcher; } @@ -66,14 +68,13 @@ protected Dispatcher initDispatcher(Map params) { * @return instance of {@see Dispatcher} */ protected Dispatcher initDispatcherWithConfigs(String configs) { - Map params = new HashMap(); + Map params = new HashMap<>(); params.put("config", configs); return initDispatcher(params); } @Override protected void tearDown() throws Exception { - super.tearDown(); // maybe someone else already destroyed Dispatcher if (dispatcher != null && dispatcher.getConfigurationManager() != null) { dispatcher.cleanup(); @@ -83,14 +84,14 @@ protected void tearDown() throws Exception { } /** - * Compare if two objects are considered equal according to their fields as accessed + * Compare if two objects are considered equal according to their fields as accessed * via reflection. - * - * Utilizes {@link EqualsBuilder#reflectionEquals(java.lang.Object, java.lang.Object, boolean)} to perform + * + * Utilizes {@link EqualsBuilder#reflectionEquals(java.lang.Object, java.lang.Object, boolean)} to perform * the check, and compares transient fields as well. This may fail when run while a security manager is * active, due to a need to user reflection. - * - * + * + * * @param obj1 the first {@link Object} to compare against the other. * @param obj2 the second {@link Object} to compare against the other. * @return true if the objects are equal based on field comparisons by reflection, false otherwise. diff --git a/core/src/test/java/org/apache/struts2/config/SettingsTest.java b/core/src/test/java/org/apache/struts2/config/SettingsTest.java index c31a53adbd..90b824ae43 100644 --- a/core/src/test/java/org/apache/struts2/config/SettingsTest.java +++ b/core/src/test/java/org/apache/struts2/config/SettingsTest.java @@ -36,7 +36,7 @@ public void testSettings() { Settings settings = new DefaultSettings(); assertEquals("12345", settings.get(StrutsConstants.STRUTS_MULTIPART_MAXSIZE)); - assertEquals("\temp", settings.get(StrutsConstants.STRUTS_MULTIPART_SAVEDIR)); + assertEquals("\\temp", settings.get(StrutsConstants.STRUTS_MULTIPART_SAVEDIR)); assertEquals("test,org/apache/struts2/othertest", settings.get( StrutsConstants.STRUTS_CUSTOM_PROPERTIES)); assertEquals("testvalue", settings.get("testkey")); diff --git a/core/src/test/resources/struts.properties b/core/src/test/resources/struts.properties index baf7481564..7d905d214a 100644 --- a/core/src/test/resources/struts.properties +++ b/core/src/test/resources/struts.properties @@ -24,7 +24,7 @@ struts.i18n.encoding=ISO-8859-1 struts.locale=de_DE -struts.multipart.saveDir=\temp +struts.multipart.saveDir=\\temp struts.multipart.maxSize=12345 ### Load custom property files (does not override struts.properties!) From 15acd72d240f356685ab2238a0ab4e62130c84ae Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 31 Dec 2023 00:29:29 +1100 Subject: [PATCH 03/30] WW-5382 Fix stale injections in Dispatcher --- .../apache/struts2/dispatcher/Dispatcher.java | 47 ++++++++----------- .../struts2/dispatcher/MockDispatcher.java | 19 +++++++- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index e378633e27..7234dfe6c3 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -119,6 +119,8 @@ public class Dispatcher { */ private static final List dispatcherListeners = new CopyOnWriteArrayList<>(); + private Container injectedContainer; + /** * Store state of StrutsConstants.STRUTS_DEVMODE setting. */ @@ -331,6 +333,12 @@ public void setHandleException(String handleException) { this.handleException = Boolean.parseBoolean(handleException); } + @Inject(StrutsConstants.STRUTS_DISPATCHER_PARAMETERSWORKAROUND) + public void setDispatchersParametersWorkaround(String dispatchersParametersWorkaround) { + this.paramsWorkaroundEnabled = Boolean.parseBoolean(dispatchersParametersWorkaround) + || (servletContext != null && StringUtils.contains(servletContext.getServerInfo(), "WebLogic")); + } + public boolean isHandleException() { return handleException; } @@ -536,17 +544,6 @@ private Container init_PreloadConfiguration() { return getContainer(); } - private void init_CheckWebLogicWorkaround(Container container) { - // test whether param-access workaround needs to be enabled - if (servletContext != null && StringUtils.contains(servletContext.getServerInfo(), "WebLogic")) { - LOG.info("WebLogic server detected. Enabling Struts parameter access work-around."); - paramsWorkaroundEnabled = true; - } else { - paramsWorkaroundEnabled = "true".equals(container.getInstance(String.class, - StrutsConstants.STRUTS_DISPATCHER_PARAMETERSWORKAROUND)); - } - } - /** * Load configurations, including both XML and zero-configuration strategies, * and update optional settings, including whether to reload configurations and resource files. @@ -567,9 +564,7 @@ public void init() { init_AliasStandardObjects(); // [7] init_DeferredXmlConfigurations(); - Container container = init_PreloadConfiguration(); - container.inject(this); - init_CheckWebLogicWorkaround(container); + getContainer(); // Inject this instance if (!dispatcherListeners.isEmpty()) { for (DispatcherListener l : dispatcherListeners) { @@ -1068,22 +1063,18 @@ public ConfigurationManager getConfigurationManager() { * @return Our dependency injection container */ public Container getContainer() { - if (ContainerHolder.get() != null) { - return ContainerHolder.get(); - } - ConfigurationManager mgr = getConfigurationManager(); - if (mgr == null) { - throw new IllegalStateException("The configuration manager shouldn't be null"); - } else { - Configuration config = mgr.getConfiguration(); - if (config == null) { - throw new IllegalStateException("Unable to load configuration"); - } else { - Container container = config.getContainer(); - ContainerHolder.store(container); - return container; + if (ContainerHolder.get() == null) { + try { + ContainerHolder.store(getConfigurationManager().getConfiguration().getContainer()); + } catch (NullPointerException e) { + throw new IllegalStateException("ConfigurationManager and/or Configuration should not be null", e); } } + if (injectedContainer != ContainerHolder.get()) { + injectedContainer = ContainerHolder.get(); + injectedContainer.inject(this); + } + return ContainerHolder.get(); } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java index b10014995d..500e1b5418 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java @@ -19,14 +19,19 @@ package org.apache.struts2.dispatcher; import com.opensymphony.xwork2.config.ConfigurationManager; +import com.opensymphony.xwork2.inject.Container; import javax.servlet.ServletContext; -import java.util.HashMap; import java.util.Map; +/** + * We really shouldn't test with this class because it relies on changing the ConfigurationManager mid-lifecycle, + * but retaining the stale injections - this will prevent us from detecting exactly these kind of bugs in our tests... + */ public class MockDispatcher extends Dispatcher { private final ConfigurationManager copyConfigurationManager; + private boolean injectedOnce = false; public MockDispatcher(ServletContext servletContext, Map context, ConfigurationManager configurationManager) { super(servletContext, context); @@ -39,4 +44,16 @@ public void init() { ContainerHolder.clear(); this.configurationManager = copyConfigurationManager; } + + @Override + public Container getContainer() { + if (!injectedOnce) { + injectedOnce = true; + return super.getContainer(); + } + if (ContainerHolder.get() == null) { + ContainerHolder.store(getConfigurationManager().getConfiguration().getContainer()); + } + return ContainerHolder.get(); + } } From 2024d831772a8ce19cd915671a7d570762135ec9 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 31 Dec 2023 00:30:15 +1100 Subject: [PATCH 04/30] WW-5382 Fix stale bootstrap context on ActionContext --- .../xwork2/config/impl/DefaultConfiguration.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 4a6ee1373e..3c60af76c1 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -325,12 +325,8 @@ public Class type() { } protected ActionContext setContext(Container cont) { - ActionContext context = ActionContext.getContext(); - if (context == null) { - ValueStack vs = cont.getInstance(ValueStackFactory.class).createValueStack(); - context = ActionContext.of(vs.getContext()).bind(); - } - return context; + ValueStack vs = cont.getInstance(ValueStackFactory.class).createValueStack(); + return ActionContext.of(vs.getContext()).bind(); } protected Container createBootstrapContainer(List providers) { From 96618ebbdfa345d2d800eb09c8b1b22f53d56eb0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 1 Jan 2024 09:58:53 +0000 Subject: [PATCH 05/30] Bump org.apache.commons:commons-compress from 1.23.0 to 1.25.0 Bumps org.apache.commons:commons-compress from 1.23.0 to 1.25.0. --- updated-dependencies: - dependency-name: org.apache.commons:commons-compress dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f059cdb217..1389bc4a92 100644 --- a/pom.xml +++ b/pom.xml @@ -1047,7 +1047,7 @@ org.apache.commons commons-compress - 1.24.0 + 1.25.0 From 6f1e1222bcf2d5e899f03b1a5e23aef1eb250071 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 2 Jan 2024 01:46:28 +1100 Subject: [PATCH 06/30] WW-5382 Rework existing Dispatcher tests and base test classes --- .../xwork2/XWorkJUnit4TestCase.java | 4 - .../opensymphony/xwork2/XWorkTestCase.java | 4 - .../struts2/dispatcher/ContainerHolder.java | 12 +- .../apache/struts2/dispatcher/Dispatcher.java | 2 +- .../struts2/dispatcher/MockDispatcher.java | 59 --- .../struts2/util/StrutsTestCaseHelper.java | 24 +- .../struts2/StrutsInternalTestCase.java | 7 +- .../struts2/StrutsJUnit4InternalTestCase.java | 61 +++ .../struts2/dispatcher/DispatcherTest.java | 461 ++++++++---------- .../struts2/views/jsp/AbstractTagTest.java | 6 +- .../struts2/junit/StrutsJUnit4TestCase.java | 10 +- .../apache/struts2/junit/StrutsTestCase.java | 11 +- .../struts2/StrutsTestCasePortletTests.java | 24 +- .../apache/struts2/testng/StrutsTestCase.java | 12 +- 14 files changed, 313 insertions(+), 384 deletions(-) delete mode 100644 core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java create mode 100644 core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java diff --git a/core/src/main/java/com/opensymphony/xwork2/XWorkJUnit4TestCase.java b/core/src/main/java/com/opensymphony/xwork2/XWorkJUnit4TestCase.java index b22789cc23..fe39c0c2a8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/XWorkJUnit4TestCase.java +++ b/core/src/main/java/com/opensymphony/xwork2/XWorkJUnit4TestCase.java @@ -51,10 +51,6 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { XWorkTestCaseHelper.tearDown(configurationManager); - configurationManager = null; - configuration = null; - container = null; - actionProxyFactory = null; } protected void loadConfigurationProviders(ConfigurationProvider... providers) { diff --git a/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java b/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java index 941a9e37c3..dc6bb25083 100644 --- a/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java +++ b/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java @@ -64,10 +64,6 @@ protected void setUp() throws Exception { @Override protected void tearDown() throws Exception { XWorkTestCaseHelper.tearDown(configurationManager); - configurationManager = null; - configuration = null; - container = null; - actionProxyFactory = null; } protected void loadConfigurationProviders(ConfigurationProvider... providers) { diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java b/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java index 9565732ace..2d1c61657e 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java @@ -23,25 +23,25 @@ /** * Simple class to hold Container instance per thread to minimise number of attempts * to read configuration and build each time a new configuration. - * + *

* As ContainerHolder operates just per thread (which means per request) there is no need * to check if configuration changed during the same request. If changed between requests, * first call to store Container in ContainerHolder will be with the new configuration. */ class ContainerHolder { - private static ThreadLocal instance = new ThreadLocal<>(); + private static final ThreadLocal instance = new ThreadLocal<>(); - public static void store(Container instance) { - ContainerHolder.instance.set(instance); + public static void store(Container newInstance) { + instance.set(newInstance); } public static Container get() { - return ContainerHolder.instance.get(); + return instance.get(); } public static void clear() { - ContainerHolder.instance.remove(); + instance.remove(); } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 7234dfe6c3..a53398b775 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -367,7 +367,7 @@ public void cleanup() { } // clean up Dispatcher itself for this thread - instance.set(null); + instance.remove(); servletContext.setAttribute(StrutsStatics.SERVLET_DISPATCHER, null); // clean up DispatcherListeners diff --git a/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java deleted file mode 100644 index 500e1b5418..0000000000 --- a/core/src/main/java/org/apache/struts2/dispatcher/MockDispatcher.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.struts2.dispatcher; - -import com.opensymphony.xwork2.config.ConfigurationManager; -import com.opensymphony.xwork2.inject.Container; - -import javax.servlet.ServletContext; -import java.util.Map; - -/** - * We really shouldn't test with this class because it relies on changing the ConfigurationManager mid-lifecycle, - * but retaining the stale injections - this will prevent us from detecting exactly these kind of bugs in our tests... - */ -public class MockDispatcher extends Dispatcher { - - private final ConfigurationManager copyConfigurationManager; - private boolean injectedOnce = false; - - public MockDispatcher(ServletContext servletContext, Map context, ConfigurationManager configurationManager) { - super(servletContext, context); - this.copyConfigurationManager = configurationManager; - } - - @Override - public void init() { - super.init(); - ContainerHolder.clear(); - this.configurationManager = copyConfigurationManager; - } - - @Override - public Container getContainer() { - if (!injectedOnce) { - injectedOnce = true; - return super.getContainer(); - } - if (ContainerHolder.get() == null) { - ContainerHolder.store(getConfigurationManager().getConfiguration().getContainer()); - } - return ContainerHolder.get(); - } -} diff --git a/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java b/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java index 42e107f40f..0f9a1bdf90 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java @@ -28,19 +28,17 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyMap; + /** - * Generic test setup methods to be used with any unit testing framework. + * Generic test setup methods to be used with any unit testing framework. */ public class StrutsTestCaseHelper { - - public static Dispatcher initDispatcher(ServletContext ctx, Map params) { - if (params == null) { - params = new HashMap<>(); - } - Dispatcher du = new DispatcherWrapper(ctx, params); + + public static Dispatcher initDispatcher(ServletContext ctx, Map params) { + Dispatcher du = new DispatcherWrapper(ctx, params != null ? params : emptyMap()); du.init(); Dispatcher.setInstance(du); @@ -52,7 +50,15 @@ public static Dispatcher initDispatcher(ServletContext ctx, Map p return du; } - public static void tearDown() throws Exception { + public static void tearDown(Dispatcher dispatcher) { + if (dispatcher != null && dispatcher.getConfigurationManager() != null) { + dispatcher.cleanup(); + } + tearDown(); + } + + public static void tearDown() { + (new Dispatcher(null, null)).cleanUpAfterInit(); // Clear ContainerHolder Dispatcher.setInstance(null); ActionContext.clear(); } diff --git a/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java b/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java index 5aad829694..ade928efb0 100644 --- a/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java +++ b/core/src/test/java/org/apache/struts2/StrutsInternalTestCase.java @@ -75,12 +75,7 @@ protected Dispatcher initDispatcherWithConfigs(String configs) { @Override protected void tearDown() throws Exception { - // maybe someone else already destroyed Dispatcher - if (dispatcher != null && dispatcher.getConfigurationManager() != null) { - dispatcher.cleanup(); - dispatcher = null; - } - StrutsTestCaseHelper.tearDown(); + StrutsTestCaseHelper.tearDown(dispatcher); } /** diff --git a/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java b/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java new file mode 100644 index 0000000000..8d72753927 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2; + +import com.opensymphony.xwork2.ActionProxyFactory; +import com.opensymphony.xwork2.XWorkJUnit4TestCase; +import org.apache.struts2.dispatcher.Dispatcher; +import org.apache.struts2.util.StrutsTestCaseHelper; +import org.apache.struts2.views.jsp.StrutsMockServletContext; +import org.junit.After; +import org.junit.Before; + +import java.util.Map; + +public class StrutsJUnit4InternalTestCase extends XWorkJUnit4TestCase { + + protected StrutsMockServletContext servletContext; + protected Dispatcher dispatcher; + + @Override + @Before + public void setUp() throws Exception { + initDispatcher(); + } + + @Override + @After + public void tearDown() throws Exception { + StrutsTestCaseHelper.tearDown(dispatcher); + } + + protected void initDispatcher() { + initDispatcher(null); + } + + protected void initDispatcher(Map params) { + StrutsTestCaseHelper.tearDown(); + servletContext = new StrutsMockServletContext(); + dispatcher = StrutsTestCaseHelper.initDispatcher(servletContext, params); + configurationManager = dispatcher.getConfigurationManager(); + configuration = configurationManager.getConfiguration(); + container = configuration.getContainer(); + actionProxyFactory = container.getInstance(ActionProxyFactory.class); + } +} diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java index 73a526535d..267fb4e227 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java @@ -18,27 +18,29 @@ */ package org.apache.struts2.dispatcher; -import com.mockobjects.dynamic.C; -import com.mockobjects.dynamic.Mock; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.ObjectFactory; import com.opensymphony.xwork2.StubValueStack; -import com.opensymphony.xwork2.config.Configuration; +import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.ConfigurationManager; import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.InterceptorStackConfig; import com.opensymphony.xwork2.config.entities.PackageConfig; import com.opensymphony.xwork2.inject.Container; +import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.interceptor.Interceptor; import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.mock.MockActionProxy; +import com.opensymphony.xwork2.test.StubConfigurationProvider; +import com.opensymphony.xwork2.util.location.LocatableProperties; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsConstants; -import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.StrutsJUnit4InternalTestCase; import org.apache.struts2.dispatcher.mapper.ActionMapping; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.util.ObjectFactoryDestroyable; +import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; @@ -46,18 +48,35 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.util.Collections; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.HashSet; import java.util.Locale; import java.util.Map; import java.util.Set; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + /** * Test case for Dispatcher. */ -public class DispatcherTest extends StrutsInternalTestCase { +public class DispatcherTest extends StrutsJUnit4InternalTestCase { + @Test public void testDefaultResourceBundlePropertyLoaded() { LocalizedTextProvider localizedTextProvider = container.getInstance(LocalizedTextProvider.class); @@ -71,115 +90,107 @@ public void testDefaultResourceBundlePropertyLoaded() { "Error uploading: some error messages"); } + @Test public void testPrepareSetEncodingProperly() { HttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); - Dispatcher du = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - }}); - du.prepare(req, res); + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); + dispatcher.prepare(req, res); - assertEquals(req.getCharacterEncoding(), "utf-8"); - assertEquals(res.getCharacterEncoding(), "utf-8"); + assertEquals(req.getCharacterEncoding(), UTF_8.name()); + assertEquals(res.getCharacterEncoding(), UTF_8.name()); } + @Test public void testEncodingForXMLHttpRequest() { // given MockHttpServletRequest req = new MockHttpServletRequest(); req.addHeader("X-Requested-With", "XMLHttpRequest"); - req.setCharacterEncoding("UTF-8"); + req.setCharacterEncoding(UTF_8.name()); HttpServletResponse res = new MockHttpServletResponse(); - Dispatcher du = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "latin-2"); - }}); + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, StandardCharsets.ISO_8859_1.name())); // when - du.prepare(req, res); + dispatcher.prepare(req, res); // then - assertEquals(req.getCharacterEncoding(), "UTF-8"); - assertEquals(res.getCharacterEncoding(), "UTF-8"); + assertEquals(req.getCharacterEncoding(), UTF_8.name()); + assertEquals(res.getCharacterEncoding(), UTF_8.name()); } + @Test public void testSetEncodingIfDiffer() { // given - Mock mock = new Mock(HttpServletRequest.class); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); - mock.expectAndReturn("getHeader", "X-Requested-With", ""); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); - HttpServletRequest req = (HttpServletRequest) mock.proxy(); + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(req.getHeader("X-Requested-With")).thenReturn(""); HttpServletResponse res = new MockHttpServletResponse(); - Dispatcher du = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - }}); - + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); // when - du.prepare(req, res); + dispatcher.prepare(req, res); // then - - assertEquals(req.getCharacterEncoding(), "utf-8"); - assertEquals(res.getCharacterEncoding(), "utf-8"); - mock.verify(); + assertEquals(UTF_8.name(), req.getCharacterEncoding()); + assertEquals(UTF_8.name(), res.getCharacterEncoding()); } + @Test public void testPrepareSetEncodingPropertyWithMultipartRequest() { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); req.setContentType("multipart/form-data"); - Dispatcher du = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - }}); - du.prepare(req, res); + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); + dispatcher.prepare(req, res); - assertEquals("utf-8", req.getCharacterEncoding()); - assertEquals("utf-8", res.getCharacterEncoding()); + assertEquals(UTF_8.name(), req.getCharacterEncoding()); + assertEquals(UTF_8.name(), res.getCharacterEncoding()); } + @Test public void testPrepareMultipartRequest() throws Exception { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); req.setMethod("post"); req.setContentType("multipart/form-data; boundary=asdcvb345asd"); - Dispatcher du = initDispatcher(Collections.emptyMap()); - du.prepare(req, res); - HttpServletRequest wrapped = du.wrapRequest(req); - assertTrue(wrapped instanceof MultiPartRequestWrapper); + dispatcher.prepare(req, res); + + assertTrue(dispatcher.wrapRequest(req) instanceof MultiPartRequestWrapper); } + @Test public void testPrepareMultipartRequestAllAllowedCharacters() throws Exception { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); req.setMethod("post"); req.setContentType("multipart/form-data; boundary=01=23a.bC:D((e)d'z?p+o_r,e-"); - Dispatcher du = initDispatcher(Collections.emptyMap()); - du.prepare(req, res); - HttpServletRequest wrapped = du.wrapRequest(req); - assertTrue(wrapped instanceof MultiPartRequestWrapper); + dispatcher.prepare(req, res); + + assertTrue(dispatcher.wrapRequest(req) instanceof MultiPartRequestWrapper); } + @Test public void testPrepareMultipartRequestIllegalCharacter() throws Exception { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); req.setMethod("post"); req.setContentType("multipart/form-data; boundary=01=2;3a.bC:D((e)d'z?p+o_r,e-"); - Dispatcher du = initDispatcher(Collections.emptyMap()); - du.prepare(req, res); - HttpServletRequest wrapped = du.wrapRequest(req); - assertFalse(wrapped instanceof MultiPartRequestWrapper); + dispatcher.prepare(req, res); + + assertFalse(dispatcher.wrapRequest(req) instanceof MultiPartRequestWrapper); } + @Test public void testDispatcherListener() { final DispatcherListenerState state = new DispatcherListenerState(); @@ -198,39 +209,32 @@ public void dispatcherInitialized(Dispatcher du) { assertFalse(state.isDestroyed); assertFalse(state.isInitialized); - Dispatcher du = initDispatcher(new HashMap<>()); + dispatcher.init(); assertTrue(state.isInitialized); - du.cleanup(); + dispatcher.cleanup(); assertTrue(state.isDestroyed); } + @Test public void testConfigurationManager() { - Dispatcher du; - final InternalConfigurationManager configurationManager = new InternalConfigurationManager(Container.DEFAULT_NAME); - try { - du = new MockDispatcher(new MockServletContext(), new HashMap<>(), configurationManager); - du.init(); - Dispatcher.setInstance(du); + configurationManager = spy(new ConfigurationManager(Container.DEFAULT_NAME)); + dispatcher = spyDispatcherWithConfigurationManager(new Dispatcher(new MockServletContext(), emptyMap()), configurationManager); - assertFalse(configurationManager.destroyConfiguration); + dispatcher.init(); - du.cleanup(); + verify(configurationManager, never()).destroyConfiguration(); - assertTrue(configurationManager.destroyConfiguration); + dispatcher.cleanup(); - } finally { - Dispatcher.setInstance(null); - } + verify(configurationManager).destroyConfiguration(); } + @Test public void testInitLoadsDefaultConfig() { - Dispatcher du = new Dispatcher(new MockServletContext(), new HashMap<>()); - du.init(); - Configuration config = du.getConfigurationManager().getConfiguration(); - assertNotNull(config); + assertNotNull(configuration); Set expected = new HashSet<>(); expected.add("struts-default.xml"); expected.add("struts-beans.xml"); @@ -238,135 +242,113 @@ public void testInitLoadsDefaultConfig() { expected.add("struts-plugin.xml"); expected.add("struts.xml"); expected.add("struts-deferred.xml"); - assertEquals(expected, config.getLoadedFileNames()); - assertTrue(config.getPackageConfigs().size() > 0); - PackageConfig packageConfig = config.getPackageConfig("struts-default"); - assertTrue(packageConfig.getInterceptorConfigs().size() > 0); - assertTrue(packageConfig.getResultTypeConfigs().size() > 0); + assertEquals(expected, configuration.getLoadedFileNames()); + assertFalse(configuration.getPackageConfigs().isEmpty()); + PackageConfig packageConfig = configuration.getPackageConfig("struts-default"); + assertFalse(packageConfig.getInterceptorConfigs().isEmpty()); + assertFalse(packageConfig.getResultTypeConfigs().isEmpty()); } + @Test public void testObjectFactoryDestroy() { + dispatcher = spy(dispatcher); + Container spiedContainer = spy(container); + doReturn(spiedContainer).when(dispatcher).getContainer(); - ConfigurationManager cm = new ConfigurationManager(Container.DEFAULT_NAME); - Dispatcher du = new MockDispatcher(new MockServletContext(), new HashMap<>(), cm); - Mock mockConfiguration = new Mock(Configuration.class); - cm.setConfiguration((Configuration) mockConfiguration.proxy()); - - Mock mockContainer = new Mock(Container.class); - final InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory(); - destroyedObjectFactory.setContainer((Container) mockContainer.proxy()); - mockContainer.expectAndReturn("getInstance", C.args(C.eq(ObjectFactory.class)), destroyedObjectFactory); + InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory(); + doReturn(destroyedObjectFactory).when(spiedContainer).getInstance(ObjectFactory.class); - mockConfiguration.expectAndReturn("getContainer", mockContainer.proxy()); - mockConfiguration.expect("destroy"); - mockConfiguration.matchAndReturn("getPackageConfigs", new HashMap()); - - du.init(); assertFalse(destroyedObjectFactory.destroyed); - du.cleanup(); + dispatcher.cleanup(); assertTrue(destroyedObjectFactory.destroyed); - mockConfiguration.verify(); - mockContainer.verify(); } + @Test public void testInterceptorDestroy() { - Mock mockInterceptor = new Mock(Interceptor.class); - mockInterceptor.matchAndReturn("hashCode", 0); - mockInterceptor.expect("destroy"); - - InterceptorMapping interceptorMapping = new InterceptorMapping("test", (Interceptor) mockInterceptor.proxy()); - + Interceptor mockedInterceptor = mock(Interceptor.class); + InterceptorMapping interceptorMapping = new InterceptorMapping("test", mockedInterceptor); InterceptorStackConfig isc = new InterceptorStackConfig.Builder("test").addInterceptor(interceptorMapping).build(); - PackageConfig packageConfig = new PackageConfig.Builder("test").addInterceptorStackConfig(isc).build(); - Map packageConfigs = new HashMap<>(); - packageConfigs.put("test", packageConfig); + configurationManager = spy(new ConfigurationManager(Container.DEFAULT_NAME)); + dispatcher = spyDispatcherWithConfigurationManager(new Dispatcher(new MockServletContext(), emptyMap()), configurationManager); - Mock mockContainer = new Mock(Container.class); - mockContainer.matchAndReturn("getInstance", C.args(C.eq(ObjectFactory.class)), new ObjectFactory()); - - Mock mockConfiguration = new Mock(Configuration.class); - mockConfiguration.matchAndReturn("getPackageConfigs", packageConfigs); - mockConfiguration.matchAndReturn("getContainer", mockContainer.proxy()); - mockConfiguration.expect("destroy"); + dispatcher.init(); - ConfigurationManager configurationManager = new ConfigurationManager(Container.DEFAULT_NAME); - configurationManager.setConfiguration((Configuration) mockConfiguration.proxy()); + configuration = spy(configurationManager.getConfiguration()); + configurationManager.setConfiguration(configuration); + when(configuration.getPackageConfigs()).thenReturn(singletonMap("test", packageConfig)); - Dispatcher dispatcher = new MockDispatcher(new MockServletContext(), new HashMap<>(), configurationManager); - dispatcher.init(); dispatcher.cleanup(); - mockInterceptor.verify(); - mockContainer.verify(); - mockConfiguration.verify(); + verify(mockedInterceptor).destroy(); + verify(configuration).destroy(); } + @Test public void testMultipartSupportEnabledByDefault() { HttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); - Dispatcher du = initDispatcher(Collections.emptyMap()); - du.prepare(req, res); + dispatcher.prepare(req, res); - assertTrue(du.isMultipartSupportEnabled(req)); + assertTrue(dispatcher.isMultipartSupportEnabled(req)); } + @Test public void testIsMultipartRequest() { MockHttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); req.setMethod("POST"); - Dispatcher du = initDispatcher(Collections.emptyMap()); - du.prepare(req, res); + + dispatcher.prepare(req, res); req.setContentType("multipart/form-data"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263;charset=UTF-8"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263;charset=ISO-8859-1"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263;charset=Windows-1250"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263;charset=US-ASCII"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data; boundary=---------------------------207103069210263;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data;boundary=---------------------------207103069210263;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data;boundary=---------------------------207103069210263; charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data;boundary=---------------------------207103069210263 ;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data;boundary=---------------------------207103069210263 ; charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data ;boundary=---------------------------207103069210263;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("multipart/form-data ; boundary=---------------------------207103069210263;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); req.setContentType("Multipart/Form-Data ; boundary=---------------------------207103069210263;charset=UTF-16LE"); - assertTrue(du.isMultipartRequest(req)); + assertTrue(dispatcher.isMultipartRequest(req)); } + @Test public void testServiceActionResumePreviousProxy() throws Exception { - Dispatcher du = initDispatcher(Collections.emptyMap()); - MockActionInvocation mai = new MockActionInvocation(); ActionContext.getContext().withActionInvocation(mai); @@ -381,17 +363,15 @@ public void testServiceActionResumePreviousProxy() throws Exception { assertFalse(actionProxy.isExecutedCalled()); - du.setDevMode("false"); - du.setHandleException("false"); - du.serviceAction(req, null, new ActionMapping()); + dispatcher.setDevMode("false"); + dispatcher.setHandleException("false"); + dispatcher.serviceAction(req, null, new ActionMapping()); assertTrue("should execute previous proxy", actionProxy.isExecutedCalled()); } + @Test public void testServiceActionCreatesNewProxyIfDifferentMapping() throws Exception { - Dispatcher du = initDispatcher(Collections.emptyMap()); - container.inject(du); - MockActionInvocation mai = new MockActionInvocation(); ActionContext.getContext().withActionInvocation(mai); @@ -412,7 +392,7 @@ public void testServiceActionCreatesNewProxyIfDifferentMapping() throws Exceptio ActionMapping newActionMapping = new ActionMapping(); newActionMapping.setName("hello"); - du.serviceAction(request, response, newActionMapping); + dispatcher.serviceAction(request, response, newActionMapping); assertFalse(previousActionProxy.isExecutedCalled()); } @@ -421,196 +401,178 @@ public void testServiceActionCreatesNewProxyIfDifferentMapping() throws Exceptio * Verify proper default (true) handleExceptionState for Dispatcher and that * it properly reflects a manually configured change to false. */ + @Test public void testHandleException() { - Dispatcher du = initDispatcher(new HashMap<>()); - assertTrue("Default Dispatcher handleException state not true ?", du.isHandleException()); + assertTrue("Default Dispatcher handleException state not true ?", dispatcher.isHandleException()); - Dispatcher du2 = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_HANDLE_EXCEPTION, "false"); - }}); - assertFalse("Modified Dispatcher handleException state not false ?", du2.isHandleException()); + initDispatcher(singletonMap(StrutsConstants.STRUTS_HANDLE_EXCEPTION, "false")); + assertFalse("Modified Dispatcher handleException state not false ?", dispatcher.isHandleException()); } /** * Verify proper default (false) devMode for Dispatcher and that * it properly reflects a manually configured change to true. */ + @Test public void testDevMode() { - Dispatcher du = initDispatcher(new HashMap<>()); - assertFalse("Default Dispatcher devMode state not false ?", du.isDevMode()); + assertFalse("Default Dispatcher devMode state not false ?", dispatcher.isDevMode()); - Dispatcher du2 = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_DEVMODE, "true"); - }}); - assertTrue("Modified Dispatcher devMode state not true ?", du2.isDevMode()); + initDispatcher(singletonMap(StrutsConstants.STRUTS_DEVMODE, "true")); + assertTrue("Modified Dispatcher devMode state not true ?", dispatcher.isDevMode()); } + @Test public void testGetLocale_With_DefaultLocale_FromConfiguration() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - // Not setting a Struts Locale here, so we should receive the default "de_DE" from the test configuration. - }}); + // Not setting a Struts Locale here, so we should receive the default "de_DE" from the test configuration. + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.GERMANY, context.getLocale()); // Expect the Dispatcher defaultLocale value "de_DE" from the test configuration. - mock.verify(); } + @Test public void testGetLocale_With_DefaultLocale_fr_CA() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); + initDispatcher(new HashMap() {{ + put(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name()); put(StrutsConstants.STRUTS_LOCALE, Locale.CANADA_FRENCH.toString()); // Set the Dispatcher defaultLocale to fr_CA. }}); // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.CANADA_FRENCH, context.getLocale()); // Expect the Dispatcher defaultLocale value. - mock.verify(); } + @Test public void testGetLocale_With_BadDefaultLocale_RequestLocale_en_UK() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getLocale", Locale.UK); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - mock.expectAndReturn("getLocale", Locale.UK); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); + when(request.getLocale()).thenReturn(Locale.UK); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); + initDispatcher(new HashMap() {{ + put(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name()); put(StrutsConstants.STRUTS_LOCALE, "This_is_not_a_valid_Locale_string"); // Set Dispatcher defaultLocale to an invalid value. }}); // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.UK, context.getLocale()); // Expect the request set value from Mock. - mock.verify(); } + @Test public void testGetLocale_With_BadDefaultLocale_And_RuntimeException() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getLocale", Locale.UK); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - mock.expectAndThrow("getLocale", new IllegalStateException("Test theoretical state preventing HTTP Request Locale access")); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); + when(request.getLocale()).thenReturn(Locale.UK); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); + initDispatcher(new HashMap() {{ + put(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name()); put(StrutsConstants.STRUTS_LOCALE, "This_is_not_a_valid_Locale_string"); // Set the Dispatcher defaultLocale to an invalid value. }}); // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + when(request.getLocale()).thenThrow(new IllegalStateException("Test theoretical state preventing HTTP Request Locale access")); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.getDefault(), context.getLocale()); // Expect the system default value, when BOTH Dispatcher default Locale AND request access fail. - mock.verify(); } + @Test public void testGetLocale_With_NullDefaultLocale() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getLocale", Locale.CANADA_FRENCH); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - mock.expectAndReturn("getLocale", Locale.CANADA_FRENCH); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); + when(request.getLocale()).thenReturn(Locale.CANADA_FRENCH); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - // Attempting to set StrutsConstants.STRUTS_LOCALE to null here via parameters causes an NPE. - }}); + // Attempting to set StrutsConstants.STRUTS_LOCALE to null here via parameters causes an NPE. + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); - testDispatcher.setDefaultLocale(null); // Force a null Struts default locale, otherwise we receive the default "de_DE" from the test configuration. + dispatcher.setDefaultLocale(null); // Force a null Struts default locale, otherwise we receive the default "de_DE" from the test configuration. // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.CANADA_FRENCH, context.getLocale()); // Expect the request set value from Mock. - mock.verify(); } + @Test public void testGetLocale_With_NullDefaultLocale_And_RuntimeException() { // Given - Mock mock = new Mock(HttpServletRequest.class); + HttpServletRequest request = mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); - mock.expectAndReturn("getCharacterEncoding", "utf-8"); // From Dispatcher prepare(). - mock.expectAndReturn("getHeader", "X-Requested-With", ""); // From Dispatcher prepare(). - mock.expectAndReturn("getLocale", Locale.CANADA_FRENCH); // From Dispatcher prepare(). - mock.expectAndReturn("getParameterMap", new HashMap()); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", false, mockHttpSession); // From Dispatcher prepare(). - mock.expectAndReturn("getSession", true, mockHttpSession); // From createTestContextMap(). - mock.expectAndThrow("getLocale", new IllegalStateException("Test some theoretical state preventing HTTP Request Locale access")); // From createTestContextMap(). - HttpServletRequest request = (HttpServletRequest) mock.proxy(); + when(request.getCharacterEncoding()).thenReturn(UTF_8.name()); + when(request.getHeader("X-Requested-With")).thenReturn(""); + when(request.getParameterMap()).thenReturn(emptyMap()); + when(request.getSession(anyBoolean())).thenReturn(mockHttpSession); + when(request.getLocale()).thenReturn(Locale.CANADA_FRENCH); HttpServletResponse response = new MockHttpServletResponse(); - Dispatcher testDispatcher = initDispatcher(new HashMap() {{ - put(StrutsConstants.STRUTS_I18N_ENCODING, "utf-8"); - // Attempting to set StrutsConstants.STRUTS_LOCALE to null via parameters causes an NPE. - }}); + // Attempting to set StrutsConstants.STRUTS_LOCALE to null via parameters causes an NPE. + initDispatcher(singletonMap(StrutsConstants.STRUTS_I18N_ENCODING, UTF_8.name())); - testDispatcher.setDefaultLocale(null); // Force a null Struts default locale, otherwise we receive the default "de_DE" from the test configuration. + dispatcher.setDefaultLocale(null); // Force a null Struts default locale, otherwise we receive the default "de_DE" from the test configuration. // When - testDispatcher.prepare(request, response); - ActionContext context = ActionContext.of(createTestContextMap(testDispatcher, request, response)); + dispatcher.prepare(request, response); + when(request.getLocale()).thenThrow(new IllegalStateException("Test theoretical state preventing HTTP Request Locale access")); + ActionContext context = ActionContext.of(createTestContextMap(dispatcher, request, response)); // Then assertEquals(Locale.getDefault(), context.getLocale()); // Expect the system default value when Mock request access fails. - mock.verify(); + } + + public static Dispatcher spyDispatcherWithConfigurationManager(Dispatcher dispatcher, ConfigurationManager configurationManager) { + Dispatcher spiedDispatcher = spy(dispatcher); + doReturn(configurationManager).when(spiedDispatcher).createConfigurationManager(any()); + return spiedDispatcher; } /** @@ -641,21 +603,6 @@ protected static Map createTestContextMap(Dispatcher dispatcher, response); } - static class InternalConfigurationManager extends ConfigurationManager { - public boolean destroyConfiguration = false; - - public InternalConfigurationManager(String name) { - super(name); - } - - @Override - public synchronized void destroyConfiguration() { - super.destroyConfiguration(); - destroyConfiguration = true; - } - } - - static class DispatcherListenerState { public boolean isInitialized = false; public boolean isDestroyed = false; diff --git a/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java index c1b8188fa9..59afa3a1ce 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/AbstractTagTest.java @@ -34,7 +34,6 @@ import org.apache.struts2.dispatcher.ApplicationMap; import org.apache.struts2.dispatcher.Dispatcher; import org.apache.struts2.dispatcher.HttpParameters; -import org.apache.struts2.dispatcher.MockDispatcher; import org.apache.struts2.dispatcher.RequestMap; import org.apache.struts2.dispatcher.SessionMap; @@ -42,9 +41,10 @@ import javax.servlet.jsp.JspWriter; import java.io.File; import java.io.StringWriter; -import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyMap; + /** * Base class to extend for unit testing UI Tags. */ @@ -111,7 +111,7 @@ protected void createMocks() { pageContext.setServletContext(servletContext); mockContainer = new Mock(Container.class); - MockDispatcher du = new MockDispatcher(pageContext.getServletContext(), new HashMap<>(), configurationManager); + Dispatcher du = new Dispatcher(pageContext.getServletContext(), emptyMap()); du.init(); Dispatcher.setInstance(du); session = new SessionMap(request); diff --git a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java index 84b76a1b61..ee486d6cc9 100644 --- a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java +++ b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java @@ -203,9 +203,9 @@ public void finishExecution() { * Sets up the configuration settings, XWork configuration, and * message resources */ + @Override @Before public void setUp() throws Exception { - super.setUp(); initServletMockObjects(); setupBeforeInitDispatcher(); initDispatcherParams(); @@ -239,14 +239,10 @@ protected String getConfigPath() { return null; } + @Override @After public void tearDown() throws Exception { - super.tearDown(); - if (dispatcher != null && dispatcher.getConfigurationManager() != null) { - dispatcher.cleanup(); - dispatcher = null; - } - StrutsTestCaseHelper.tearDown(); + StrutsTestCaseHelper.tearDown(dispatcher); } } diff --git a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java index f64a9966f1..6280de8a81 100644 --- a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java +++ b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java @@ -168,8 +168,8 @@ protected void injectStrutsDependencies(Object object) { * Sets up the configuration settings, XWork configuration, and * message resources */ + @Override protected void setUp() throws Exception { - super.setUp(); initServletMockObjects(); setupBeforeInitDispatcher(); dispatcher = initDispatcher(dispatcherInitParams); @@ -199,14 +199,9 @@ protected Dispatcher initDispatcher(Map params) { return du; } + @Override protected void tearDown() throws Exception { - super.tearDown(); - // maybe someone else already destroyed Dispatcher - if (dispatcher != null && dispatcher.getConfigurationManager() != null) { - dispatcher.cleanup(); - dispatcher = null; - } - StrutsTestCaseHelper.tearDown(); + StrutsTestCaseHelper.tearDown(dispatcher); } } diff --git a/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java b/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java index acea0bec3e..0da8d1043c 100644 --- a/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java +++ b/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java @@ -23,12 +23,6 @@ import com.opensymphony.xwork2.ActionProxyFactory; import com.opensymphony.xwork2.XWorkTestCase; import com.opensymphony.xwork2.config.Configuration; -import java.io.UnsupportedEncodingException; -import java.util.HashMap; -import java.util.Map; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import org.apache.struts2.dispatcher.Dispatcher; import org.apache.struts2.dispatcher.HttpParameters; import org.apache.struts2.dispatcher.mapper.ActionMapper; @@ -41,6 +35,13 @@ import org.springframework.mock.web.MockPageContext; import org.springframework.mock.web.MockServletContext; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.UnsupportedEncodingException; +import java.util.HashMap; +import java.util.Map; + /* * Changes: This is a copy of org.apache.struts2.StrutsTestCase from the Struts 2 junit-plugin, kept in * in the same package org.apache.struts2 and renamed. Removed some unused imports, made @@ -180,8 +181,8 @@ protected void injectStrutsDependencies(Object object) { * Sets up the configuration settings, XWork configuration, and * message resources */ + @Override protected void setUp() throws Exception { - super.setUp(); initServletMockObjects(); setupBeforeInitDispatcher(); dispatcher = initDispatcher(dispatcherInitParams); @@ -211,14 +212,9 @@ protected Dispatcher initDispatcher(Map params) { return du; } + @Override protected void tearDown() throws Exception { - super.tearDown(); - // maybe someone else already destroyed Dispatcher - if (dispatcher != null && dispatcher.getConfigurationManager() != null) { - dispatcher.cleanup(); - dispatcher = null; - } - StrutsTestCaseHelper.tearDown(); + StrutsTestCaseHelper.tearDown(dispatcher); } } diff --git a/plugins/testng/src/main/java/org/apache/struts2/testng/StrutsTestCase.java b/plugins/testng/src/main/java/org/apache/struts2/testng/StrutsTestCase.java index 630eb2df69..6df81ff667 100644 --- a/plugins/testng/src/main/java/org/apache/struts2/testng/StrutsTestCase.java +++ b/plugins/testng/src/main/java/org/apache/struts2/testng/StrutsTestCase.java @@ -18,13 +18,13 @@ */ package org.apache.struts2.testng; -import java.util.Map; - import org.apache.struts2.dispatcher.Dispatcher; import org.apache.struts2.util.StrutsTestCaseHelper; +import org.springframework.mock.web.MockServletContext; import org.testng.annotations.AfterTest; import org.testng.annotations.BeforeTest; -import org.springframework.mock.web.MockServletContext; + +import java.util.Map; /** * Base test class for TestNG unit tests. Provides common Struts variables @@ -33,12 +33,12 @@ public class StrutsTestCase extends TestNGXWorkTestCase { @BeforeTest + @Override protected void setUp() throws Exception { - super.setUp(); initDispatcher(null); } - protected Dispatcher initDispatcher(Map params) { + protected Dispatcher initDispatcher(Map params) { Dispatcher du = StrutsTestCaseHelper.initDispatcher(new MockServletContext(), params); configurationManager = du.getConfigurationManager(); configuration = configurationManager.getConfiguration(); @@ -56,8 +56,8 @@ protected T createAction(Class clazz) { } @AfterTest + @Override protected void tearDown() throws Exception { - super.tearDown(); StrutsTestCaseHelper.tearDown(); } } From fa5b46c7844d114a52a91b606627fc6082e37c3f Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 2 Jan 2024 02:57:19 +1100 Subject: [PATCH 07/30] WW-5382 Add test for Dispatcher reinjection --- .../struts2/dispatcher/DispatcherTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java index 267fb4e227..f633b7c012 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java @@ -569,6 +569,29 @@ public void testGetLocale_With_NullDefaultLocale_And_RuntimeException() { assertEquals(Locale.getDefault(), context.getLocale()); // Expect the system default value when Mock request access fails. } + @Test + public void dispatcherReinjectedAfterReload() { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + + dispatcher.prepare(request, response); + + assertEquals(Locale.GERMANY, dispatcher.getLocale(request)); + + configurationManager.addContainerProvider(new StubConfigurationProvider() { + @Override + public void register(ContainerBuilder builder, + LocatableProperties props) throws ConfigurationException { + props.setProperty(StrutsConstants.STRUTS_LOCALE, "fr_CA"); + } + }); + configurationManager.reload(); + dispatcher.cleanUpRequest(request); + dispatcher.prepare(request, response); + + assertEquals(Locale.CANADA_FRENCH, dispatcher.getLocale(request)); + } + public static Dispatcher spyDispatcherWithConfigurationManager(Dispatcher dispatcher, ConfigurationManager configurationManager) { Dispatcher spiedDispatcher = spy(dispatcher); doReturn(configurationManager).when(spiedDispatcher).createConfigurationManager(any()); From ae71c464a815c56c8817b609615783e37949bffb Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 2 Jan 2024 02:59:40 +1100 Subject: [PATCH 08/30] WW-5382 Delete redundant code --- .../org/apache/struts2/dispatcher/Dispatcher.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index a53398b775..6fab13b729 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -983,18 +983,7 @@ protected boolean isMultipartRequest(HttpServletRequest request) { * @return a multi part request object */ protected MultiPartRequest getMultiPartRequest() { - MultiPartRequest mpr = null; - //check for alternate implementations of MultiPartRequest - Set multiNames = getContainer().getInstanceNames(MultiPartRequest.class); - for (String multiName : multiNames) { - if (multiName.equals(multipartHandlerName)) { - mpr = getContainer().getInstance(MultiPartRequest.class, multiName); - } - } - if (mpr == null) { - mpr = getContainer().getInstance(MultiPartRequest.class); - } - return mpr; + return getContainer().getInstance(MultiPartRequest.class); } /** From 946737c81184f4b8c398a58e852faae197d6f84e Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 2 Jan 2024 03:29:54 +1100 Subject: [PATCH 09/30] WW-5382 Rework Dispatcher injections --- .../apache/struts2/dispatcher/Dispatcher.java | 91 +- .../struts2/dispatcher/ExecuteOperations.java | 4 +- .../struts2/dispatcher/InitOperations.java | 2 +- .../struts2/dispatcher/PrepareOperations.java | 8 +- .../struts2/util/StrutsTestCaseHelper.java | 2 +- .../struts2/dispatcher/DispatcherTest.java | 6 +- .../struts2/validators/DWRValidator.java | 33 +- .../portlet/dispatcher/Jsr168Dispatcher.java | 1348 ++++++++--------- .../OldDecorator2NewStrutsDecorator.java | 403 ++--- 9 files changed, 969 insertions(+), 928 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 6fab13b729..c534069c37 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -64,6 +64,7 @@ import org.apache.struts2.config.StrutsJavaConfiguration; import org.apache.struts2.config.StrutsJavaConfigurationProvider; import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; @@ -119,6 +120,10 @@ public class Dispatcher { */ private static final List dispatcherListeners = new CopyOnWriteArrayList<>(); + /** + * This field exists so {@link #getContainer()} can determine whether to (re-)inject this instance in the case of + * a {@link ConfigurationManager} reload. + */ private Container injectedContainer; /** @@ -146,11 +151,6 @@ public class Dispatcher { */ private String multipartSaveDir; - /** - * Stores the value of {@link StrutsConstants#STRUTS_MULTIPART_PARSER} setting - */ - private String multipartHandlerName; - /** * Stores the value of {@link StrutsConstants#STRUTS_MULTIPART_ENABLED} */ @@ -194,6 +194,11 @@ public class Dispatcher { * Store ConfigurationManager instance, set on init. */ protected ConfigurationManager configurationManager; + private ObjectFactory objectFactory; + private ActionProxyFactory actionProxyFactory; + private LocaleProviderFactory localeProviderFactory; + private StaticContentLoader staticContentLoader; + private ActionMapper actionMapper; /** * Provide the dispatcher instance for the current thread. @@ -213,6 +218,13 @@ public static void setInstance(Dispatcher instance) { Dispatcher.instance.set(instance); } + /** + * Removes the dispatcher instance for this thread. + */ + public static void clearInstance() { + Dispatcher.instance.remove(); + } + /** * Add a dispatcher lifecycle listener. * @@ -308,9 +320,12 @@ public void setMultipartSaveDir(String val) { multipartSaveDir = val; } - @Inject(StrutsConstants.STRUTS_MULTIPART_PARSER) + /** + * @deprecated since 6.4.0, no replacement. + */ + @Deprecated public void setMultipartHandler(String val) { - multipartHandlerName = val; + // no-op } @Inject(value = StrutsConstants.STRUTS_MULTIPART_ENABLED, required = false) @@ -328,6 +343,10 @@ public void setValueStackFactory(ValueStackFactory valueStackFactory) { this.valueStackFactory = valueStackFactory; } + public ValueStackFactory getValueStackFactory() { + return valueStackFactory; + } + @Inject(StrutsConstants.STRUTS_HANDLE_EXCEPTION) public void setHandleException(String handleException) { this.handleException = Boolean.parseBoolean(handleException); @@ -348,12 +367,48 @@ public void setDispatcherErrorHandler(DispatcherErrorHandler errorHandler) { this.errorHandler = errorHandler; } + @Inject + public void setObjectFactory(ObjectFactory objectFactory) { + this.objectFactory = objectFactory; + } + + @Inject + public void setActionProxyFactory(ActionProxyFactory actionProxyFactory) { + this.actionProxyFactory = actionProxyFactory; + } + + public ActionProxyFactory getActionProxyFactory() { + return actionProxyFactory; + } + + @Inject + public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory) { + this.localeProviderFactory = localeProviderFactory; + } + + @Inject + public void setStaticContentLoader(StaticContentLoader staticContentLoader) { + this.staticContentLoader = staticContentLoader; + } + + public StaticContentLoader getStaticContentLoader() { + return staticContentLoader; + } + + @Inject + public void setActionMapper(ActionMapper actionMapper) { + this.actionMapper = actionMapper; + } + + public ActionMapper getActionMapper() { + return actionMapper; + } + /** * Releases all instances bound to this dispatcher instance. */ public void cleanup() { // clean up ObjectFactory - ObjectFactory objectFactory = getContainer().getInstance(ObjectFactory.class); if (objectFactory == null) { LOG.warn("Object Factory is null, something is seriously wrong, no clean up will be performed"); } @@ -540,10 +595,6 @@ private void init_DeferredXmlConfigurations() { loadConfigPaths("struts-deferred.xml"); } - private Container init_PreloadConfiguration() { - return getContainer(); - } - /** * Load configurations, including both XML and zero-configuration strategies, * and update optional settings, including whether to reload configurations and resource files. @@ -684,7 +735,6 @@ protected ActionProxy prepareActionProxy(Map extraContext, Strin } protected ActionProxy createActionProxy(String namespace, String name, String method, Map extraContext) { - ActionProxyFactory actionProxyFactory = getContainer().getInstance(ActionProxyFactory.class); return actionProxyFactory.createActionProxy(namespace, name, method, extraContext, true, false); } @@ -860,6 +910,7 @@ protected String getSaveDir() { * @param response The response */ public void prepare(HttpServletRequest request, HttpServletResponse response) { + getContainer(); // Init ContainerHolder and reinject this instance IF ConfigurationManager was reloaded String encoding = null; if (defaultEncoding != null) { encoding = defaultEncoding; @@ -932,15 +983,12 @@ public HttpServletRequest wrapRequest(HttpServletRequest request) throws IOExcep } if (isMultipartSupportEnabled(request) && isMultipartRequest(request)) { - MultiPartRequest multiPartRequest = getMultiPartRequest(); - LocaleProviderFactory localeProviderFactory = getContainer().getInstance(LocaleProviderFactory.class); - request = new MultiPartRequestWrapper( - multiPartRequest, - request, - getSaveDir(), - localeProviderFactory.createLocaleProvider(), - disableRequestAttributeValueStackLookup + getMultiPartRequest(), + request, + getSaveDir(), + localeProviderFactory.createLocaleProvider(), + disableRequestAttributeValueStackLookup ); } else { request = new StrutsRequestWrapper(request, disableRequestAttributeValueStackLookup); @@ -1065,5 +1113,4 @@ public Container getContainer() { } return ContainerHolder.get(); } - } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java index 8671688de2..769ca0a390 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java @@ -18,8 +18,8 @@ */ package org.apache.struts2.dispatcher; -import org.apache.struts2.dispatcher.mapper.ActionMapping; import org.apache.struts2.RequestUtils; +import org.apache.struts2.dispatcher.mapper.ActionMapping; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -54,7 +54,7 @@ public boolean executeStaticResourceRequest(HttpServletRequest request, HttpServ resourcePath = request.getPathInfo(); } - StaticContentLoader staticResourceLoader = dispatcher.getContainer().getInstance(StaticContentLoader.class); + StaticContentLoader staticResourceLoader = dispatcher.getStaticContentLoader(); if (staticResourceLoader.canHandle(resourcePath)) { staticResourceLoader.findStaticResource(resourcePath, request, response); // The framework did its job here diff --git a/core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java index 819e7cdb94..367aeba55c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java @@ -57,7 +57,7 @@ public Dispatcher initDispatcher(HostConfig filterConfig) { * @return the static content loader */ public StaticContentLoader initStaticContentLoader(HostConfig filterConfig, Dispatcher dispatcher) { - StaticContentLoader loader = dispatcher.getContainer().getInstance(StaticContentLoader.class); + StaticContentLoader loader = dispatcher.getStaticContentLoader(); loader.setHostConfig(filterConfig); return loader; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java index a643de3101..6888c5b7aa 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java @@ -20,13 +20,11 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.util.ValueStack; -import com.opensymphony.xwork2.util.ValueStackFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.RequestUtils; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsException; -import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; import javax.servlet.ServletException; @@ -78,7 +76,7 @@ public void cleanupRequest(final HttpServletRequest request) { dispatcher.cleanUpRequest(request); } finally { ActionContext.clear(); - Dispatcher.setInstance(null); + Dispatcher.clearInstance(); devModeOverride.remove(); } }); @@ -101,7 +99,7 @@ public ActionContext createActionContext(HttpServletRequest request, HttpServlet } else { ctx = ServletActionContext.getActionContext(request); //checks if we are probably in an async if (ctx == null) { - ValueStack stack = dispatcher.getContainer().getInstance(ValueStackFactory.class).createValueStack(); + ValueStack stack = dispatcher.getValueStackFactory().createValueStack(); stack.getContext().putAll(dispatcher.createContextMap(request, response, null)); ctx = ActionContext.of(stack.getContext()).bind(); } @@ -188,7 +186,7 @@ public ActionMapping findActionMapping(HttpServletRequest request, HttpServletRe Object mappingAttr = request.getAttribute(STRUTS_ACTION_MAPPING_KEY); if (mappingAttr == null || forceLookup) { try { - mapping = dispatcher.getContainer().getInstance(ActionMapper.class).getMapping(request, dispatcher.getConfigurationManager()); + mapping = dispatcher.getActionMapper().getMapping(request, dispatcher.getConfigurationManager()); if (mapping != null) { request.setAttribute(STRUTS_ACTION_MAPPING_KEY, mapping); } else { diff --git a/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java b/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java index 0f9a1bdf90..418db330ce 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsTestCaseHelper.java @@ -59,7 +59,7 @@ public static void tearDown(Dispatcher dispatcher) { public static void tearDown() { (new Dispatcher(null, null)).cleanUpAfterInit(); // Clear ContainerHolder - Dispatcher.setInstance(null); + Dispatcher.clearInstance(); ActionContext.clear(); } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java index f633b7c012..8884667204 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java @@ -251,12 +251,8 @@ public void testInitLoadsDefaultConfig() { @Test public void testObjectFactoryDestroy() { - dispatcher = spy(dispatcher); - Container spiedContainer = spy(container); - doReturn(spiedContainer).when(dispatcher).getContainer(); - InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory(); - doReturn(destroyedObjectFactory).when(spiedContainer).getInstance(ObjectFactory.class); + dispatcher.setObjectFactory(destroyedObjectFactory); assertFalse(destroyedObjectFactory.destroyed); dispatcher.cleanup(); diff --git a/plugins/dwr/src/main/java/org/apache/struts2/validators/DWRValidator.java b/plugins/dwr/src/main/java/org/apache/struts2/validators/DWRValidator.java index 0809ea0e21..f189cd1820 100644 --- a/plugins/dwr/src/main/java/org/apache/struts2/validators/DWRValidator.java +++ b/plugins/dwr/src/main/java/org/apache/struts2/validators/DWRValidator.java @@ -18,31 +18,26 @@ */ package org.apache.struts2.validators; -import java.util.HashMap; -import java.util.Map; - -import javax.servlet.ServletContext; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.apache.struts2.ServletActionContext; -import org.apache.struts2.dispatcher.ApplicationMap; -import org.apache.struts2.dispatcher.Dispatcher; -import org.apache.struts2.dispatcher.HttpParameters; -import org.apache.struts2.dispatcher.RequestMap; -import org.apache.struts2.dispatcher.SessionMap; - -import org.directwebremoting.WebContextFactory; - import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionProxy; import com.opensymphony.xwork2.ActionProxyFactory; import com.opensymphony.xwork2.DefaultActionInvocation; -import com.opensymphony.xwork2.interceptor.ValidationAware; import com.opensymphony.xwork2.ValidationAwareSupport; import com.opensymphony.xwork2.config.entities.ActionConfig; -import org.apache.logging.log4j.Logger; +import com.opensymphony.xwork2.interceptor.ValidationAware; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.dispatcher.ApplicationMap; +import org.apache.struts2.dispatcher.Dispatcher; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.RequestMap; +import org.apache.struts2.dispatcher.SessionMap; +import org.directwebremoting.WebContextFactory; + +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Map; /** *

@@ -87,7 +82,7 @@ public ValidationAwareSupport doPost(String namespace, String actionName, Map pa res); try { - ActionProxyFactory actionProxyFactory = du.getContainer().getInstance(ActionProxyFactory.class); + ActionProxyFactory actionProxyFactory = du.getActionProxyFactory(); ActionProxy proxy = actionProxyFactory.createActionProxy(namespace, actionName, null, ctx, true, true); proxy.execute(); Object action = proxy.getAction(); diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java index 394ef60420..29b563dac2 100644 --- a/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java +++ b/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java @@ -1,674 +1,674 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.struts2.portlet.dispatcher; - -import com.opensymphony.xwork2.ActionContext; -import com.opensymphony.xwork2.ActionProxy; -import com.opensymphony.xwork2.ActionProxyFactory; -import com.opensymphony.xwork2.config.ConfigurationException; -import com.opensymphony.xwork2.inject.Container; -import org.apache.commons.lang3.LocaleUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.struts2.StrutsConstants; -import org.apache.struts2.StrutsException; -import org.apache.struts2.StrutsStatics; -import org.apache.struts2.dispatcher.ApplicationMap; -import org.apache.struts2.dispatcher.Dispatcher; -import org.apache.struts2.dispatcher.DispatcherConstants; -import org.apache.struts2.dispatcher.HttpParameters; -import org.apache.struts2.dispatcher.RequestMap; -import org.apache.struts2.dispatcher.SessionMap; -import org.apache.struts2.dispatcher.mapper.ActionMapper; -import org.apache.struts2.dispatcher.mapper.ActionMapping; -import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; -import org.apache.struts2.portlet.PortletApplicationMap; -import org.apache.struts2.portlet.PortletConstants; -import org.apache.struts2.portlet.PortletPhase; -import org.apache.struts2.portlet.PortletRequestMap; -import org.apache.struts2.portlet.PortletSessionMap; -import org.apache.struts2.portlet.context.PortletActionContext; -import org.apache.struts2.portlet.servlet.PortletServletContext; -import org.apache.struts2.portlet.servlet.PortletServletRequest; -import org.apache.struts2.portlet.servlet.PortletServletResponse; -import org.apache.struts2.dispatcher.AttributeMap; - -import javax.portlet.ActionRequest; -import javax.portlet.ActionResponse; -import javax.portlet.GenericPortlet; -import javax.portlet.PortletConfig; -import javax.portlet.PortletException; -import javax.portlet.PortletMode; -import javax.portlet.PortletRequest; -import javax.portlet.PortletResponse; -import javax.portlet.RenderRequest; -import javax.portlet.RenderResponse; -import javax.portlet.WindowState; -import javax.servlet.ServletContext; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; - -import static org.apache.struts2.portlet.PortletConstants.ACTION_PARAM; -import static org.apache.struts2.portlet.PortletConstants.ACTION_RESET; -import static org.apache.struts2.portlet.PortletConstants.DEFAULT_ACTION_FOR_MODE; -import static org.apache.struts2.portlet.PortletConstants.DEFAULT_ACTION_NAME; -import static org.apache.struts2.portlet.PortletConstants.MODE_NAMESPACE_MAP; -import static org.apache.struts2.portlet.PortletConstants.MODE_PARAM; -import static org.apache.struts2.portlet.PortletConstants.PORTLET_CONFIG; -import static org.apache.struts2.portlet.PortletConstants.PORTLET_NAMESPACE; -import static org.apache.struts2.portlet.PortletConstants.REQUEST; -import static org.apache.struts2.portlet.PortletConstants.RESPONSE; - -/** - * - *

- * Struts JSR-168 portlet dispatcher. Similar to the WW2 Servlet dispatcher, - * but adjusted to a portal environment. The portlet is configured through the portlet.xml - * descriptor. Examples and descriptions follow below: - *

- * - * - * @author Nils-Helge Garli - * @author Rainer Hermanns - * - *

Init parameters

- * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - *
NameDescriptionDefault value
portletNamespaceThe namespace for the portlet in the xwork configuration. This - * namespace is prepended to all action lookups, and makes it possible to host multiple - * portlets in the same portlet application. If this parameter is set, the complete namespace - * will be /portletNamespace/modeNamespace/actionNameThe default namespace
viewNamespaceBase namespace in the xwork configuration for the view portlet - * modeThe default namespace
editNamespaceBase namespace in the xwork configuration for the edit portlet - * modeThe default namespace
helpNamespaceBase namespace in the xwork configuration for the help portlet - * modeThe default namespace
defaultViewActionDefault action to invoke in the view portlet mode if no action is - * specifieddefault
defaultEditActionDefault action to invoke in the edit portlet mode if no action is - * specifieddefault
defaultHelpActionDefault action to invoke in the help portlet mode if no action is - * specifieddefault
- * - *

Example:

- * 
- * 
- *
- * <init-param>
- *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
- *     <name>viewNamespace</name>
- *     <value>/view</value>
- * </init-param>
- * <init-param>
- *    <!-- The default action to invoke in view mode -->
- *    <name>defaultViewAction</name>
- *    <value>index</value>
- * </init-param>
- * <init-param>
- *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
- *     <name>editNamespace</name>
- *     <value>/edit</value>
- * </init-param>
- * <init-param>
- *     <!-- The default action to invoke in view mode -->
- *     <name>defaultEditAction</name>
- *     <value>index</value>
- * </init-param>
- * <init-param>
- *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
- *     <name>helpNamespace</name>
- *     <value>/help</value>
- * </init-param>
- * <init-param>
- *     <!-- The default action to invoke in view mode -->
- *     <name>defaultHelpAction</name>
- *     <value>index</value>
- * </init-param>
- *
- * 
- *
- */ -public class Jsr168Dispatcher extends GenericPortlet implements StrutsStatics { - - private static final Logger LOG = LogManager.getLogger(Jsr168Dispatcher.class); - - protected String portletNamespace = null; - - private ActionProxyFactory factory = null; - private Map modeMap = new HashMap(3); - private Map actionMap = new HashMap(3); - private Dispatcher dispatcherUtils; - private ActionMapper actionMapper; - private Container container; - private ServletContext servletContext; - - /** - * Initialize the portlet with the init parameters from portlet.xml - * - * @param cfg portlet configuration - * @throws PortletException in case of errors - */ - public void init(PortletConfig cfg) throws PortletException { - super.init(cfg); - LOG.debug("Initializing portlet {}", getPortletName()); - - Map params = new HashMap(); - for (Enumeration e = cfg.getInitParameterNames(); e.hasMoreElements(); ) { - String name = (String) e.nextElement(); - String value = cfg.getInitParameter(name); - params.put(name, value); - } - - servletContext = new PortletServletContext(cfg.getPortletContext()); - dispatcherUtils = new Dispatcher(servletContext, params); - dispatcherUtils.init(); - - // For testability - if (factory == null) { - factory = dispatcherUtils.getContainer().getInstance(ActionProxyFactory.class); - } - portletNamespace = cfg.getInitParameter("portletNamespace"); - LOG.debug("PortletNamespace: {}", portletNamespace); - - parseModeConfig(actionMap, cfg, PortletMode.VIEW, "viewNamespace", - "defaultViewAction"); - parseModeConfig(actionMap, cfg, PortletMode.EDIT, "editNamespace", - "defaultEditAction"); - parseModeConfig(actionMap, cfg, PortletMode.HELP, "helpNamespace", - "defaultHelpAction"); - parseModeConfig(actionMap, cfg, new PortletMode("config"), "configNamespace", - "defaultConfigAction"); - parseModeConfig(actionMap, cfg, new PortletMode("about"), "aboutNamespace", - "defaultAboutAction"); - parseModeConfig(actionMap, cfg, new PortletMode("print"), "printNamespace", - "defaultPrintAction"); - parseModeConfig(actionMap, cfg, new PortletMode("preview"), "previewNamespace", - "defaultPreviewAction"); - parseModeConfig(actionMap, cfg, new PortletMode("edit_defaults"), - "editDefaultsNamespace", "defaultEditDefaultsAction"); - if (StringUtils.isEmpty(portletNamespace)) { - portletNamespace = ""; - } - - container = dispatcherUtils.getContainer(); - actionMapper = container.getInstance(ActionMapper.class); - } - - /** - * Parse the mode to namespace mappings configured in portlet.xml - * - * @param actionMap The map with mode <-> default action mapping. - * @param portletConfig The PortletConfig. - * @param portletMode The PortletMode. - * @param nameSpaceParam Name of the init parameter where the namespace for the mode - * is configured. - * @param defaultActionParam Name of the init parameter where the default action to - * execute for the mode is configured. - */ - void parseModeConfig(Map actionMap, PortletConfig portletConfig, - PortletMode portletMode, String nameSpaceParam, - String defaultActionParam) { - String namespace = portletConfig.getInitParameter(nameSpaceParam); - if (StringUtils.isEmpty(namespace)) { - namespace = ""; - } - modeMap.put(portletMode, namespace); - String defaultAction = portletConfig.getInitParameter(defaultActionParam); - String method = null; - if (StringUtils.isEmpty(defaultAction)) { - defaultAction = DEFAULT_ACTION_NAME; - } - if (defaultAction.indexOf('!') >= 0) { - method = defaultAction.substring(defaultAction.indexOf('!') + 1); - defaultAction = defaultAction.substring(0, defaultAction.indexOf('!')); - } - StringBuilder fullPath = new StringBuilder(); - if (StringUtils.isNotEmpty(portletNamespace)) { - fullPath.append(portletNamespace); - } - if (StringUtils.isNotEmpty(namespace)) { - fullPath.append(namespace).append("/"); - } else { - fullPath.append("/"); - } - fullPath.append(defaultAction); - ActionMapping mapping = new ActionMapping(); - mapping.setName(getActionName(fullPath.toString())); - mapping.setNamespace(getNamespace(fullPath.toString())); - if (method != null) { - mapping.setMethod(method); - } - actionMap.put(portletMode, mapping); - } - - /** - * Service an action from the event phase. - * - * @param request action request - * @param response action response - * @throws PortletException in case of errors - * @throws IOException in case of IO errors - * @see javax.portlet.Portlet#processAction(javax.portlet.ActionRequest, - * javax.portlet.ActionResponse) - */ - public void processAction(ActionRequest request, ActionResponse response) - throws PortletException, IOException { - if (LOG.isDebugEnabled()) { - LOG.debug("Entering processAction in mode ", request.getPortletMode().toString()); - } - resetActionContext(); - try { - serviceAction(request, response, getRequestMap(request), getParameterMap(request), - getSessionMap(request), getApplicationMap(), - portletNamespace, PortletPhase.ACTION_PHASE); - if (LOG.isDebugEnabled()) LOG.debug("Leaving processAction"); - } finally { - ActionContext.clear(); - } - } - - /** - * Service an action from the render phase. - * - * @param request render request - * @param response render response - * @throws PortletException in case of errors - * @throws IOException in case of IO errors - * @see javax.portlet.Portlet#render(javax.portlet.RenderRequest, - * javax.portlet.RenderResponse) - */ - public void render(RenderRequest request, RenderResponse response) - throws PortletException, IOException { - - if (LOG.isDebugEnabled()) { - LOG.debug("Entering render in mode ", request.getPortletMode().toString()); - } - resetActionContext(); - response.setTitle(getTitle(request)); - if (!request.getWindowState().equals(WindowState.MINIMIZED)) { - try { - // Check to see if an event set the render to be included directly - serviceAction(request, response, getRequestMap(request), getParameterMap(request), - getSessionMap(request), getApplicationMap(), - portletNamespace, PortletPhase.RENDER_PHASE); - if (LOG.isDebugEnabled()) LOG.debug("Leaving render"); - } finally { - resetActionContext(); - } - } - } - - /** - * Reset the action context. - */ - void resetActionContext() { - ActionContext.clear(); - } - - /** - * Merges all application and portlet attributes into a single - * HashMap to represent the entire Action context. - * - * @param requestMap a Map of all request attributes. - * @param parameterMap a Map of all request parameters. - * @param sessionMap a Map of all session attributes. - * @param applicationMap a Map of all servlet context attributes. - * @param request the PortletRequest object. - * @param response the PortletResponse object. - * @param servletRequest the HttpServletRequest object. - * @param servletResponse the HttpServletResponse object. - * @param servletContext the ServletContext object. - * @param portletConfig the PortletConfig object. - * @param phase The portlet phase (render or action, see - * {@link PortletConstants}) - * @return a HashMap representing the Action context. - * @throws IOException in case of IO errors - */ - public Map createContextMap(Map requestMap, Map parameterMap, - Map sessionMap, Map applicationMap, - PortletRequest request, PortletResponse response, HttpServletRequest servletRequest, - HttpServletResponse servletResponse, ServletContext servletContext, - PortletConfig portletConfig, PortletPhase phase) throws IOException { - - // TODO Must put http request/response objects into map for use with - container.inject(servletRequest); - - // ServletActionContext - Map extraContext = ActionContext.of(new HashMap<>()) - .withServletRequest(servletRequest) - .withServletResponse(servletResponse) - .withServletContext(servletContext) - .withParameters(HttpParameters.create(parameterMap).build()) - .withSession(sessionMap) - .withApplication(applicationMap) - .withLocale(getLocale(request)) - .with(StrutsStatics.STRUTS_PORTLET_CONTEXT, getPortletContext()) - .with(REQUEST, request) - .with(RESPONSE, response) - .with(PORTLET_CONFIG, portletConfig) - .with(PORTLET_NAMESPACE, portletNamespace) - .with(DEFAULT_ACTION_FOR_MODE, actionMap.get(request.getPortletMode())) - // helpers to get access to request/session/application scope - .with(DispatcherConstants.REQUEST, requestMap) - .with(DispatcherConstants.SESSION, sessionMap) - .with(DispatcherConstants.APPLICATION, applicationMap) - .with(DispatcherConstants.PARAMETERS, parameterMap) - .with(MODE_NAMESPACE_MAP, modeMap) - .with(PortletConstants.DEFAULT_ACTION_MAP, actionMap) - .with(PortletConstants.PHASE, phase) - .getContextMap(); - - AttributeMap attrMap = new AttributeMap(extraContext); - extraContext.put("attr", attrMap); - - return extraContext; - } - - protected Locale getLocale(PortletRequest request) { - String defaultLocale = container.getInstance(String.class, StrutsConstants.STRUTS_LOCALE); - Locale locale; - if (defaultLocale != null) { - try { - locale = LocaleUtils.toLocale(defaultLocale); - } catch (IllegalArgumentException e) { - LOG.warn(new ParameterizedMessage("Cannot convert 'struts.locale' = [{}] to proper locale, defaulting to request locale [{}]", - defaultLocale, request.getLocale()), e); - locale = request.getLocale(); - } - } else { - locale = request.getLocale(); - } - return locale; - } - - /** - * Loads the action and executes it. This method first creates the action - * context from the given parameters then loads an ActionProxy - * from the given action name and namespace. After that, the action is - * executed and output channels throught the response object. - * - * @param request the HttpServletRequest object. - * @param response the HttpServletResponse object. - * @param requestMap a Map of request attributes. - * @param parameterMap a Map of request parameters. - * @param sessionMap a Map of all session attributes. - * @param applicationMap a Map of all application attributes. - * @param portletNamespace the namespace or context of the action. - * @param phase The portlet phase (render or action, see {@link PortletConstants}) - * @throws PortletException in case of errors - */ - public void serviceAction(PortletRequest request, PortletResponse response, Map requestMap, Map parameterMap, - Map sessionMap, Map applicationMap, String portletNamespace, - PortletPhase phase) throws PortletException { - if (LOG.isDebugEnabled()) LOG.debug("serviceAction"); - Dispatcher.setInstance(dispatcherUtils); - String actionName = null; - String namespace; - try { - HttpServletRequest servletRequest = new PortletServletRequest(request, getPortletContext()); - HttpServletResponse servletResponse = createPortletServletResponse(response); - if (phase.isAction()) { - servletRequest = dispatcherUtils.wrapRequest(servletRequest); - if (servletRequest instanceof MultiPartRequestWrapper) { - // Multipart request. Request parameters are encoded in the multipart data, - // so we need to manually add them to the parameter map. - parameterMap.putAll(servletRequest.getParameterMap()); - } - } - container.inject(servletRequest); - ActionMapping mapping = getActionMapping(request, servletRequest); - actionName = mapping.getName(); - if ("renderDirect".equals(actionName)) { - namespace = request.getParameter(PortletConstants.RENDER_DIRECT_NAMESPACE); - } else { - namespace = mapping.getNamespace(); - } - Map extraContext = createContextMap(requestMap, parameterMap, - sessionMap, applicationMap, request, response, servletRequest, servletResponse, - servletContext, getPortletConfig(), phase); - extraContext.put(PortletConstants.ACTION_MAPPING, mapping); - if (LOG.isDebugEnabled()) { - LOG.debug("Creating action proxy for name = " + actionName + ", namespace = " + namespace); - } - ActionProxy proxy = factory.createActionProxy(namespace, actionName, mapping.getMethod(), extraContext); - request.setAttribute("struts.valueStack", proxy.getInvocation().getStack()); - proxy.execute(); - } catch (ConfigurationException e) { - if (LOG.isErrorEnabled()) { - LOG.error("Could not find action", e); - } - throw new PortletException("Could not find action " + actionName, e); - } catch (Exception e) { - if (LOG.isErrorEnabled()) { - LOG.error("Could not execute action", e); - } - throw new PortletException("Error executing action " + actionName, e); - } finally { - Dispatcher.setInstance(null); - } - } - - /** - * Returns a Map of all application attributes. Copies all attributes from - * the {@link PortletActionContext}into an {@link ApplicationMap}. - * - * @return a Map of all application attributes. - */ - protected Map getApplicationMap() { - return new PortletApplicationMap(getPortletContext()); - } - - /** - * Gets the namespace of the action from the request. The namespace is the - * same as the portlet mode. E.g, view mode is mapped to namespace - * view, and edit mode is mapped to the namespace - * edit - * - * @param portletRequest the PortletRequest object. - * @param servletRequest the ServletRequest to use - * @return the namespace of the action. - */ - protected ActionMapping getActionMapping(final PortletRequest portletRequest, final HttpServletRequest servletRequest) { - ActionMapping mapping; - String actionPath = getDefaultActionPath(portletRequest); - if (resetAction(portletRequest)) { - mapping = actionMap.get(portletRequest.getPortletMode()); - } else { - actionPath = servletRequest.getParameter(ACTION_PARAM); - if (StringUtils.isEmpty(actionPath)) { - mapping = actionMap.get(portletRequest.getPortletMode()); - } else { - - // Use the usual action mapper, but it is expecting an action extension - // on the uri, so we add the default one, which should be ok as the - // portlet is a portlet first, a servlet second - mapping = actionMapper.getMapping(servletRequest, dispatcherUtils.getConfigurationManager()); - } - } - - if (mapping == null) { - throw new StrutsException("Unable to locate action mapping for request, probably due to an invalid action path: " + actionPath); - } - return mapping; - } - - protected String getDefaultActionPath(PortletRequest portletRequest) { - return null; - } - - /** - * Get the namespace part of the action path. - * - * @param actionPath Full path to action - * @return The namespace part. - */ - String getNamespace(String actionPath) { - int idx = actionPath.lastIndexOf('/'); - String namespace = ""; - if (idx >= 0) { - namespace = actionPath.substring(0, idx); - } - return namespace; - } - - /** - * Get the action name part of the action path. - * - * @param actionPath Full path to action - * @return The action name. - */ - String getActionName(String actionPath) { - int idx = actionPath.lastIndexOf('/'); - String action = actionPath; - if (idx >= 0) { - action = actionPath.substring(idx + 1); - } - return action; - } - - /** - * Returns a Map of all request parameters. This implementation just calls - * {@link PortletRequest#getParameterMap()}. - * - * @param request the PortletRequest object. - * @return a Map of all request parameters. - * @throws IOException if an exception occurs while retrieving the parameter - * map. - */ - protected Map getParameterMap(PortletRequest request) throws IOException { - return new HashMap(request.getParameterMap()); - } - - /** - * Returns a Map of all request attributes. The default implementation is to - * wrap the request in a {@link RequestMap}. Override this method to - * customize how request attributes are mapped. - * - * @param request the PortletRequest object. - * @return a Map of all request attributes. - */ - protected Map getRequestMap(PortletRequest request) { - return new PortletRequestMap(request); - } - - /** - * Returns a Map of all session attributes. The default implementation is to - * wrap the reqeust in a {@link SessionMap}. Override this method to - * customize how session attributes are mapped. - * - * @param request the PortletRequest object. - * @return a Map of all session attributes. - */ - protected Map getSessionMap(PortletRequest request) { - return new PortletSessionMap(request); - } - - /** - * Convenience method to ease testing. - * - * @param factory action proxy factory - */ - protected void setActionProxyFactory(ActionProxyFactory factory) { - this.factory = factory; - } - - /** - * Check to see if the action parameter is valid for the current portlet mode. If the portlet - * mode has been changed with the portal widgets, the action name is invalid, since the - * action name belongs to the previous executing portlet mode. If this method evaluates to - * true the default<Mode>Action is used instead. - * - * @param request The portlet request. - * @return true if the action should be reset. - */ - private boolean resetAction(PortletRequest request) { - boolean reset = false; - Map paramMap = request.getParameterMap(); - String[] modeParam = (String[]) paramMap.get(MODE_PARAM); - if (modeParam != null && modeParam.length == 1) { - String originatingMode = modeParam[0]; - String currentMode = request.getPortletMode().toString(); - if (!currentMode.equals(originatingMode)) { - reset = true; - } - } - if (reset) { - request.setAttribute(ACTION_RESET, Boolean.TRUE); - } else { - request.setAttribute(ACTION_RESET, Boolean.FALSE); - } - return reset; - } - - public void destroy() { - if (dispatcherUtils != null) { - dispatcherUtils.cleanup(); - } else { - if (LOG.isWarnEnabled()) { - LOG.warn("Something is seriously wrong, DispatcherUtil is not initialized (null) "); - } - } - } - - /** - * @param actionMapper the actionMapper to set - */ - public void setActionMapper(ActionMapper actionMapper) { - this.actionMapper = actionMapper; - } - - /** - * Method to create a PortletServletResponse matching the used Portlet API, to be overridden for JSR286 Dispatcher. - * - * @param response The Response used for building the wrapper. - * @return The wrapper response for Servlet bound usage. - */ - protected PortletServletResponse createPortletServletResponse(PortletResponse response) { - return new PortletServletResponse(response); - } - -} +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.portlet.dispatcher; + +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.ActionProxy; +import com.opensymphony.xwork2.ActionProxyFactory; +import com.opensymphony.xwork2.config.ConfigurationException; +import com.opensymphony.xwork2.inject.Container; +import org.apache.commons.lang3.LocaleUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.StrutsException; +import org.apache.struts2.StrutsStatics; +import org.apache.struts2.dispatcher.ApplicationMap; +import org.apache.struts2.dispatcher.AttributeMap; +import org.apache.struts2.dispatcher.Dispatcher; +import org.apache.struts2.dispatcher.DispatcherConstants; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.RequestMap; +import org.apache.struts2.dispatcher.SessionMap; +import org.apache.struts2.dispatcher.mapper.ActionMapper; +import org.apache.struts2.dispatcher.mapper.ActionMapping; +import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; +import org.apache.struts2.portlet.PortletApplicationMap; +import org.apache.struts2.portlet.PortletConstants; +import org.apache.struts2.portlet.PortletPhase; +import org.apache.struts2.portlet.PortletRequestMap; +import org.apache.struts2.portlet.PortletSessionMap; +import org.apache.struts2.portlet.context.PortletActionContext; +import org.apache.struts2.portlet.servlet.PortletServletContext; +import org.apache.struts2.portlet.servlet.PortletServletRequest; +import org.apache.struts2.portlet.servlet.PortletServletResponse; + +import javax.portlet.ActionRequest; +import javax.portlet.ActionResponse; +import javax.portlet.GenericPortlet; +import javax.portlet.PortletConfig; +import javax.portlet.PortletException; +import javax.portlet.PortletMode; +import javax.portlet.PortletRequest; +import javax.portlet.PortletResponse; +import javax.portlet.RenderRequest; +import javax.portlet.RenderResponse; +import javax.portlet.WindowState; +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +import static org.apache.struts2.portlet.PortletConstants.ACTION_PARAM; +import static org.apache.struts2.portlet.PortletConstants.ACTION_RESET; +import static org.apache.struts2.portlet.PortletConstants.DEFAULT_ACTION_FOR_MODE; +import static org.apache.struts2.portlet.PortletConstants.DEFAULT_ACTION_NAME; +import static org.apache.struts2.portlet.PortletConstants.MODE_NAMESPACE_MAP; +import static org.apache.struts2.portlet.PortletConstants.MODE_PARAM; +import static org.apache.struts2.portlet.PortletConstants.PORTLET_CONFIG; +import static org.apache.struts2.portlet.PortletConstants.PORTLET_NAMESPACE; +import static org.apache.struts2.portlet.PortletConstants.REQUEST; +import static org.apache.struts2.portlet.PortletConstants.RESPONSE; + +/** + * + *

+ * Struts JSR-168 portlet dispatcher. Similar to the WW2 Servlet dispatcher, + * but adjusted to a portal environment. The portlet is configured through the portlet.xml + * descriptor. Examples and descriptions follow below: + *

+ * + * + * @author Nils-Helge Garli + * @author Rainer Hermanns + * + *

Init parameters

+ * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
NameDescriptionDefault value
portletNamespaceThe namespace for the portlet in the xwork configuration. This + * namespace is prepended to all action lookups, and makes it possible to host multiple + * portlets in the same portlet application. If this parameter is set, the complete namespace + * will be /portletNamespace/modeNamespace/actionNameThe default namespace
viewNamespaceBase namespace in the xwork configuration for the view portlet + * modeThe default namespace
editNamespaceBase namespace in the xwork configuration for the edit portlet + * modeThe default namespace
helpNamespaceBase namespace in the xwork configuration for the help portlet + * modeThe default namespace
defaultViewActionDefault action to invoke in the view portlet mode if no action is + * specifieddefault
defaultEditActionDefault action to invoke in the edit portlet mode if no action is + * specifieddefault
defaultHelpActionDefault action to invoke in the help portlet mode if no action is + * specifieddefault
+ * + *

Example:

+ * 
+ * 
+ *
+ * <init-param>
+ *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
+ *     <name>viewNamespace</name>
+ *     <value>/view</value>
+ * </init-param>
+ * <init-param>
+ *    <!-- The default action to invoke in view mode -->
+ *    <name>defaultViewAction</name>
+ *    <value>index</value>
+ * </init-param>
+ * <init-param>
+ *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
+ *     <name>editNamespace</name>
+ *     <value>/edit</value>
+ * </init-param>
+ * <init-param>
+ *     <!-- The default action to invoke in view mode -->
+ *     <name>defaultEditAction</name>
+ *     <value>index</value>
+ * </init-param>
+ * <init-param>
+ *     <!-- The view mode namespace. Maps to a namespace in the xwork config file -->
+ *     <name>helpNamespace</name>
+ *     <value>/help</value>
+ * </init-param>
+ * <init-param>
+ *     <!-- The default action to invoke in view mode -->
+ *     <name>defaultHelpAction</name>
+ *     <value>index</value>
+ * </init-param>
+ *
+ * 
+ *
+ */ +public class Jsr168Dispatcher extends GenericPortlet implements StrutsStatics { + + private static final Logger LOG = LogManager.getLogger(Jsr168Dispatcher.class); + + protected String portletNamespace = null; + + private ActionProxyFactory factory = null; + private Map modeMap = new HashMap(3); + private Map actionMap = new HashMap(3); + private Dispatcher dispatcherUtils; + private ActionMapper actionMapper; + private Container container; + private ServletContext servletContext; + + /** + * Initialize the portlet with the init parameters from portlet.xml + * + * @param cfg portlet configuration + * @throws PortletException in case of errors + */ + public void init(PortletConfig cfg) throws PortletException { + super.init(cfg); + LOG.debug("Initializing portlet {}", getPortletName()); + + Map params = new HashMap(); + for (Enumeration e = cfg.getInitParameterNames(); e.hasMoreElements(); ) { + String name = (String) e.nextElement(); + String value = cfg.getInitParameter(name); + params.put(name, value); + } + + servletContext = new PortletServletContext(cfg.getPortletContext()); + dispatcherUtils = new Dispatcher(servletContext, params); + dispatcherUtils.init(); + + // For testability + if (factory == null) { + factory = dispatcherUtils.getActionProxyFactory(); + } + portletNamespace = cfg.getInitParameter("portletNamespace"); + LOG.debug("PortletNamespace: {}", portletNamespace); + + parseModeConfig(actionMap, cfg, PortletMode.VIEW, "viewNamespace", + "defaultViewAction"); + parseModeConfig(actionMap, cfg, PortletMode.EDIT, "editNamespace", + "defaultEditAction"); + parseModeConfig(actionMap, cfg, PortletMode.HELP, "helpNamespace", + "defaultHelpAction"); + parseModeConfig(actionMap, cfg, new PortletMode("config"), "configNamespace", + "defaultConfigAction"); + parseModeConfig(actionMap, cfg, new PortletMode("about"), "aboutNamespace", + "defaultAboutAction"); + parseModeConfig(actionMap, cfg, new PortletMode("print"), "printNamespace", + "defaultPrintAction"); + parseModeConfig(actionMap, cfg, new PortletMode("preview"), "previewNamespace", + "defaultPreviewAction"); + parseModeConfig(actionMap, cfg, new PortletMode("edit_defaults"), + "editDefaultsNamespace", "defaultEditDefaultsAction"); + if (StringUtils.isEmpty(portletNamespace)) { + portletNamespace = ""; + } + + container = dispatcherUtils.getContainer(); + actionMapper = container.getInstance(ActionMapper.class); + } + + /** + * Parse the mode to namespace mappings configured in portlet.xml + * + * @param actionMap The map with mode <-> default action mapping. + * @param portletConfig The PortletConfig. + * @param portletMode The PortletMode. + * @param nameSpaceParam Name of the init parameter where the namespace for the mode + * is configured. + * @param defaultActionParam Name of the init parameter where the default action to + * execute for the mode is configured. + */ + void parseModeConfig(Map actionMap, PortletConfig portletConfig, + PortletMode portletMode, String nameSpaceParam, + String defaultActionParam) { + String namespace = portletConfig.getInitParameter(nameSpaceParam); + if (StringUtils.isEmpty(namespace)) { + namespace = ""; + } + modeMap.put(portletMode, namespace); + String defaultAction = portletConfig.getInitParameter(defaultActionParam); + String method = null; + if (StringUtils.isEmpty(defaultAction)) { + defaultAction = DEFAULT_ACTION_NAME; + } + if (defaultAction.indexOf('!') >= 0) { + method = defaultAction.substring(defaultAction.indexOf('!') + 1); + defaultAction = defaultAction.substring(0, defaultAction.indexOf('!')); + } + StringBuilder fullPath = new StringBuilder(); + if (StringUtils.isNotEmpty(portletNamespace)) { + fullPath.append(portletNamespace); + } + if (StringUtils.isNotEmpty(namespace)) { + fullPath.append(namespace).append("/"); + } else { + fullPath.append("/"); + } + fullPath.append(defaultAction); + ActionMapping mapping = new ActionMapping(); + mapping.setName(getActionName(fullPath.toString())); + mapping.setNamespace(getNamespace(fullPath.toString())); + if (method != null) { + mapping.setMethod(method); + } + actionMap.put(portletMode, mapping); + } + + /** + * Service an action from the event phase. + * + * @param request action request + * @param response action response + * @throws PortletException in case of errors + * @throws IOException in case of IO errors + * @see javax.portlet.Portlet#processAction(javax.portlet.ActionRequest, + * javax.portlet.ActionResponse) + */ + public void processAction(ActionRequest request, ActionResponse response) + throws PortletException, IOException { + if (LOG.isDebugEnabled()) { + LOG.debug("Entering processAction in mode ", request.getPortletMode().toString()); + } + resetActionContext(); + try { + serviceAction(request, response, getRequestMap(request), getParameterMap(request), + getSessionMap(request), getApplicationMap(), + portletNamespace, PortletPhase.ACTION_PHASE); + if (LOG.isDebugEnabled()) LOG.debug("Leaving processAction"); + } finally { + ActionContext.clear(); + } + } + + /** + * Service an action from the render phase. + * + * @param request render request + * @param response render response + * @throws PortletException in case of errors + * @throws IOException in case of IO errors + * @see javax.portlet.Portlet#render(javax.portlet.RenderRequest, + * javax.portlet.RenderResponse) + */ + public void render(RenderRequest request, RenderResponse response) + throws PortletException, IOException { + + if (LOG.isDebugEnabled()) { + LOG.debug("Entering render in mode ", request.getPortletMode().toString()); + } + resetActionContext(); + response.setTitle(getTitle(request)); + if (!request.getWindowState().equals(WindowState.MINIMIZED)) { + try { + // Check to see if an event set the render to be included directly + serviceAction(request, response, getRequestMap(request), getParameterMap(request), + getSessionMap(request), getApplicationMap(), + portletNamespace, PortletPhase.RENDER_PHASE); + if (LOG.isDebugEnabled()) LOG.debug("Leaving render"); + } finally { + resetActionContext(); + } + } + } + + /** + * Reset the action context. + */ + void resetActionContext() { + ActionContext.clear(); + } + + /** + * Merges all application and portlet attributes into a single + * HashMap to represent the entire Action context. + * + * @param requestMap a Map of all request attributes. + * @param parameterMap a Map of all request parameters. + * @param sessionMap a Map of all session attributes. + * @param applicationMap a Map of all servlet context attributes. + * @param request the PortletRequest object. + * @param response the PortletResponse object. + * @param servletRequest the HttpServletRequest object. + * @param servletResponse the HttpServletResponse object. + * @param servletContext the ServletContext object. + * @param portletConfig the PortletConfig object. + * @param phase The portlet phase (render or action, see + * {@link PortletConstants}) + * @return a HashMap representing the Action context. + * @throws IOException in case of IO errors + */ + public Map createContextMap(Map requestMap, Map parameterMap, + Map sessionMap, Map applicationMap, + PortletRequest request, PortletResponse response, HttpServletRequest servletRequest, + HttpServletResponse servletResponse, ServletContext servletContext, + PortletConfig portletConfig, PortletPhase phase) throws IOException { + + // TODO Must put http request/response objects into map for use with + container.inject(servletRequest); + + // ServletActionContext + Map extraContext = ActionContext.of(new HashMap<>()) + .withServletRequest(servletRequest) + .withServletResponse(servletResponse) + .withServletContext(servletContext) + .withParameters(HttpParameters.create(parameterMap).build()) + .withSession(sessionMap) + .withApplication(applicationMap) + .withLocale(getLocale(request)) + .with(StrutsStatics.STRUTS_PORTLET_CONTEXT, getPortletContext()) + .with(REQUEST, request) + .with(RESPONSE, response) + .with(PORTLET_CONFIG, portletConfig) + .with(PORTLET_NAMESPACE, portletNamespace) + .with(DEFAULT_ACTION_FOR_MODE, actionMap.get(request.getPortletMode())) + // helpers to get access to request/session/application scope + .with(DispatcherConstants.REQUEST, requestMap) + .with(DispatcherConstants.SESSION, sessionMap) + .with(DispatcherConstants.APPLICATION, applicationMap) + .with(DispatcherConstants.PARAMETERS, parameterMap) + .with(MODE_NAMESPACE_MAP, modeMap) + .with(PortletConstants.DEFAULT_ACTION_MAP, actionMap) + .with(PortletConstants.PHASE, phase) + .getContextMap(); + + AttributeMap attrMap = new AttributeMap(extraContext); + extraContext.put("attr", attrMap); + + return extraContext; + } + + protected Locale getLocale(PortletRequest request) { + String defaultLocale = container.getInstance(String.class, StrutsConstants.STRUTS_LOCALE); + Locale locale; + if (defaultLocale != null) { + try { + locale = LocaleUtils.toLocale(defaultLocale); + } catch (IllegalArgumentException e) { + LOG.warn(new ParameterizedMessage("Cannot convert 'struts.locale' = [{}] to proper locale, defaulting to request locale [{}]", + defaultLocale, request.getLocale()), e); + locale = request.getLocale(); + } + } else { + locale = request.getLocale(); + } + return locale; + } + + /** + * Loads the action and executes it. This method first creates the action + * context from the given parameters then loads an ActionProxy + * from the given action name and namespace. After that, the action is + * executed and output channels throught the response object. + * + * @param request the HttpServletRequest object. + * @param response the HttpServletResponse object. + * @param requestMap a Map of request attributes. + * @param parameterMap a Map of request parameters. + * @param sessionMap a Map of all session attributes. + * @param applicationMap a Map of all application attributes. + * @param portletNamespace the namespace or context of the action. + * @param phase The portlet phase (render or action, see {@link PortletConstants}) + * @throws PortletException in case of errors + */ + public void serviceAction(PortletRequest request, PortletResponse response, Map requestMap, Map parameterMap, + Map sessionMap, Map applicationMap, String portletNamespace, + PortletPhase phase) throws PortletException { + if (LOG.isDebugEnabled()) LOG.debug("serviceAction"); + Dispatcher.setInstance(dispatcherUtils); + String actionName = null; + String namespace; + try { + HttpServletRequest servletRequest = new PortletServletRequest(request, getPortletContext()); + HttpServletResponse servletResponse = createPortletServletResponse(response); + if (phase.isAction()) { + servletRequest = dispatcherUtils.wrapRequest(servletRequest); + if (servletRequest instanceof MultiPartRequestWrapper) { + // Multipart request. Request parameters are encoded in the multipart data, + // so we need to manually add them to the parameter map. + parameterMap.putAll(servletRequest.getParameterMap()); + } + } + container.inject(servletRequest); + ActionMapping mapping = getActionMapping(request, servletRequest); + actionName = mapping.getName(); + if ("renderDirect".equals(actionName)) { + namespace = request.getParameter(PortletConstants.RENDER_DIRECT_NAMESPACE); + } else { + namespace = mapping.getNamespace(); + } + Map extraContext = createContextMap(requestMap, parameterMap, + sessionMap, applicationMap, request, response, servletRequest, servletResponse, + servletContext, getPortletConfig(), phase); + extraContext.put(PortletConstants.ACTION_MAPPING, mapping); + if (LOG.isDebugEnabled()) { + LOG.debug("Creating action proxy for name = " + actionName + ", namespace = " + namespace); + } + ActionProxy proxy = factory.createActionProxy(namespace, actionName, mapping.getMethod(), extraContext); + request.setAttribute("struts.valueStack", proxy.getInvocation().getStack()); + proxy.execute(); + } catch (ConfigurationException e) { + if (LOG.isErrorEnabled()) { + LOG.error("Could not find action", e); + } + throw new PortletException("Could not find action " + actionName, e); + } catch (Exception e) { + if (LOG.isErrorEnabled()) { + LOG.error("Could not execute action", e); + } + throw new PortletException("Error executing action " + actionName, e); + } finally { + Dispatcher.clearInstance(); + } + } + + /** + * Returns a Map of all application attributes. Copies all attributes from + * the {@link PortletActionContext}into an {@link ApplicationMap}. + * + * @return a Map of all application attributes. + */ + protected Map getApplicationMap() { + return new PortletApplicationMap(getPortletContext()); + } + + /** + * Gets the namespace of the action from the request. The namespace is the + * same as the portlet mode. E.g, view mode is mapped to namespace + * view, and edit mode is mapped to the namespace + * edit + * + * @param portletRequest the PortletRequest object. + * @param servletRequest the ServletRequest to use + * @return the namespace of the action. + */ + protected ActionMapping getActionMapping(final PortletRequest portletRequest, final HttpServletRequest servletRequest) { + ActionMapping mapping; + String actionPath = getDefaultActionPath(portletRequest); + if (resetAction(portletRequest)) { + mapping = actionMap.get(portletRequest.getPortletMode()); + } else { + actionPath = servletRequest.getParameter(ACTION_PARAM); + if (StringUtils.isEmpty(actionPath)) { + mapping = actionMap.get(portletRequest.getPortletMode()); + } else { + + // Use the usual action mapper, but it is expecting an action extension + // on the uri, so we add the default one, which should be ok as the + // portlet is a portlet first, a servlet second + mapping = actionMapper.getMapping(servletRequest, dispatcherUtils.getConfigurationManager()); + } + } + + if (mapping == null) { + throw new StrutsException("Unable to locate action mapping for request, probably due to an invalid action path: " + actionPath); + } + return mapping; + } + + protected String getDefaultActionPath(PortletRequest portletRequest) { + return null; + } + + /** + * Get the namespace part of the action path. + * + * @param actionPath Full path to action + * @return The namespace part. + */ + String getNamespace(String actionPath) { + int idx = actionPath.lastIndexOf('/'); + String namespace = ""; + if (idx >= 0) { + namespace = actionPath.substring(0, idx); + } + return namespace; + } + + /** + * Get the action name part of the action path. + * + * @param actionPath Full path to action + * @return The action name. + */ + String getActionName(String actionPath) { + int idx = actionPath.lastIndexOf('/'); + String action = actionPath; + if (idx >= 0) { + action = actionPath.substring(idx + 1); + } + return action; + } + + /** + * Returns a Map of all request parameters. This implementation just calls + * {@link PortletRequest#getParameterMap()}. + * + * @param request the PortletRequest object. + * @return a Map of all request parameters. + * @throws IOException if an exception occurs while retrieving the parameter + * map. + */ + protected Map getParameterMap(PortletRequest request) throws IOException { + return new HashMap(request.getParameterMap()); + } + + /** + * Returns a Map of all request attributes. The default implementation is to + * wrap the request in a {@link RequestMap}. Override this method to + * customize how request attributes are mapped. + * + * @param request the PortletRequest object. + * @return a Map of all request attributes. + */ + protected Map getRequestMap(PortletRequest request) { + return new PortletRequestMap(request); + } + + /** + * Returns a Map of all session attributes. The default implementation is to + * wrap the reqeust in a {@link SessionMap}. Override this method to + * customize how session attributes are mapped. + * + * @param request the PortletRequest object. + * @return a Map of all session attributes. + */ + protected Map getSessionMap(PortletRequest request) { + return new PortletSessionMap(request); + } + + /** + * Convenience method to ease testing. + * + * @param factory action proxy factory + */ + protected void setActionProxyFactory(ActionProxyFactory factory) { + this.factory = factory; + } + + /** + * Check to see if the action parameter is valid for the current portlet mode. If the portlet + * mode has been changed with the portal widgets, the action name is invalid, since the + * action name belongs to the previous executing portlet mode. If this method evaluates to + * true the default<Mode>Action is used instead. + * + * @param request The portlet request. + * @return true if the action should be reset. + */ + private boolean resetAction(PortletRequest request) { + boolean reset = false; + Map paramMap = request.getParameterMap(); + String[] modeParam = (String[]) paramMap.get(MODE_PARAM); + if (modeParam != null && modeParam.length == 1) { + String originatingMode = modeParam[0]; + String currentMode = request.getPortletMode().toString(); + if (!currentMode.equals(originatingMode)) { + reset = true; + } + } + if (reset) { + request.setAttribute(ACTION_RESET, Boolean.TRUE); + } else { + request.setAttribute(ACTION_RESET, Boolean.FALSE); + } + return reset; + } + + public void destroy() { + if (dispatcherUtils != null) { + dispatcherUtils.cleanup(); + } else { + if (LOG.isWarnEnabled()) { + LOG.warn("Something is seriously wrong, DispatcherUtil is not initialized (null) "); + } + } + } + + /** + * @param actionMapper the actionMapper to set + */ + public void setActionMapper(ActionMapper actionMapper) { + this.actionMapper = actionMapper; + } + + /** + * Method to create a PortletServletResponse matching the used Portlet API, to be overridden for JSR286 Dispatcher. + * + * @param response The Response used for building the wrapper. + * @return The wrapper response for Servlet bound usage. + */ + protected PortletServletResponse createPortletServletResponse(PortletResponse response) { + return new PortletServletResponse(response); + } + +} diff --git a/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsDecorator.java b/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsDecorator.java index f5a0cd1a8d..989141044d 100644 --- a/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsDecorator.java +++ b/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsDecorator.java @@ -1,199 +1,204 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.struts2.sitemesh; - -import com.opensymphony.module.sitemesh.RequestConstants; -import com.opensymphony.sitemesh.Content; -import com.opensymphony.sitemesh.webapp.SiteMeshWebAppContext; -import com.opensymphony.sitemesh.webapp.decorator.BaseWebAppDecorator; -import com.opensymphony.xwork2.*; -import com.opensymphony.xwork2.interceptor.PreResultListener; -import com.opensymphony.xwork2.util.ValueStack; -import com.opensymphony.xwork2.util.ValueStackFactory; -import freemarker.template.Configuration; -import org.apache.struts2.ServletActionContext; -import org.apache.struts2.dispatcher.Dispatcher; - -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.Locale; - -/** - * Adapts a SiteMesh 2 {@link com.opensymphony.module.sitemesh.Decorator} to a - * SiteMesh 3 {@link com.opensymphony.sitemesh.Decorator}. - * - * @since SiteMesh 3 - */ -public abstract class OldDecorator2NewStrutsDecorator extends BaseWebAppDecorator implements RequestConstants { - - protected com.opensymphony.module.sitemesh.Decorator oldDecorator; - private static String customEncoding; - - public OldDecorator2NewStrutsDecorator(com.opensymphony.module.sitemesh.Decorator oldDecorator) { - this.oldDecorator = oldDecorator; - } - - public OldDecorator2NewStrutsDecorator() { - oldDecorator = null; - } - - - /** - * Applies the decorator, using the relevent contexts - * - * @param content The content - * @param request The servlet request - * @param response The servlet response - * @param servletContext The servlet context - * @param ctx The action context for this request, populated with the server state - */ - protected abstract void render(Content content, HttpServletRequest request, HttpServletResponse response, ServletContext servletContext, ActionContext ctx) throws ServletException, IOException; - - /** - * Applies the decorator, creating the relevent contexts and delegating to - * the extended applyDecorator(). - * - * @param content The content - * @param request The servlet request - * @param response The servlet response - * @param servletContext The servlet context - * @param webAppContext The web app context - */ - - protected void render(Content content, HttpServletRequest request, HttpServletResponse response, ServletContext servletContext, SiteMeshWebAppContext webAppContext) throws IOException, ServletException { - - // see if the URI path (webapp) is set - if (oldDecorator.getURIPath() != null) { - // in a security conscious environment, the servlet container - // may return null for a given URL - if (servletContext.getContext(oldDecorator.getURIPath()) != null) { - servletContext = servletContext.getContext(oldDecorator.getURIPath()); - } - } - - ActionContext ctx = ServletActionContext.getActionContext(request); - if (ctx == null) { - // ok, one isn't associated with the request, so let's create one using the current Dispatcher - ValueStack vs = Dispatcher.getInstance().getContainer().getInstance(ValueStackFactory.class).createValueStack(); - vs.getContext().putAll(Dispatcher.getInstance().createContextMap(request, response, null)); - ctx = ActionContext.of(vs.getContext()); - if (ctx.getActionInvocation() == null) { - // put in a dummy ActionSupport so basic functionality still works - ActionSupport action = new ActionSupport(); - vs.push(action); - ctx.withActionInvocation(new DummyActionInvocation(action)); - } - } - - // delegate to the actual page decorator - render(content, request, response, servletContext, ctx); - - } - - /** - * Returns the locale used for the {@link freemarker.template.Configuration#getTemplate(String, java.util.Locale)} call. The base implementation - * simply returns the locale setting of the action (assuming the action implements {@link LocaleProvider}) or, if - * the action does not the configuration's locale is returned. Override this method to provide different behaviour, - */ - protected Locale getLocale(ActionInvocation invocation, Configuration configuration) { - if (invocation.getAction() instanceof LocaleProvider) { - return ((LocaleProvider) invocation.getAction()).getLocale(); - } else { - return configuration.getLocale(); - } - } - - - /** - * Gets the L18N encoding of the system. The default is UTF-8. - */ - protected String getEncoding() { - String encoding = customEncoding; - if (encoding == null) { - encoding = System.getProperty("file.encoding"); - } - if (encoding == null) { - encoding = "UTF-8"; - } - return encoding; - } - - - static class DummyActionInvocation implements ActionInvocation { - - ActionSupport action; - - public DummyActionInvocation(ActionSupport action) { - this.action = action; - } - - public Object getAction() { - return action; - } - - public boolean isExecuted() { - return false; - } - - public ActionContext getInvocationContext() { - return null; - } - - public ActionProxy getProxy() { - return null; - } - - public Result getResult() throws Exception { - return null; - } - - public String getResultCode() { - return null; - } - - public void setResultCode(String resultCode) { - } - - public ValueStack getStack() { - return null; - } - - public void addPreResultListener(PreResultListener listener) { - } - - public String invoke() throws Exception { - return null; - } - - public String invokeActionOnly() throws Exception { - return null; - } - - public void setActionEventListener(ActionEventListener listener) { - } - - public void init(ActionProxy proxy) { - } - - } - -} +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.sitemesh; + +import com.opensymphony.module.sitemesh.RequestConstants; +import com.opensymphony.sitemesh.Content; +import com.opensymphony.sitemesh.webapp.SiteMeshWebAppContext; +import com.opensymphony.sitemesh.webapp.decorator.BaseWebAppDecorator; +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.ActionEventListener; +import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.ActionProxy; +import com.opensymphony.xwork2.ActionSupport; +import com.opensymphony.xwork2.LocaleProvider; +import com.opensymphony.xwork2.Result; +import com.opensymphony.xwork2.interceptor.PreResultListener; +import com.opensymphony.xwork2.util.ValueStack; +import freemarker.template.Configuration; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.dispatcher.Dispatcher; + +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Locale; + +/** + * Adapts a SiteMesh 2 {@link com.opensymphony.module.sitemesh.Decorator} to a + * SiteMesh 3 {@link com.opensymphony.sitemesh.Decorator}. + * + * @since SiteMesh 3 + */ +public abstract class OldDecorator2NewStrutsDecorator extends BaseWebAppDecorator implements RequestConstants { + + protected com.opensymphony.module.sitemesh.Decorator oldDecorator; + private static String customEncoding; + + public OldDecorator2NewStrutsDecorator(com.opensymphony.module.sitemesh.Decorator oldDecorator) { + this.oldDecorator = oldDecorator; + } + + public OldDecorator2NewStrutsDecorator() { + oldDecorator = null; + } + + + /** + * Applies the decorator, using the relevent contexts + * + * @param content The content + * @param request The servlet request + * @param response The servlet response + * @param servletContext The servlet context + * @param ctx The action context for this request, populated with the server state + */ + protected abstract void render(Content content, HttpServletRequest request, HttpServletResponse response, ServletContext servletContext, ActionContext ctx) throws ServletException, IOException; + + /** + * Applies the decorator, creating the relevent contexts and delegating to + * the extended applyDecorator(). + * + * @param content The content + * @param request The servlet request + * @param response The servlet response + * @param servletContext The servlet context + * @param webAppContext The web app context + */ + + protected void render(Content content, HttpServletRequest request, HttpServletResponse response, ServletContext servletContext, SiteMeshWebAppContext webAppContext) throws IOException, ServletException { + + // see if the URI path (webapp) is set + if (oldDecorator.getURIPath() != null) { + // in a security conscious environment, the servlet container + // may return null for a given URL + if (servletContext.getContext(oldDecorator.getURIPath()) != null) { + servletContext = servletContext.getContext(oldDecorator.getURIPath()); + } + } + + ActionContext ctx = ServletActionContext.getActionContext(request); + if (ctx == null) { + // ok, one isn't associated with the request, so let's create one using the current Dispatcher + ValueStack vs = Dispatcher.getInstance().getValueStackFactory().createValueStack(); + vs.getContext().putAll(Dispatcher.getInstance().createContextMap(request, response, null)); + ctx = ActionContext.of(vs.getContext()); + if (ctx.getActionInvocation() == null) { + // put in a dummy ActionSupport so basic functionality still works + ActionSupport action = new ActionSupport(); + vs.push(action); + ctx.withActionInvocation(new DummyActionInvocation(action)); + } + } + + // delegate to the actual page decorator + render(content, request, response, servletContext, ctx); + + } + + /** + * Returns the locale used for the {@link freemarker.template.Configuration#getTemplate(String, java.util.Locale)} call. The base implementation + * simply returns the locale setting of the action (assuming the action implements {@link LocaleProvider}) or, if + * the action does not the configuration's locale is returned. Override this method to provide different behaviour, + */ + protected Locale getLocale(ActionInvocation invocation, Configuration configuration) { + if (invocation.getAction() instanceof LocaleProvider) { + return ((LocaleProvider) invocation.getAction()).getLocale(); + } else { + return configuration.getLocale(); + } + } + + + /** + * Gets the L18N encoding of the system. The default is UTF-8. + */ + protected String getEncoding() { + String encoding = customEncoding; + if (encoding == null) { + encoding = System.getProperty("file.encoding"); + } + if (encoding == null) { + encoding = "UTF-8"; + } + return encoding; + } + + + static class DummyActionInvocation implements ActionInvocation { + + ActionSupport action; + + public DummyActionInvocation(ActionSupport action) { + this.action = action; + } + + public Object getAction() { + return action; + } + + public boolean isExecuted() { + return false; + } + + public ActionContext getInvocationContext() { + return null; + } + + public ActionProxy getProxy() { + return null; + } + + public Result getResult() throws Exception { + return null; + } + + public String getResultCode() { + return null; + } + + public void setResultCode(String resultCode) { + } + + public ValueStack getStack() { + return null; + } + + public void addPreResultListener(PreResultListener listener) { + } + + public String invoke() throws Exception { + return null; + } + + public String invokeActionOnly() throws Exception { + return null; + } + + public void setActionEventListener(ActionEventListener listener) { + } + + public void init(ActionProxy proxy) { + } + + } + +} From 3d25caa0a8ecd2175b59d6b85f2ba3918e67000d Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 2 Jan 2024 03:48:49 +1100 Subject: [PATCH 10/30] WW-5382 Update Dispatcher#getContainer JavaDoc --- .../main/java/org/apache/struts2/dispatcher/Dispatcher.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index c534069c37..3947d89d25 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -1095,7 +1095,11 @@ public ConfigurationManager getConfigurationManager() { } /** - * Expose the dependency injection container. + * Exposes a thread-cached reference of the dependency injection container. If the container is found to have + * changed since the last time it was cached, this Dispatcher instance is re-injected to ensure no stale + * configuration/dependencies persist. + *

+ * A non-cached reference can be obtained by calling {@link #getConfigurationManager()}. * * @return Our dependency injection container */ From 8a245e865bae044181af4a1901b558fcf17e200a Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Wed, 3 Jan 2024 20:54:55 +1100 Subject: [PATCH 11/30] WW-5352 Refactor ParametersInterceptor --- .../security/AcceptedPatternsChecker.java | 12 +- .../security/ExcludedPatternsChecker.java | 12 +- .../apache/struts2/dispatcher/Parameter.java | 8 +- .../ActionMappingParametersInterceptor.java | 6 +- .../parameter/ParametersInterceptor.java | 250 +++++++++--------- .../parameter/ParametersInterceptorTest.java | 2 +- 6 files changed, 148 insertions(+), 142 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java index f5a329459f..4af56ff9e1 100644 --- a/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java +++ b/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java @@ -32,37 +32,37 @@ public interface AcceptedPatternsChecker { * @param value to check * @return object containing result of matched pattern and pattern itself */ - public IsAccepted isAccepted(String value); + IsAccepted isAccepted(String value); /** * Sets excluded patterns during runtime * * @param commaDelimitedPatterns comma delimited string with patterns */ - public void setAcceptedPatterns(String commaDelimitedPatterns); + void setAcceptedPatterns(String commaDelimitedPatterns); /** * Set excluded patterns during runtime * * @param patterns array of additional excluded patterns */ - public void setAcceptedPatterns(String[] patterns); + void setAcceptedPatterns(String[] patterns); /** * Sets excluded patterns during runtime * * @param patterns set of additional patterns */ - public void setAcceptedPatterns(Set patterns); + void setAcceptedPatterns(Set patterns); /** * Allow access list of all defined excluded patterns * * @return set of excluded patterns */ - public Set getAcceptedPatterns(); + Set getAcceptedPatterns(); - public final static class IsAccepted { + final class IsAccepted { private final boolean accepted; private final String acceptedPattern; diff --git a/core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java index 6fa54d0d43..086c75d0b6 100644 --- a/core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java +++ b/core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java @@ -32,37 +32,37 @@ public interface ExcludedPatternsChecker { * @param value to check * @return object containing result of matched pattern and pattern itself */ - public IsExcluded isExcluded(String value); + IsExcluded isExcluded(String value); /** * Sets excluded patterns during runtime * * @param commaDelimitedPatterns comma delimited string with patterns */ - public void setExcludedPatterns(String commaDelimitedPatterns); + void setExcludedPatterns(String commaDelimitedPatterns); /** * Sets excluded patterns during runtime * * @param patterns array of additional excluded patterns */ - public void setExcludedPatterns(String[] patterns); + void setExcludedPatterns(String[] patterns); /** * Sets excluded patterns during runtime * * @param patterns set of additional patterns */ - public void setExcludedPatterns(Set patterns); + void setExcludedPatterns(Set patterns); /** * Allow access list of all defined excluded patterns * * @return set of excluded patterns */ - public Set getExcludedPatterns(); + Set getExcludedPatterns(); - public final static class IsExcluded { + final class IsExcluded { private final boolean excluded; private final String excludedPattern; diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java b/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java index edae6c3a17..06aa6783fc 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Parameter.java @@ -18,12 +18,12 @@ */ package org.apache.struts2.dispatcher; -import java.util.Objects; - import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.Objects; + public interface Parameter { String getName(); @@ -58,7 +58,7 @@ public String getName() { @Override public String getValue() { String[] values = toStringArray(); - return (values != null && values.length > 0) ? values[0] : null; + return values.length > 0 ? values[0] : null; } private String[] toStringArray() { @@ -124,7 +124,7 @@ public String toString() { class Empty implements Parameter { - private String name; + private final String name; public Empty(String name) { this.name = name; diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionMappingParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionMappingParametersInterceptor.java index ac9d41708d..ecb1f7f9f1 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionMappingParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionMappingParametersInterceptor.java @@ -76,12 +76,12 @@ public class ActionMappingParametersInterceptor extends ParametersInterceptor { /** * Get the parameter map from ActionMapping associated with the provided ActionContext. * - * @param ac The action context + * @param actionContext The action context * @return the parameters from the action mapping in the context. If none found, returns an empty map. */ @Override - protected HttpParameters retrieveParameters(ActionContext ac) { - ActionMapping mapping = ac.getActionMapping(); + protected HttpParameters retrieveParameters(ActionContext actionContext) { + ActionMapping mapping = actionContext.getActionMapping(); if (mapping != null) { return HttpParameters.create(mapping.getParams()).buildNoNestedWrapping(); } else { diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index c55f97a6a8..4887232551 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -43,7 +43,6 @@ import org.apache.struts2.dispatcher.Parameter; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.HashSet; import java.util.Map; @@ -51,6 +50,8 @@ import java.util.TreeMap; import java.util.regex.Pattern; +import static java.util.Collections.unmodifiableSet; +import static java.util.stream.Collectors.joining; import static org.apache.commons.lang3.StringUtils.normalizeSpace; /** @@ -132,138 +133,149 @@ static private int countOGNLCharacters(String s) { @Override public String doIntercept(ActionInvocation invocation) throws Exception { Object action = invocation.getAction(); - if (!(action instanceof NoParameters)) { - ActionContext ac = invocation.getInvocationContext(); - HttpParameters parameters = retrieveParameters(ac); + if (action instanceof NoParameters) { + return invocation.invoke(); + } - if (LOG.isDebugEnabled()) { - LOG.debug("Setting params {}", normalizeSpace(getParameterLogMap(parameters))); - } + ActionContext actionContext = invocation.getInvocationContext(); + HttpParameters parameters = retrieveParameters(actionContext); - if (parameters != null) { - Map contextMap = ac.getContextMap(); - try { - ReflectionContextState.setCreatingNullObjects(contextMap, true); - ReflectionContextState.setDenyMethodExecution(contextMap, true); - ReflectionContextState.setReportingConversionErrors(contextMap, true); - - ValueStack stack = ac.getValueStack(); - setParameters(action, stack, parameters); - } finally { - ReflectionContextState.setCreatingNullObjects(contextMap, false); - ReflectionContextState.setDenyMethodExecution(contextMap, false); - ReflectionContextState.setReportingConversionErrors(contextMap, false); - } - } + if (parameters == null) { + return invocation.invoke(); + } + + if (LOG.isDebugEnabled()) { + LOG.debug("Setting params {}", normalizeSpace(getParameterLogMap(parameters))); + } + + Map contextMap = actionContext.getContextMap(); + batchSetReflectionContextState(contextMap, true); + try { + setParameters(action, actionContext.getValueStack(), parameters); + } finally { + batchSetReflectionContextState(contextMap, false); } + return invocation.invoke(); } /** * Gets the parameter map to apply from wherever appropriate * - * @param ac The action context + * @param actionContext The action context * @return The parameter map to apply */ - protected HttpParameters retrieveParameters(ActionContext ac) { - return ac.getParameters(); + protected HttpParameters retrieveParameters(ActionContext actionContext) { + return actionContext.getParameters(); } /** * Adds the parameters into context's ParameterMap + *

+ * In this class this is a no-op, since the parameters were fetched from the same location. In subclasses both this + * and {@link #retrieveParameters} should be overridden. * * @param ac The action context * @param newParams The parameter map to apply - *

- * In this class this is a no-op, since the parameters were fetched from the same location. - * In subclasses both retrieveParameters() and addParametersToContext() should be overridden. - *

*/ protected void addParametersToContext(ActionContext ac, Map newParams) { } protected void setParameters(final Object action, ValueStack stack, HttpParameters parameters) { - HttpParameters params; - Map acceptableParameters; - if (ordered) { - params = HttpParameters.create().withComparator(getOrderedComparator()).withParent(parameters).build(); - acceptableParameters = new TreeMap<>(getOrderedComparator()); - } else { - params = HttpParameters.create().withParent(parameters).build(); - acceptableParameters = new TreeMap<>(); - } + Map acceptableParameters = toAcceptableParameters(parameters, action); - for (Map.Entry entry : params.entrySet()) { - String parameterName = entry.getKey(); - boolean isAcceptableParameter = isAcceptableParameter(parameterName, action); - isAcceptableParameter &= isAcceptableParameterValue(entry.getValue(), action); + ValueStack newStack = toNewStack(stack); + batchSetReflectionContextState(newStack.getContext(), true); + setMemberAccessProperties(newStack); - if (isAcceptableParameter) { - acceptableParameters.put(parameterName, entry.getValue()); - } + setParametersOnStack(newStack, acceptableParameters, action); + + if (newStack instanceof ClearableValueStack) { + stack.getActionContext().withConversionErrors(newStack.getActionContext().getConversionErrors()); } + addParametersToContext(ActionContext.getContext(), acceptableParameters); + } + + protected void batchSetReflectionContextState(Map context, boolean value) { + ReflectionContextState.setCreatingNullObjects(context, value); + ReflectionContextState.setDenyMethodExecution(context, value); + ReflectionContextState.setReportingConversionErrors(context, value); + } + + protected ValueStack toNewStack(ValueStack stack) { ValueStack newStack = valueStackFactory.createValueStack(stack); - boolean clearableStack = newStack instanceof ClearableValueStack; - if (clearableStack) { - //if the stack's context can be cleared, do that to prevent OGNL - //from having access to objects in the stack, see XW-641 + if (newStack instanceof ClearableValueStack) { ((ClearableValueStack) newStack).clearContextValues(); - Map context = newStack.getContext(); - ReflectionContextState.setCreatingNullObjects(context, true); - ReflectionContextState.setDenyMethodExecution(context, true); - ReflectionContextState.setReportingConversionErrors(context, true); - - //keep locale from original context newStack.getActionContext().withLocale(stack.getActionContext().getLocale()).withValueStack(stack); } + return newStack; + } + + protected void setMemberAccessProperties(ValueStack stack) { + if (!(stack instanceof MemberAccessValueStack)) { + return; + } + ((MemberAccessValueStack) stack).useAcceptProperties(acceptedPatterns.getAcceptedPatterns()); + ((MemberAccessValueStack) stack).useExcludeProperties(excludedPatterns.getExcludedPatterns()); + } + + protected Map toAcceptableParameters(HttpParameters parameters, Object action) { + HttpParameters newParams = initNewHttpParameters(parameters); + Map acceptableParameters = initParameterMap(); + + for (Map.Entry entry : newParams.entrySet()) { + String parameterName = entry.getKey(); + Parameter parameterValue = entry.getValue(); + if (isAcceptableParameter(parameterName, action) && isAcceptableParameterValue(parameterValue, action)) { + acceptableParameters.put(parameterName, parameterValue); + } + } + return acceptableParameters; + } - boolean memberAccessStack = newStack instanceof MemberAccessValueStack; - if (memberAccessStack) { - //block or allow access to properties - //see WW-2761 for more details - MemberAccessValueStack accessValueStack = (MemberAccessValueStack) newStack; - accessValueStack.useAcceptProperties(acceptedPatterns.getAcceptedPatterns()); - accessValueStack.useExcludeProperties(excludedPatterns.getExcludedPatterns()); + protected Map initParameterMap() { + if (ordered) { + return new TreeMap<>(getOrderedComparator()); + } else { + return new TreeMap<>(); } + } + + protected HttpParameters initNewHttpParameters(HttpParameters parameters) { + if (ordered) { + return HttpParameters.create().withComparator(getOrderedComparator()).withParent(parameters).build(); + } else { + return HttpParameters.create().withParent(parameters).build(); + } + } - for (Map.Entry entry : acceptableParameters.entrySet()) { - String name = entry.getKey(); - Parameter value = entry.getValue(); + protected void setParametersOnStack(ValueStack stack, Map parameters, Object action) { + for (Map.Entry entry : parameters.entrySet()) { try { - newStack.setParameter(name, value.getObject()); + stack.setParameter(entry.getKey(), entry.getValue().getObject()); } catch (RuntimeException e) { if (devMode) { - notifyDeveloperParameterException(action, name, e.getMessage()); + notifyDeveloperParameterException(action, entry.getKey(), e.getMessage()); } } } - - if (clearableStack) { - stack.getActionContext().withConversionErrors(newStack.getActionContext().getConversionErrors()); - } - - addParametersToContext(ActionContext.getContext(), acceptableParameters); } protected void notifyDeveloperParameterException(Object action, String property, String message) { - String developerNotification = "Unexpected Exception caught setting '" + property + "' on '" + action.getClass() + ": " + message; + String logMsg = "Unexpected Exception caught setting '" + property + "' on '" + action.getClass() + ": " + message; if (action instanceof TextProvider) { TextProvider tp = (TextProvider) action; - developerNotification = tp.getText("devmode.notification", - "Developer Notification:\n{0}", - new String[]{developerNotification} - ); + logMsg = tp.getText("devmode.notification", "Developer Notification:\n{0}", new String[]{logMsg}); } - - LOG.error(developerNotification); + LOG.error(logMsg); if (action instanceof ValidationAware) { - // see https://issues.apache.org/jira/browse/WW-4066 - Collection messages = ((ValidationAware) action).getActionMessages(); + ValidationAware validationAware = (ValidationAware) action; + Collection messages = validationAware.getActionMessages(); messages.add(message); - ((ValidationAware) action).setActionMessages(messages); + validationAware.setActionMessages(messages); } } @@ -275,8 +287,11 @@ protected void notifyDeveloperParameterException(Object action, String property, * @return true if parameter is accepted */ protected boolean isAcceptableParameter(String name, Object action) { - ParameterNameAware parameterNameAware = (action instanceof ParameterNameAware) ? (ParameterNameAware) action : null; - return acceptableName(name) && (parameterNameAware == null || parameterNameAware.acceptableParameterName(name)); + return acceptableName(name) && isAcceptableParameterNameAware(name, action); + } + + protected boolean isAcceptableParameterNameAware(String name, Object action) { + return !(action instanceof ParameterNameAware) || ((ParameterNameAware) action).acceptableParameterName(name); } /** @@ -287,13 +302,11 @@ protected boolean isAcceptableParameter(String name, Object action) { * @return true if parameter is accepted */ protected boolean isAcceptableParameterValue(Parameter param, Object action) { - ParameterValueAware parameterValueAware = (action instanceof ParameterValueAware) ? (ParameterValueAware) action : null; - boolean acceptableParamValue = (parameterValueAware == null || parameterValueAware.acceptableParameterValue(param.getValue())); - if (hasParamValuesToExclude() || hasParamValuesToAccept()) { - // Additional validations to process - acceptableParamValue &= acceptableValue(param.getName(), param.getValue()); - } - return acceptableParamValue; + return isAcceptableParameterValueAware(param, action) && acceptableValue(param.getName(), param.getValue()); + } + + protected boolean isAcceptableParameterValueAware(Parameter param, Object action) { + return !(action instanceof ParameterValueAware) || ((ParameterValueAware) action).acceptableParameterValue(param.getValue()); } /** @@ -311,16 +324,9 @@ protected String getParameterLogMap(HttpParameters parameters) { if (parameters == null) { return "NONE"; } - - StringBuilder logEntry = new StringBuilder(); - for (Map.Entry entry : parameters.entrySet()) { - logEntry.append(entry.getKey()); - logEntry.append(" => "); - logEntry.append(entry.getValue().getValue()); - logEntry.append(" "); - } - - return logEntry.toString(); + return parameters.entrySet().stream() + .map(entry -> String.format("%s => %s ", entry.getKey(), entry.getValue().getValue())) + .collect(joining()); } /** @@ -338,18 +344,17 @@ protected boolean acceptableName(String name) { return false; } boolean accepted = isWithinLengthLimit(name) && !isExcluded(name) && isAccepted(name); - if (devMode && accepted) { // notify only when in devMode + if (devMode && accepted) { LOG.debug("Parameter [{}] was accepted and will be appended to action!", name); } return accepted; } private boolean isIgnoredDMI(String name) { - if (dmiEnabled) { - return DMI_IGNORED_PATTERN.matcher(name).matches(); - } else { + if (!dmiEnabled) { return false; } + return DMI_IGNORED_PATTERN.matcher(name).matches(); } /** @@ -363,7 +368,7 @@ private boolean isIgnoredDMI(String name) { * @return true if accepted */ protected boolean acceptableValue(String name, String value) { - boolean accepted = (value == null || value.isEmpty() || (!isParamValueExcluded(value) && isParamValueAccepted(value))); + boolean accepted = value == null || value.isEmpty() || (!isParamValueExcluded(value) && isParamValueAccepted(value)); if (!accepted) { String message = "Value [{}] of parameter [{}] was not accepted and will be dropped!"; if (devMode) { @@ -378,7 +383,7 @@ protected boolean acceptableValue(String name, String value) { protected boolean isWithinLengthLimit(String name) { boolean matchLength = name.length() <= paramNameMaxLength; if (!matchLength) { - if (devMode) { // warn only when in devMode + if (devMode) { LOG.warn("Parameter [{}] is too long, allowed length is [{}]. Use Interceptor Parameter Overriding " + "to override the limit, see more at\n" + "https://struts.apache.org/core-developers/interceptors.html#interceptor-parameter-overriding", @@ -392,22 +397,23 @@ protected boolean isWithinLengthLimit(String name) { protected boolean isAccepted(String paramName) { AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName); - if (result.isAccepted()) { - return true; - } else if (devMode) { // warn only when in devMode - LOG.warn("Parameter [{}] didn't match accepted pattern [{}]! See Accepted / Excluded patterns at\n" + - "https://struts.apache.org/security/#accepted--excluded-patterns", - paramName, result.getAcceptedPattern()); - } else { - LOG.debug("Parameter [{}] didn't match accepted pattern [{}]!", paramName, result.getAcceptedPattern()); + if (!result.isAccepted()) { + if (devMode) { + LOG.warn("Parameter [{}] didn't match accepted pattern [{}]! See Accepted / Excluded patterns at\n" + + "https://struts.apache.org/security/#accepted--excluded-patterns", + paramName, result.getAcceptedPattern()); + } else { + LOG.debug("Parameter [{}] didn't match accepted pattern [{}]!", paramName, result.getAcceptedPattern()); + } + return false; } - return false; + return true; } protected boolean isExcluded(String paramName) { ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName); if (result.isExcluded()) { - if (devMode) { // warn only when in devMode + if (devMode) { LOG.warn("Parameter [{}] matches excluded pattern [{}]! See Accepted / Excluded patterns at\n" + "https://struts.apache.org/security/#accepted--excluded-patterns", paramName, result.getExcludedPattern()); @@ -460,11 +466,11 @@ protected boolean isParamValueAccepted(String value) { } private boolean hasParamValuesToExclude() { - return excludedValuePatterns != null && excludedValuePatterns.size() > 0; + return excludedValuePatterns != null && !excludedValuePatterns.isEmpty(); } private boolean hasParamValuesToAccept() { - return acceptedValuePatterns != null && acceptedValuePatterns.size() > 0; + return acceptedValuePatterns != null && !acceptedValuePatterns.isEmpty(); } /** @@ -530,7 +536,7 @@ public void setAcceptedValuePatterns(String commaDelimitedPatterns) { acceptedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns); + acceptedValuePatterns = unmodifiableSet(acceptedValuePatterns); } } @@ -555,7 +561,7 @@ public void setExcludedValuePatterns(String commaDelimitedPatterns) { excludedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns); + excludedValuePatterns = unmodifiableSet(excludedValuePatterns); } } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java index 333556ae70..33c3ce6e25 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java @@ -1053,7 +1053,7 @@ public boolean hasActionErrors() { } public boolean hasActionMessages() { - return messages.size() > 0; + return !messages.isEmpty(); } public boolean hasErrors() { From 573dbbcea87fe0d60378d1f588531389bf595bcd Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Wed, 3 Jan 2024 21:41:37 +1100 Subject: [PATCH 12/30] WW-5381 Revert bean removal for backwards compatibility --- core/src/main/resources/struts-beans.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 3343b63864..ddc2de7ca1 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -201,6 +201,11 @@ + + + Date: Sat, 6 Jan 2024 02:10:12 +1100 Subject: [PATCH 13/30] WW-5352 Do not use setter notation for helper methods --- .../parameter/ParametersInterceptor.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index 4887232551..e93b60a28c 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -149,11 +149,11 @@ public String doIntercept(ActionInvocation invocation) throws Exception { } Map contextMap = actionContext.getContextMap(); - batchSetReflectionContextState(contextMap, true); + batchApplyReflectionContextState(contextMap, true); try { setParameters(action, actionContext.getValueStack(), parameters); } finally { - batchSetReflectionContextState(contextMap, false); + batchApplyReflectionContextState(contextMap, false); } return invocation.invoke(); @@ -182,14 +182,22 @@ protected HttpParameters retrieveParameters(ActionContext actionContext) { protected void addParametersToContext(ActionContext ac, Map newParams) { } + /** + * @deprecated since 6.4.0, use {@link #applyParameters} + */ + @Deprecated protected void setParameters(final Object action, ValueStack stack, HttpParameters parameters) { + applyParameters(action, stack, parameters); + } + + protected void applyParameters(final Object action, ValueStack stack, HttpParameters parameters) { Map acceptableParameters = toAcceptableParameters(parameters, action); ValueStack newStack = toNewStack(stack); - batchSetReflectionContextState(newStack.getContext(), true); - setMemberAccessProperties(newStack); + batchApplyReflectionContextState(newStack.getContext(), true); + applyMemberAccessProperties(newStack); - setParametersOnStack(newStack, acceptableParameters, action); + applyParametersOnStack(newStack, acceptableParameters, action); if (newStack instanceof ClearableValueStack) { stack.getActionContext().withConversionErrors(newStack.getActionContext().getConversionErrors()); @@ -198,7 +206,7 @@ protected void setParameters(final Object action, ValueStack stack, HttpParamete addParametersToContext(ActionContext.getContext(), acceptableParameters); } - protected void batchSetReflectionContextState(Map context, boolean value) { + protected void batchApplyReflectionContextState(Map context, boolean value) { ReflectionContextState.setCreatingNullObjects(context, value); ReflectionContextState.setDenyMethodExecution(context, value); ReflectionContextState.setReportingConversionErrors(context, value); @@ -213,7 +221,7 @@ protected ValueStack toNewStack(ValueStack stack) { return newStack; } - protected void setMemberAccessProperties(ValueStack stack) { + protected void applyMemberAccessProperties(ValueStack stack) { if (!(stack instanceof MemberAccessValueStack)) { return; } @@ -251,7 +259,7 @@ protected HttpParameters initNewHttpParameters(HttpParameters parameters) { } } - protected void setParametersOnStack(ValueStack stack, Map parameters, Object action) { + protected void applyParametersOnStack(ValueStack stack, Map parameters, Object action) { for (Map.Entry entry : parameters.entrySet()) { try { stack.setParameter(entry.getKey(), entry.getValue().getObject()); From 199ea0db69ec7ed7bed221508341dfed2f840dcc Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sat, 6 Jan 2024 02:10:34 +1100 Subject: [PATCH 14/30] WW-5352 Rename acceptable name/value methods --- .../parameter/ParametersInterceptor.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index e93b60a28c..efc4a7b04e 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -337,6 +337,13 @@ protected String getParameterLogMap(HttpParameters parameters) { .collect(joining()); } + /** + * @deprecated since 6.4.0, use {@link #isAcceptableName} + */ + protected boolean acceptableName(String name) { + return isAcceptableName(name); + } + /** * Validates the name passed is: * * Within the max length of a parameter name @@ -346,7 +353,7 @@ protected String getParameterLogMap(HttpParameters parameters) { * @param name - Name to check * @return true if accepted */ - protected boolean acceptableName(String name) { + protected boolean isAcceptableName(String name) { if (isIgnoredDMI(name)) { LOG.trace("DMI is enabled, ignoring DMI method: {}", name); return false; @@ -365,6 +372,13 @@ private boolean isIgnoredDMI(String name) { return DMI_IGNORED_PATTERN.matcher(name).matches(); } + /** + * @deprecated since 6.4.0, use {@link #isAcceptableValue} + */ + protected boolean acceptableValue(String name, String value) { + return isAcceptableValue(name, value); + } + /** * Validates: * * Value is null/blank @@ -375,7 +389,7 @@ private boolean isIgnoredDMI(String name) { * @param value - value to check * @return true if accepted */ - protected boolean acceptableValue(String name, String value) { + protected boolean isAcceptableValue(String name, String value) { boolean accepted = value == null || value.isEmpty() || (!isParamValueExcluded(value) && isParamValueAccepted(value)); if (!accepted) { String message = "Value [{}] of parameter [{}] was not accepted and will be dropped!"; From d4a0b3b85214577475b71d55b731ac0413f24b85 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sat, 6 Jan 2024 02:42:37 +1100 Subject: [PATCH 15/30] WW-5381 Reimplement ability to register additional MethodAccessors --- .../xwork2/ognl/OgnlValueStackFactory.java | 70 +++++++++++++++++-- 1 file changed, 65 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java index b1d6ea4998..8d3139f2e2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java @@ -31,6 +31,8 @@ import ognl.OgnlRuntime; import ognl.PropertyAccessor; import org.apache.commons.lang3.BooleanUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; import java.util.Set; @@ -40,6 +42,8 @@ */ public class OgnlValueStackFactory implements ValueStackFactory { + private static final Logger LOG = LogManager.getLogger(OgnlValueStackFactory.class); + protected XWorkConverter xworkConverter; protected RootAccessor compoundRootAccessor; protected TextProvider textProvider; @@ -84,20 +88,76 @@ protected ValueStack createValueStack(ValueStack stack, boolean useTextProvider) return newStack.getActionContext().withContainer(container).withValueStack(newStack).getValueStack(); } + /** + * {@link PropertyAccessor}'s, {@link MethodAccessor}'s and {@link NullHandler}'s are registered on a per-class + * basis by defining a bean adhering to the corresponding interface with a name corresponding to the class it is + * intended to handle. + *

+ * The only exception is the {@link MethodAccessor} for the {@link Object} type which has its own extension point. + * + * @see #setMethodAccessor(MethodAccessor) + * @see #registerAdditionalMethodAccessors(Container) + */ @Inject protected void setContainer(Container container) throws ClassNotFoundException { - Set names = container.getInstanceNames(PropertyAccessor.class); + this.container = container; + registerPropertyAccessors(container); + registerNullHandlers(container); + registerAdditionalMethodAccessors(container); + } + + /** + * Note that the default {@link MethodAccessor} for handling {@link Object} methods is registered in + * {@link #setMethodAccessor} and can be configured using the extension point + * {@link StrutsConstants#STRUTS_METHOD_ACCESSOR}. + */ + protected void registerAdditionalMethodAccessors(Container container) { + Set names = container.getInstanceNames(MethodAccessor.class); + for (String name : names) { + Class cls; + try { + cls = Class.forName(name); + if (cls.equals(Object.class)) { + // The Object method accessor can only be configured using the struts.methodAccessor extension point + continue; + } + if (cls.equals(CompoundRoot.class)) { + // TODO: This bean is deprecated, please remove this if statement when removing the struts-beans.xml entry + continue; + } + } catch (ClassNotFoundException e) { + // Since this interface is also used as an extension point for the Object MethodAccessor, we expect + // there to be beans with names that don't correspond to classes. We can safely ignore these. + continue; + } + MethodAccessor methodAccessor = container.getInstance(MethodAccessor.class, name); + OgnlRuntime.setMethodAccessor(cls, methodAccessor); + LOG.info("Registered custom OGNL MethodAccessor [{}] for class [{}]", methodAccessor.getClass().getName(), cls.getName()); + } + } + + protected void registerNullHandlers(Container container) throws ClassNotFoundException { + Set names = container.getInstanceNames(NullHandler.class); for (String name : names) { Class cls = Class.forName(name); - OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name)); + NullHandler nullHandler = container.getInstance(NullHandler.class, name); + OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(nullHandler)); + LOG.info("Registered custom OGNL NullHandler [{}] for class [{}]", nullHandler.getClass().getName(), cls.getName()); } + } - names = container.getInstanceNames(NullHandler.class); + protected void registerPropertyAccessors(Container container) throws ClassNotFoundException { + Set names = container.getInstanceNames(PropertyAccessor.class); for (String name : names) { Class cls = Class.forName(name); - OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(container.getInstance(NullHandler.class, name))); + if (cls.equals(CompoundRoot.class)) { + // TODO: This bean is deprecated, please remove this if statement when removing the struts-beans.xml entry + continue; + } + PropertyAccessor propertyAccessor = container.getInstance(PropertyAccessor.class, name); + OgnlRuntime.setPropertyAccessor(cls, propertyAccessor); + LOG.info("Registered custom OGNL PropertyAccessor [{}] for class [{}]", propertyAccessor.getClass().getName(), cls.getName()); } - this.container = container; } /** From 503c8daa16616af263892300700e566c264d49d0 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sat, 6 Jan 2024 02:49:47 +1100 Subject: [PATCH 16/30] WW-5381 Remove unnecessary/confusing parameters --- .../xwork2/ognl/OgnlValueStackFactory.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java index 8d3139f2e2..f72ad4be8a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java @@ -96,14 +96,14 @@ protected ValueStack createValueStack(ValueStack stack, boolean useTextProvider) * The only exception is the {@link MethodAccessor} for the {@link Object} type which has its own extension point. * * @see #setMethodAccessor(MethodAccessor) - * @see #registerAdditionalMethodAccessors(Container) + * @see #registerAdditionalMethodAccessors() */ @Inject protected void setContainer(Container container) throws ClassNotFoundException { this.container = container; - registerPropertyAccessors(container); - registerNullHandlers(container); - registerAdditionalMethodAccessors(container); + registerPropertyAccessors(); + registerNullHandlers(); + registerAdditionalMethodAccessors(); } /** @@ -111,7 +111,7 @@ protected void setContainer(Container container) throws ClassNotFoundException { * {@link #setMethodAccessor} and can be configured using the extension point * {@link StrutsConstants#STRUTS_METHOD_ACCESSOR}. */ - protected void registerAdditionalMethodAccessors(Container container) { + protected void registerAdditionalMethodAccessors() { Set names = container.getInstanceNames(MethodAccessor.class); for (String name : names) { Class cls; @@ -136,7 +136,7 @@ protected void registerAdditionalMethodAccessors(Container container) { } } - protected void registerNullHandlers(Container container) throws ClassNotFoundException { + protected void registerNullHandlers() throws ClassNotFoundException { Set names = container.getInstanceNames(NullHandler.class); for (String name : names) { Class cls = Class.forName(name); @@ -146,7 +146,7 @@ protected void registerNullHandlers(Container container) throws ClassNotFoundExc } } - protected void registerPropertyAccessors(Container container) throws ClassNotFoundException { + protected void registerPropertyAccessors() throws ClassNotFoundException { Set names = container.getInstanceNames(PropertyAccessor.class); for (String name : names) { Class cls = Class.forName(name); From fb7fec3550a6e14ce2429961123bd6fa7ee98387 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 6 Jan 2024 15:11:01 +0100 Subject: [PATCH 17/30] Stops cleaning nightlies to allow to coexist different versions --- Jenkinsfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 22e608f4d4..1c62cdb5af 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -242,8 +242,7 @@ pipeline { sshTransfer( remoteDirectory: '/struts/snapshot', removePrefix: 'assembly/target/assembly/out', - sourceFiles: 'assembly/target/assembly/out/struts-*.zip', - cleanRemote: true + sourceFiles: 'assembly/target/assembly/out/struts-*.zip' ) ], verbose: true From 55ca7a5b345b00898f43e7dd5af61ea9268beba0 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 7 Jan 2024 11:37:10 +0100 Subject: [PATCH 18/30] WW-5365 Reverts changes introduced in WW-5192 to allow evaluate the value attribute --- .../org/apache/struts2/components/Radio.java | 8 ---- .../java/org/apache/struts2/TestAction.java | 9 +++++ .../struts2/views/jsp/ui/RadioTest.java | 37 ++++++++++++++++++- .../apache/struts2/views/jsp/ui/Radio-11.txt | 4 ++ 4 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-11.txt diff --git a/core/src/main/java/org/apache/struts2/components/Radio.java b/core/src/main/java/org/apache/struts2/components/Radio.java index 7ef81000ad..5c59e70ec5 100644 --- a/core/src/main/java/org/apache/struts2/components/Radio.java +++ b/core/src/main/java/org/apache/struts2/components/Radio.java @@ -66,10 +66,6 @@ protected String getDefaultTemplate() { return TEMPLATE; } - public void evaluateExtraParams() { - super.evaluateExtraParams(); - } - /** * Radio tag requires lazy evaluation as list of tags is dynamically generated using * @@ -80,8 +76,4 @@ protected boolean lazyEvaluation() { return true; } - protected Class getValueClassType() { - return String.class; - } - } diff --git a/core/src/test/java/org/apache/struts2/TestAction.java b/core/src/test/java/org/apache/struts2/TestAction.java index 5543b44623..77f784a618 100644 --- a/core/src/test/java/org/apache/struts2/TestAction.java +++ b/core/src/test/java/org/apache/struts2/TestAction.java @@ -53,6 +53,7 @@ public class TestAction extends ActionSupport { private Long id; private List enumList; private List intList; + private Boolean someBool; private final Map texts = new HashMap<>(); @@ -254,4 +255,12 @@ public List getIntList() { public void setIntList(List intList) { this.intList = intList; } + + public Boolean getSomeBool() { + return someBool; + } + + public void setSomeBool(Boolean someBool) { + this.someBool = someBool; + } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/RadioTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/RadioTest.java index 72e4cbf06c..47a5caad7b 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/RadioTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/RadioTest.java @@ -64,6 +64,37 @@ public void testMapWithBooleanAsKey() throws Exception { strutsBodyTagsAreReflectionEqual(tag, freshTag)); } + public void testMapWithBooleanAsKeyWithoutForceValue() throws Exception { + TestAction testAction = (TestAction) action; + + Map map = new LinkedHashMap<>(); + map.put(Boolean.TRUE, "male"); + map.put(Boolean.FALSE, "female"); + testAction.setMap(map); + + testAction.setSomeBool(false); + + RadioTag tag = new RadioTag(); + tag.setPageContext(pageContext); + tag.setLabel("mylabel"); + tag.setName("myname"); + tag.setValue("someBool"); + tag.setList("map"); + tag.setTheme("simple"); + + tag.doStartTag(); + tag.doEndTag(); + + verify(RadioTag.class.getResource("Radio-11.txt")); + + // Basic sanity check of clearTagStateForTagPoolingServers() behaviour for Struts Tags after doEndTag(). + RadioTag freshTag = new RadioTag(); + freshTag.setPageContext(pageContext); + assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); + } + public void testMapWithBooleanAsKey_clearTagStateSet() throws Exception { TestAction testAction = (TestAction) action; @@ -165,12 +196,13 @@ public void testMapCheckedUsingEnum() throws Exception { List enumList = new ArrayList<>(Arrays.asList(SomeEnum.values())); testAction.setEnumList(enumList); + testAction.setStatus(SomeEnum.INIT); RadioTag tag = new RadioTag(); tag.setTheme("simple"); tag.setPageContext(pageContext); tag.setName("status"); - tag.setValue("INIT"); + tag.setValue("status"); tag.setList("enumList"); tag.doStartTag(); @@ -191,13 +223,14 @@ public void testMapCheckedUsingEnum_clearTagStateSet() throws Exception { List enumList = new ArrayList<>(Arrays.asList(SomeEnum.values())); testAction.setEnumList(enumList); + testAction.setStatus(SomeEnum.INIT); RadioTag tag = new RadioTag(); tag.setPerformClearTagStateForTagPoolingServers(true); // Explicitly request tag state clearing. tag.setTheme("simple"); tag.setPageContext(pageContext); tag.setName("status"); - tag.setValue("INIT"); + tag.setValue("status"); tag.setList("enumList"); tag.doStartTag(); diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-11.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-11.txt new file mode 100644 index 0000000000..8f591387b9 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-11.txt @@ -0,0 +1,4 @@ + + + + From b9d072d18c165d31ddc000a3bc52593158f05f2c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Jan 2024 01:17:20 +0000 Subject: [PATCH 19/30] Bump org.apache.maven.plugins:maven-release-plugin Bumps [org.apache.maven.plugins:maven-release-plugin](https://github.com/apache/maven-release) from 3.0.0-M1 to 3.0.1. - [Release notes](https://github.com/apache/maven-release/releases) - [Commits](https://github.com/apache/maven-release/compare/maven-release-3.0.0-M1...maven-release-3.0.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-release-plugin dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f059cdb217..679be82168 100644 --- a/pom.xml +++ b/pom.xml @@ -389,7 +389,7 @@ org.apache.maven.plugins maven-release-plugin - 3.0.0-M1 + 3.0.1 maven-jar-plugin From 5b020eb570efb91cc345973b277ac0affec86871 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 8 Jan 2024 07:11:35 +0100 Subject: [PATCH 20/30] Reduces log level to debug to reduce noise in the logs --- .../com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java index f72ad4be8a..2910d40a69 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java @@ -132,7 +132,7 @@ protected void registerAdditionalMethodAccessors() { } MethodAccessor methodAccessor = container.getInstance(MethodAccessor.class, name); OgnlRuntime.setMethodAccessor(cls, methodAccessor); - LOG.info("Registered custom OGNL MethodAccessor [{}] for class [{}]", methodAccessor.getClass().getName(), cls.getName()); + LOG.debug("Registered custom OGNL MethodAccessor [{}] for class [{}]", methodAccessor.getClass().getName(), cls.getName()); } } @@ -142,7 +142,7 @@ protected void registerNullHandlers() throws ClassNotFoundException { Class cls = Class.forName(name); NullHandler nullHandler = container.getInstance(NullHandler.class, name); OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(nullHandler)); - LOG.info("Registered custom OGNL NullHandler [{}] for class [{}]", nullHandler.getClass().getName(), cls.getName()); + LOG.debug("Registered custom OGNL NullHandler [{}] for class [{}]", nullHandler.getClass().getName(), cls.getName()); } } @@ -156,7 +156,7 @@ protected void registerPropertyAccessors() throws ClassNotFoundException { } PropertyAccessor propertyAccessor = container.getInstance(PropertyAccessor.class, name); OgnlRuntime.setPropertyAccessor(cls, propertyAccessor); - LOG.info("Registered custom OGNL PropertyAccessor [{}] for class [{}]", propertyAccessor.getClass().getName(), cls.getName()); + LOG.debug("Registered custom OGNL PropertyAccessor [{}] for class [{}]", propertyAccessor.getClass().getName(), cls.getName()); } } From 3c8fff46d4a6e9cdd5460b527b25f847d5f524d3 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 14 Jan 2024 16:37:14 +1100 Subject: [PATCH 21/30] WW-5352 Clean up OgnlValueStackTest --- .../xwork2/ognl/OgnlValueStackTest.java | 123 ++++++++---------- 1 file changed, 51 insertions(+), 72 deletions(-) diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java index ebd8096749..ad5a301ad2 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java @@ -90,43 +90,37 @@ public static Integer staticInteger100Method() { @Override public void setUp() throws Exception { super.setUp(); - ognlUtil = container.getInstance(OgnlUtil.class); - vs = createValueStack(true); - } - - private OgnlValueStack createValueStack(boolean allowStaticFieldAccess) { - OgnlValueStack stack = new OgnlValueStack( - container.getInstance(XWorkConverter.class), - (CompoundRootAccessor) container.getInstance(RootAccessor.class), - container.getInstance(TextProvider.class, "system"), allowStaticFieldAccess); - container.inject(stack); - return stack; + refreshContainerFields(); } - /** - * @return current OgnlValueStackFactory instance from current container - */ - private OgnlValueStackFactory getValueStackFactory() { - return (OgnlValueStackFactory) container.getInstance(ValueStackFactory.class); + protected void refreshContainerFields() { + ognlUtil = container.getInstance(OgnlUtil.class); + vs = (OgnlValueStack) container.getInstance(ValueStackFactory.class).createValueStack(); } /** - * Reloads container and gets a new OgnlValueStackFactory with specified new configuration. + * Reloads container and sets a new OgnlValueStackFactory with specified new configuration. * Intended for testing OgnlValueStack instance(s) that are minimally configured. * This should help ensure no underlying configuration/injection side-effects are responsible * for the behaviour of fundamental access control flags). * * @param allowStaticField new allowStaticField configuration - * @return a new OgnlValueStackFactory with specified new configuration */ - private OgnlValueStackFactory reloadValueStackFactory(boolean allowStaticField) { - try { - reloadTestContainerConfiguration(allowStaticField); - } catch (Exception ex) { - fail("Unable to reload container configuration and configure ognlValueStackFactory - exception: " + ex); - } + private void reloadTestContainerConfiguration(boolean allowStaticField) { + Map properties = new HashMap<>(); + properties.put(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Boolean.toString(allowStaticField)); + reloadTestContainerConfiguration(properties); + } - return getValueStackFactory(); + private void reloadTestContainerConfiguration(Map properties) { + loadConfigurationProviders(new StubConfigurationProvider() { + @Override + public void register(ContainerBuilder builder, + LocatableProperties props) throws ConfigurationException { + properties.forEach(props::setProperty); + } + }); + refreshContainerFields(); } public void testExpOverridesCanStackExpUp() throws Exception { @@ -1159,31 +1153,29 @@ public void testWarnAboutInvalidProperties() { * when a default configuration is used. */ public void testOgnlValueStackFromOgnlValueStackFactoryDefaultConfig() throws IllegalAccessException { - OgnlValueStackFactory ognlValueStackFactory = getValueStackFactory(); - OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); Object accessedValue; assertTrue("OgnlValueStackFactory staticFieldAccess (default flags) not true?", - reflectField(ognlValueStack.securityMemberAccess, "allowStaticFieldAccess")); + reflectField(vs.securityMemberAccess, "allowStaticFieldAccess")); // An OgnlValueStack created from the above OgnlValueStackFactory should allow public field access, // but prevent non-public field access. It should also deny static method access. - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); assertNull("able to access static method (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); assertEquals("accessed static final public field value not equal to actual?", accessedValue, STATIC_FINAL_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); assertEquals("accessed static public field value not equal to actual?", accessedValue, STATIC_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); assertNull("accessed final package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); assertNull("accessed package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); assertNull("accessed final protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); assertNull("accessed protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); assertNull("accessed final private field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); assertNull("accessed private field (result not null) ?", accessedValue); } @@ -1192,31 +1184,30 @@ public void testOgnlValueStackFromOgnlValueStackFactoryDefaultConfig() throws Il * when static access flag is set to false. */ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() throws IllegalAccessException { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(false); - OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); + reloadTestContainerConfiguration(false); Object accessedValue; assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not false?", - reflectField(ognlValueStack.securityMemberAccess, "allowStaticFieldAccess")); + reflectField(vs.securityMemberAccess, "allowStaticFieldAccess")); // An OgnlValueStack created from the above OgnlValueStackFactory should prevent public field access, // and prevent non-public field access. It should also deny static method access. - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); assertNull("able to access static method (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); assertNull("able to access static final public field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); assertNull("able to access static public field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); assertNull("accessed final package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); assertNull("accessed package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); assertNull("accessed final protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); assertNull("accessed protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); assertNull("accessed final private field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); assertNull("accessed private field (result not null) ?", accessedValue); } @@ -1225,45 +1216,33 @@ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() throws I * when static access flag is set to true. */ public void testOgnlValueStackFromOgnlValueStackFactoryAllStaticAccess() throws IllegalAccessException { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(true); - OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); + reloadTestContainerConfiguration(true); Object accessedValue; assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not true?", - reflectField(ognlValueStack.securityMemberAccess, "allowStaticFieldAccess")); + reflectField(vs.securityMemberAccess, "allowStaticFieldAccess")); // An OgnlValueStack created from the above OgnlValueStackFactory should allow public field access, // but prevent non-public field access. It should also allow static method access. - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); assertNull("able to access static method (result non-null)!!!", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); assertEquals("accessed static final public field value not equal to actual?", accessedValue, STATIC_FINAL_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); assertEquals("accessed static public field value not equal to actual?", accessedValue, STATIC_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); assertNull("accessed final package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); assertNull("accessed package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); assertNull("accessed final protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); assertNull("accessed protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); assertNull("accessed final private field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); + accessedValue = vs.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); assertNull("accessed private field (result not null) ?", accessedValue); } - private void reloadTestContainerConfiguration(boolean allowStaticField) throws Exception { - loadConfigurationProviders(new StubConfigurationProvider() { - @Override - public void register(ContainerBuilder builder, - LocatableProperties props) throws ConfigurationException { - props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, String.valueOf(allowStaticField)); - } - }); - ognlUtil = container.getInstance(OgnlUtil.class); - } - static class BadJavaBean { private int count; private int count2; From 9b5cb2d7b5c12d3e0041e62d1d591290456cfb3e Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 14 Jan 2024 18:08:56 +1100 Subject: [PATCH 22/30] WW-5352 Move method to XWorkTestCase --- .../com/opensymphony/xwork2/XWorkTestCase.java | 10 ++++++++++ .../xwork2/ognl/OgnlValueStackTest.java | 18 ++++-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java b/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java index dc6bb25083..88790fc6f7 100644 --- a/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java +++ b/core/src/main/java/com/opensymphony/xwork2/XWorkTestCase.java @@ -73,6 +73,16 @@ protected void loadConfigurationProviders(ConfigurationProvider... providers) { actionProxyFactory = container.getInstance(ActionProxyFactory.class); } + protected void loadButSet(Map properties) { + loadConfigurationProviders(new StubConfigurationProvider() { + @Override + public void register(ContainerBuilder builder, + LocatableProperties props) throws ConfigurationException { + properties.forEach((k, v) -> props.setProperty(k, String.valueOf(v))); + } + }); + } + protected void loadButAdd(final Class type, final T impl) { loadButAdd(type, Container.DEFAULT_NAME, impl); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java index ad5a301ad2..3bdfd67fcc 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java @@ -106,20 +106,10 @@ protected void refreshContainerFields() { * * @param allowStaticField new allowStaticField configuration */ - private void reloadTestContainerConfiguration(boolean allowStaticField) { + private void reloadContainer(boolean allowStaticField) { Map properties = new HashMap<>(); properties.put(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Boolean.toString(allowStaticField)); - reloadTestContainerConfiguration(properties); - } - - private void reloadTestContainerConfiguration(Map properties) { - loadConfigurationProviders(new StubConfigurationProvider() { - @Override - public void register(ContainerBuilder builder, - LocatableProperties props) throws ConfigurationException { - properties.forEach(props::setProperty); - } - }); + loadButSet(properties); refreshContainerFields(); } @@ -1184,7 +1174,7 @@ public void testOgnlValueStackFromOgnlValueStackFactoryDefaultConfig() throws Il * when static access flag is set to false. */ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() throws IllegalAccessException { - reloadTestContainerConfiguration(false); + reloadContainer(false); Object accessedValue; assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not false?", @@ -1216,7 +1206,7 @@ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() throws I * when static access flag is set to true. */ public void testOgnlValueStackFromOgnlValueStackFactoryAllStaticAccess() throws IllegalAccessException { - reloadTestContainerConfiguration(true); + reloadContainer(true); Object accessedValue; assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not true?", From 686189c1b3b11512486c6e7d53f4e0ec6096d572 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jan 2024 01:30:13 +0000 Subject: [PATCH 23/30] Bump actions/upload-artifact from 4.0.0 to 4.1.0 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.0.0 to 4.1.0. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/c7d193f32edcb7bfad88892161225aeda64e9392...1eb3cb2b3e0f29609092a73eb033bb759a334595) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/scorecards-analysis.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/scorecards-analysis.yaml b/.github/workflows/scorecards-analysis.yaml index 193ccce82a..0fde102be0 100644 --- a/.github/workflows/scorecards-analysis.yaml +++ b/.github/workflows/scorecards-analysis.yaml @@ -57,7 +57,7 @@ jobs: publish_results: true - name: "Upload artifact" - uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 # 4.0.0 + uses: actions/upload-artifact@1eb3cb2b3e0f29609092a73eb033bb759a334595 # 4.1.0 with: name: SARIF file path: results.sarif From e9738698a465aebb69d83f89e80792bf4bb349e0 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 16 Jan 2024 20:36:00 +0100 Subject: [PATCH 24/30] WW-5387 Fixes remove() signature --- .../org/apache/struts2/dispatcher/ApplicationMap.java | 9 +++++++-- .../java/org/apache/struts2/dispatcher/RequestMap.java | 9 +++++++-- .../apache/struts2/portlet/PortletApplicationMap.java | 9 +++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java b/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java index f0c5b4bf23..6b9053338a 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java @@ -151,11 +151,16 @@ public Object put(final String key, final Object value) { * @param key the attribute to remove. * @return the entry that was just removed. */ - public Object remove(final String key) { + @Override + public Object remove(Object key) { + if (key == null) { + return null; + } + entries = null; Object value = get(key); - context.removeAttribute(key); + context.removeAttribute(key.toString()); return value; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java index b8e5b8e671..2af730d94a 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java @@ -125,11 +125,16 @@ public Object put(final String key, final Object value) { * @param key the name of the attribute to remove. * @return the value that was removed or null if the value was not found (and hence, not removed). */ - public Object remove(final String key) { + @Override + public Object remove(final Object key) { + if (key == null) { + return null; + } + entries = null; Object value = get(key); - request.removeAttribute(key); + request.removeAttribute(key.toString()); return value; } diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java index 701deb004b..236e0aa67c 100644 --- a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java +++ b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java @@ -197,11 +197,16 @@ public Object put(String key, Object value) { * the attribute to remove. * @return the entry that was just removed. */ - public Object remove(String key) { + @Override + public Object remove(Object key) { + if (key == null) { + return null; + } + entries = null; Object value = get(key); - context.removeAttribute(key); + context.removeAttribute(key.toString()); return value; } From dc96c257d4198f1b255728f878c48840de13a3f4 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 17 Jan 2024 13:22:09 +0100 Subject: [PATCH 25/30] WW-5374 Allows to prepend reportUri with Servlet context --- .../interceptor/csp/CspInterceptor.java | 42 ++++++++++++++----- .../interceptor/CspInterceptorTest.java | 37 +++++++++++----- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java index 8e4356646a..aca583a324 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java @@ -43,7 +43,8 @@ public final class CspInterceptor extends AbstractInterceptor { private static final Logger LOG = LogManager.getLogger(CspInterceptor.class); - private Boolean enforcingMode; + private boolean prependServletContext = true; + private boolean enforcingMode; private String reportUri; @Override @@ -60,17 +61,22 @@ public String intercept(ActionInvocation invocation) throws Exception { } private void applySettings(ActionInvocation invocation, CspSettings cspSettings) { - if (enforcingMode != null) { - LOG.trace("Applying: {} to enforcingMode", enforcingMode); - cspSettings.setEnforcingMode(enforcingMode); - } + HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); + HttpServletResponse response = invocation.getInvocationContext().getServletResponse(); + + LOG.trace("Applying: {} to enforcingMode", enforcingMode); + cspSettings.setEnforcingMode(enforcingMode); + if (reportUri != null) { LOG.trace("Applying: {} to reportUri", reportUri); - cspSettings.setReportUri(reportUri); - } + String finalReportUri = reportUri; - HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); - HttpServletResponse response = invocation.getInvocationContext().getServletResponse(); + if (prependServletContext && (request.getContextPath() != null) && (!request.getContextPath().isEmpty())) { + finalReportUri = request.getContextPath() + finalReportUri; + } + + cspSettings.setReportUri(finalReportUri); + } invocation.addPreResultListener((actionInvocation, resultCode) -> { LOG.trace("Applying CSP header: {} to the request", cspSettings); @@ -99,8 +105,22 @@ private Optional buildUri(String reportUri) { } } - public void setEnforcingMode(String value) { - this.enforcingMode = Boolean.parseBoolean(value); + /** + * Enables enforcing mode, by default all exceptions are only reported + * + * @param enforcingMode true to enable enforcing mode, false to keep reporting mode. + */ + public void setEnforcingMode(boolean enforcingMode) { + this.enforcingMode = enforcingMode; + } + + /** + * Sets whether to prepend the servlet context path to the {@link #reportUri}. + * + * @param prependServletContext true to prepend the location with the servlet context path, false otherwise. + */ + public void setPrependServletContext(boolean prependServletContext) { + this.prependServletContext = prependServletContext; } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java index 2811b289f2..38ef25b823 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java @@ -22,6 +22,7 @@ import com.opensymphony.xwork2.mock.MockActionInvocation; import org.apache.logging.log4j.util.Strings; import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.TestAction; import org.apache.struts2.action.CspSettingsAware; import org.apache.struts2.dispatcher.SessionMap; import org.apache.struts2.interceptor.csp.CspInterceptor; @@ -45,7 +46,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase { public void test_whenRequestReceived_thenNonceIsSetInSession_andCspHeaderContainsIt() throws Exception { String reportUri = "/barfoo"; - String reporting = "false"; + boolean reporting = false; interceptor.setReportUri(reportUri); interceptor.setEnforcingMode(reporting); @@ -58,7 +59,7 @@ public void test_whenRequestReceived_thenNonceIsSetInSession_andCspHeaderContain public void test_whenNonceAlreadySetInSession_andRequestReceived_thenNewNonceIsSet() throws Exception { String reportUri = "https://www.google.com/"; - String enforcingMode = "true"; + boolean enforcingMode = true; interceptor.setReportUri(reportUri); interceptor.setEnforcingMode(enforcingMode); session.setAttribute("nonce", "foo"); @@ -73,7 +74,7 @@ public void test_whenNonceAlreadySetInSession_andRequestReceived_thenNewNonceIsS public void testEnforcingCspHeadersSet() throws Exception { String reportUri = "/csp-reports"; - String enforcingMode = "true"; + boolean enforcingMode = true; interceptor.setReportUri(reportUri); interceptor.setEnforcingMode(enforcingMode); session.setAttribute("nonce", "foo"); @@ -88,7 +89,7 @@ public void testEnforcingCspHeadersSet() throws Exception { public void testReportingCspHeadersSet() throws Exception { String reportUri = "/csp-reports"; - String enforcingMode = "false"; + boolean enforcingMode = false; interceptor.setReportUri(reportUri); interceptor.setEnforcingMode(enforcingMode); session.setAttribute("nonce", "foo"); @@ -101,7 +102,7 @@ public void testReportingCspHeadersSet() throws Exception { } public void test_uriSetOnlyWhenSetIsCalled() throws Exception { - String enforcingMode = "false"; + boolean enforcingMode = false; interceptor.setEnforcingMode(enforcingMode); interceptor.intercept(mai); @@ -115,7 +116,7 @@ public void test_uriSetOnlyWhenSetIsCalled() throws Exception { } public void testCannotParseUri() { - String enforcingMode = "false"; + boolean enforcingMode = false; interceptor.setEnforcingMode(enforcingMode); try { @@ -127,7 +128,7 @@ public void testCannotParseUri() { } public void testCannotParseRelativeUri() { - String enforcingMode = "false"; + boolean enforcingMode = false; interceptor.setEnforcingMode(enforcingMode); try { @@ -139,13 +140,27 @@ public void testCannotParseRelativeUri() { } public void testCustomPreResultListener() throws Exception { + boolean enforcingMode = false; mai.setAction(new CustomerCspAction("/report-uri")); - interceptor.setEnforcingMode("false"); + interceptor.setEnforcingMode(enforcingMode); + interceptor.intercept(mai); + checkHeader("/report-uri", enforcingMode); + } + + public void testPrependContext() throws Exception { + boolean enforcingMode = true; + mai.setAction(new TestAction()); + request.setContextPath("/app"); + + interceptor.setEnforcingMode(enforcingMode); + interceptor.setReportUri("/report-uri"); + interceptor.intercept(mai); - checkHeader("/report-uri", "false"); + + checkHeader("/app/report-uri", enforcingMode); } - public void checkHeader(String reportUri, String enforcingMode) { + public void checkHeader(String reportUri, boolean enforcingMode) { String expectedCspHeader; if (Strings.isEmpty(reportUri)) { expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; ", @@ -163,7 +178,7 @@ public void checkHeader(String reportUri, String enforcingMode) { } String header; - if (enforcingMode.equals("true")) { + if (enforcingMode) { header = response.getHeader(CspSettings.CSP_ENFORCE_HEADER); } else { header = response.getHeader(CspSettings.CSP_REPORT_HEADER); From 72f551f40baa7c96b614b6e9f2c6ce92dd7b103e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 18 Jan 2024 06:48:12 +0100 Subject: [PATCH 26/30] WW-5369 Re-define minimal library set --- assembly/src/main/assembly/min-lib.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/assembly/src/main/assembly/min-lib.xml b/assembly/src/main/assembly/min-lib.xml index 3cae96356b..a88b175fde 100644 --- a/assembly/src/main/assembly/min-lib.xml +++ b/assembly/src/main/assembly/min-lib.xml @@ -41,6 +41,8 @@ ognl:ognl commons-fileupload:commons-fileupload org.apache.commons:commons-io + com.github.ben-manes.caffeine:caffeine + org.javassist:javassist From d5932f82faf914fe16c680ba3479e55195fe5207 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 18 Jan 2024 11:24:08 +0100 Subject: [PATCH 27/30] WW-5374 Uses @code instead of --- .../org/apache/struts2/interceptor/csp/CspInterceptor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java index aca583a324..32d6777869 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java @@ -108,7 +108,7 @@ private Optional buildUri(String reportUri) { /** * Enables enforcing mode, by default all exceptions are only reported * - * @param enforcingMode true to enable enforcing mode, false to keep reporting mode. + * @param enforcingMode {@code true} to enable enforcing mode, {@code false} to keep reporting mode. */ public void setEnforcingMode(boolean enforcingMode) { this.enforcingMode = enforcingMode; @@ -117,7 +117,8 @@ public void setEnforcingMode(boolean enforcingMode) { /** * Sets whether to prepend the servlet context path to the {@link #reportUri}. * - * @param prependServletContext true to prepend the location with the servlet context path, false otherwise. + * @param prependServletContext {@code true} to prepend the location with the servlet context path, + * {@code false} otherwise. */ public void setPrependServletContext(boolean prependServletContext) { this.prependServletContext = prependServletContext; From 790c663ddd625c20dbbc35796e28a1c93562d4b4 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 18 Jan 2024 11:24:34 +0100 Subject: [PATCH 28/30] WW-5374 Adds additional test case to cover disabling prepending context --- .../struts2/interceptor/CspInterceptorTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java index 38ef25b823..0b03c6e54c 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java @@ -160,6 +160,20 @@ public void testPrependContext() throws Exception { checkHeader("/app/report-uri", enforcingMode); } + public void testNoPrependContext() throws Exception { + boolean enforcingMode = true; + mai.setAction(new TestAction()); + request.setContextPath("/app"); + + interceptor.setEnforcingMode(enforcingMode); + interceptor.setReportUri("/report-uri"); + interceptor.setPrependServletContext(false); + + interceptor.intercept(mai); + + checkHeader("/report-uri", enforcingMode); + } + public void checkHeader(String reportUri, boolean enforcingMode) { String expectedCspHeader; if (Strings.isEmpty(reportUri)) { From 3ec313aa0bbca1061531bac632b37e39ca5c58cc Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 19 Jan 2024 07:35:09 +0100 Subject: [PATCH 29/30] WW-5357 Adds support for disabled attribute to anchor tag --- .../main/resources/template/simple/a-close.ftl | 3 +++ .../apache/struts2/views/jsp/ui/AnchorTest.java | 17 +++++++++++++++++ .../org/apache/struts2/views/jsp/ui/href-6.txt | 1 + 3 files changed, 21 insertions(+) create mode 100644 core/src/test/resources/org/apache/struts2/views/jsp/ui/href-6.txt diff --git a/core/src/main/resources/template/simple/a-close.ftl b/core/src/main/resources/template/simple/a-close.ftl index 4cacaf5edd..1c808e18ba 100644 --- a/core/src/main/resources/template/simple/a-close.ftl +++ b/core/src/main/resources/template/simple/a-close.ftl @@ -25,6 +25,9 @@ <#if parameters.href??> href="${parameters.href?no_esc}"<#rt/> +<#if parameters.disabled!false> + disabled="disabled"<#rt/> + <#if parameters.tabindex??> tabindex="${parameters.tabindex}"<#rt/> diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/AnchorTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/AnchorTest.java index 7c3f80d715..9e45ed7a75 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/AnchorTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/AnchorTest.java @@ -276,6 +276,23 @@ public void testSimpleWithBodyNotHTMLEscaped() throws Exception { verifyResource("href-5.txt"); } + public void testSimpleDisabled() throws Exception { + createAction(); + + AnchorTag tag = createTag(); + tag.setHref("a"); + tag.setDisabled("true"); + + StrutsBodyContent body = new StrutsBodyContent(null); + body.print("should have disabled attribute"); + tag.setBodyContent(body); + + tag.doStartTag(); + tag.doEndTag(); + + verifyResource("href-6.txt"); + } + public void testInjectEscapeHtmlBodyFlag() throws Exception { // given initDispatcherWithConfigs("struts-default.xml, struts-escape-body.xml"); diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/href-6.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/href-6.txt new file mode 100644 index 0000000000..1e75ca9aac --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/href-6.txt @@ -0,0 +1 @@ +should have disabled attribute \ No newline at end of file From 9a6411c8c5661094b8b457cd5e9bbcd641c3443e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 19 Jan 2024 07:44:53 +0100 Subject: [PATCH 30/30] Extends sleep period to avoid breaking a build --- .../struts2/interceptor/exec/StrutsBackgroundProcessTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java index 331b5a7a9c..5906c995a6 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java @@ -112,7 +112,7 @@ public void testMultipleProcesses() throws InterruptedException { executor.execute(bp); } - Thread.sleep(500); + Thread.sleep(800); for (BackgroundProcess bp : bps) { assertTrue("Process is still active: " + bp, bp.isDone());