diff --git a/plugins/tiles/README.md b/plugins/tiles/README.md index 41a4fbe28b..7c2a98f392 100644 --- a/plugins/tiles/README.md +++ b/plugins/tiles/README.md @@ -4,3 +4,19 @@ You will find more details in [documentation](https://struts.apache.org/plugins/ ## Installation Just drop this plugin JAR into `WEB-INF/lib` folder or add it as a Maven dependency. + +## Legacy Tiles OGNL expressions + +The legacy Tiles `OGNL:` attribute-expression evaluator is deprecated in Struts 7.4.0 and disabled by default. Use +`S2:` for expressions that should be evaluated against the Struts ValueStack, or use an ordinary Tiles mechanism. + +Applications that temporarily require the legacy raw evaluator can set the following Struts constant: + +```xml + +``` + +The plugin resolves this constant from the current web application's Struts configuration on the first `OGNL:` +evaluation and caches the result for that evaluator lifecycle. Enabling the constant produces a one-time migration +warning when the legacy evaluator is first used. The compatibility constant is deprecated in Struts 7.4.0; both it +and the legacy evaluator are targeted for removal in Struts 8.0.0. diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluator.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluator.java new file mode 100644 index 0000000000..775f6bdfaa --- /dev/null +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluator.java @@ -0,0 +1,38 @@ +/* + * 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.tiles; + +import org.apache.tiles.core.evaluator.AbstractAttributeEvaluator; +import org.apache.tiles.core.evaluator.EvaluationException; +import org.apache.tiles.request.Request; + +/** + * Fails closed when the deprecated Tiles OGNL evaluator has not been explicitly enabled. + */ +final class DisabledOgnlAttributeEvaluator extends AbstractAttributeEvaluator { + + static final String DISABLED_MESSAGE = "The Tiles OGNL evaluator is disabled. Migrate the expression to S2:, " + + "or temporarily enable struts.tiles.ognl.legacy.enabled. Legacy Tiles OGNL support will be removed in " + + "Struts 8.0.0."; + + @Override + public Object evaluate(String expression, Request request) { + throw new EvaluationException(DISABLED_MESSAGE, null); + } +} diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java index 4111d1e7d5..636a2cfab7 100644 --- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java @@ -26,12 +26,15 @@ import jakarta.el.ListELResolver; import jakarta.el.MapELResolver; import jakarta.el.ResourceBundleELResolver; +import jakarta.servlet.ServletContext; import jakarta.servlet.jsp.JspFactory; import ognl.OgnlException; 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.dispatcher.Dispatcher; import org.apache.tiles.api.TilesContainer; import org.apache.tiles.core.definition.DefinitionsFactory; import org.apache.tiles.core.definition.pattern.DefinitionPatternMatcherFactory; @@ -39,6 +42,8 @@ import org.apache.tiles.core.definition.pattern.PrefixedPatternDefinitionResolver; import org.apache.tiles.core.definition.pattern.regexp.RegexpDefinitionPatternMatcherFactory; import org.apache.tiles.core.definition.pattern.wildcard.WildcardDefinitionPatternMatcherFactory; +import org.apache.tiles.core.evaluator.AbstractAttributeEvaluator; +import org.apache.tiles.core.evaluator.AttributeEvaluator; import org.apache.tiles.core.evaluator.AttributeEvaluatorFactory; import org.apache.tiles.core.evaluator.BasicAttributeEvaluatorFactory; import org.apache.tiles.core.evaluator.impl.DirectAttributeEvaluator; @@ -66,6 +71,8 @@ import org.apache.tiles.request.render.BasicRendererFactory; import org.apache.tiles.request.render.ChainedDelegateRenderer; import org.apache.tiles.request.render.Renderer; +import org.apache.tiles.request.servlet.NotAServletEnvironmentException; +import org.apache.tiles.request.servlet.ServletUtil; import java.util.ArrayList; import java.util.Collection; @@ -73,6 +80,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; /** * Dedicated Struts factory to build Tiles container with support for: @@ -89,6 +97,13 @@ public class StrutsTilesContainerFactory extends BasicTilesContainerFactory { private static final Logger LOG = LogManager.getLogger(StrutsTilesContainerFactory.class); + static final String LEGACY_OGNL_WARNING = "Legacy Tiles OGNL evaluation is enabled through " + + "struts.tiles.ognl.legacy.enabled. Migrate expressions to S2: or ordinary Tiles mechanisms; the " + + "compatibility flag and legacy evaluator will be removed in Struts 8.0.0."; + + private final Boolean legacyOgnlEnabled; + private final AtomicBoolean legacyOgnlWarningLogged = new AtomicBoolean(); + /** * The freemarker renderer name. */ @@ -113,6 +128,14 @@ public class StrutsTilesContainerFactory extends BasicTilesContainerFactory { public static final String S2 = "S2"; public static final String I18N = "I18N"; + public StrutsTilesContainerFactory() { + legacyOgnlEnabled = null; + } + + public StrutsTilesContainerFactory(boolean legacyOgnlEnabled) { + this.legacyOgnlEnabled = legacyOgnlEnabled; + } + @Override public TilesContainer createDecoratedContainer(TilesContainer originalContainer, ApplicationContext applicationContext) { return new CachingTilesContainer(originalContainer); @@ -155,7 +178,7 @@ protected AttributeEvaluatorFactory createAttributeEvaluatorFactory( BasicAttributeEvaluatorFactory attributeEvaluatorFactory = new BasicAttributeEvaluatorFactory(new DirectAttributeEvaluator()); attributeEvaluatorFactory.registerAttributeEvaluator(S2, createStrutsEvaluator()); attributeEvaluatorFactory.registerAttributeEvaluator(I18N, createI18NEvaluator()); - attributeEvaluatorFactory.registerAttributeEvaluator(OGNL, createOGNLEvaluator()); + attributeEvaluatorFactory.registerAttributeEvaluator(OGNL, createConfiguredOgnlEvaluator()); ELAttributeEvaluator elEvaluator = createELEvaluator(applicationContext); if (elEvaluator != null) { @@ -252,6 +275,45 @@ protected I18NAttributeEvaluator createI18NEvaluator() { return new I18NAttributeEvaluator(); } + private AttributeEvaluator createConfiguredOgnlEvaluator() { + if (legacyOgnlEnabled == null) { + return new ConfiguredOgnlAttributeEvaluator(); + } + return createOgnlEvaluator(legacyOgnlEnabled); + } + + private AttributeEvaluator createOgnlEvaluator(boolean enabled) { + if (enabled) { + if (legacyOgnlWarningLogged.compareAndSet(false, true)) { + logLegacyOgnlWarning(); + } + return createOGNLEvaluator(); + } + return new DisabledOgnlAttributeEvaluator(); + } + + @SuppressWarnings("removal") + boolean isLegacyOgnlEnabled(Request request) { + try { + ServletContext servletContext = ServletUtil.getServletRequest(request) + .getRequest().getServletContext(); + Dispatcher dispatcher = Dispatcher.getInstance(servletContext); + if (dispatcher == null) { + return false; + } + String configuredValue = dispatcher.getConfigurationManager().getConfiguration().getContainer().getInstance( + String.class, TilesConstants.STRUTS_TILES_OGNL_LEGACY_ENABLED); + return BooleanUtils.toBoolean(configuredValue); + } catch (NotAServletEnvironmentException ignored) { + return false; + } + } + + void logLegacyOgnlWarning() { + LOG.warn(LEGACY_OGNL_WARNING); + } + + @SuppressWarnings("removal") protected OGNLAttributeEvaluator createOGNLEvaluator() { try { PropertyAccessor objectPropertyAccessor = OgnlRuntime.getPropertyAccessor(Object.class); @@ -270,4 +332,28 @@ protected OGNLAttributeEvaluator createOGNLEvaluator() { } } + private final class ConfiguredOgnlAttributeEvaluator extends AbstractAttributeEvaluator { + + private volatile AttributeEvaluator delegate; + + @Override + public Object evaluate(String expression, Request request) { + return getDelegate(request).evaluate(expression, request); + } + + private AttributeEvaluator getDelegate(Request request) { + AttributeEvaluator result = delegate; + if (result == null) { + synchronized (this) { + result = delegate; + if (result == null) { + result = createOgnlEvaluator(isLegacyOgnlEnabled(request)); + delegate = result; + } + } + } + return result; + } + } + } diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesInitializer.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesInitializer.java index 03c1ebd8cf..53219c1112 100644 --- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesInitializer.java +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesInitializer.java @@ -32,6 +32,16 @@ public class StrutsTilesInitializer extends AbstractTilesInitializer { private static final Logger LOG = LogManager.getLogger(StrutsTilesInitializer.class); + private final Boolean legacyOgnlEnabled; + + public StrutsTilesInitializer() { + legacyOgnlEnabled = null; + } + + public StrutsTilesInitializer(boolean legacyOgnlEnabled) { + this.legacyOgnlEnabled = legacyOgnlEnabled; + } + @Override protected ApplicationContext createTilesApplicationContext(ApplicationContext preliminaryContext) { ServletContext servletContext = (ServletContext) preliminaryContext.getContext(); @@ -48,7 +58,10 @@ protected ApplicationContext createTilesApplicationContext(ApplicationContext pr @Override protected AbstractTilesContainerFactory createContainerFactory(ApplicationContext context) { LOG.trace("Creating dedicated Struts factory to create Tiles container"); - return new StrutsTilesContainerFactory(); + if (legacyOgnlEnabled == null) { + return new StrutsTilesContainerFactory(); + } + return new StrutsTilesContainerFactory(legacyOgnlEnabled); } } diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/TilesConstants.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/TilesConstants.java new file mode 100644 index 0000000000..44383047da --- /dev/null +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/TilesConstants.java @@ -0,0 +1,37 @@ +/* + * 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.tiles; + +/** + * Constants used by the Tiles plugin. + */ +public final class TilesConstants { + + /** + * Temporarily enables legacy raw Tiles OGNL evaluation. + * + * @deprecated Migrate Tiles expressions to {@code S2:} or ordinary Tiles mechanisms. This compatibility + * constant and the legacy evaluator are targeted for removal in Struts 8.0.0. + */ + @Deprecated(since = "7.4.0", forRemoval = true) + public static final String STRUTS_TILES_OGNL_LEGACY_ENABLED = "struts.tiles.ognl.legacy.enabled"; + + private TilesConstants() { + } +} diff --git a/plugins/tiles/src/main/java/org/apache/tiles/ognl/OGNLAttributeEvaluator.java b/plugins/tiles/src/main/java/org/apache/tiles/ognl/OGNLAttributeEvaluator.java index cac54fdcb2..59aaff5223 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/ognl/OGNLAttributeEvaluator.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/ognl/OGNLAttributeEvaluator.java @@ -28,10 +28,15 @@ * Evaluates attribute expressions and expressions with OGNL language. * * @since 2.2.0 + * @deprecated This legacy evaluator does not use the Struts OGNL controls used by {@code S2:} and is disabled by + * default. Temporary use requires {@code struts.tiles.ognl.legacy.enabled=true}. Migrate to {@code S2:} or ordinary + * Tiles mechanisms. This evaluator is targeted for removal in Struts 8.0.0. */ +@Deprecated(since = "7.4.0", forRemoval = true) public class OGNLAttributeEvaluator extends AbstractAttributeEvaluator { /** {@inheritDoc} */ + @Override public Object evaluate(String expression, Request request) { if (expression == null) { throw new IllegalArgumentException("The expression parameter cannot be null"); diff --git a/plugins/tiles/src/main/resources/struts-plugin.xml b/plugins/tiles/src/main/resources/struts-plugin.xml index 09d33f5fa4..df2152900b 100644 --- a/plugins/tiles/src/main/resources/struts-plugin.xml +++ b/plugins/tiles/src/main/resources/struts-plugin.xml @@ -24,6 +24,8 @@ "https://struts.apache.org/dtds/struts-6.0.dtd"> + + diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluatorTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluatorTest.java new file mode 100644 index 0000000000..01e31f5804 --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluatorTest.java @@ -0,0 +1,44 @@ +/* + * 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.tiles; + +import org.apache.tiles.api.Attribute; +import org.apache.tiles.api.Expression; +import org.apache.tiles.core.evaluator.EvaluationException; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; + +public class DisabledOgnlAttributeEvaluatorTest { + + @Test + public void failsClosedWithoutEvaluatingOrDisclosingExpression() { + String expression = "sensitive-marker.touch()"; + Attribute attribute = new Attribute(); + attribute.setExpressionObject(new Expression(expression)); + + EvaluationException exception = assertThrows(EvaluationException.class, + () -> new DisabledOgnlAttributeEvaluator().evaluate(attribute, null)); + + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, exception.getMessage()); + assertFalse(exception.getMessage().contains(expression)); + } +} diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java index 89d4b53975..aa92da82d5 100644 --- a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java @@ -18,8 +18,18 @@ */ package org.apache.struts2.tiles; +import ognl.OgnlException; +import ognl.OgnlRuntime; +import ognl.PropertyAccessor; +import org.apache.struts2.StrutsStatics; +import org.apache.struts2.config.Configuration; +import org.apache.struts2.config.ConfigurationManager; +import org.apache.struts2.dispatcher.Dispatcher; +import org.apache.struts2.inject.Container; import org.apache.tiles.api.TilesContainer; +import org.apache.tiles.core.evaluator.AttributeEvaluator; import org.apache.tiles.core.evaluator.AttributeEvaluatorFactory; +import org.apache.tiles.core.evaluator.EvaluationException; import org.apache.tiles.core.evaluator.impl.DirectAttributeEvaluator; import org.apache.tiles.core.locale.LocaleResolver; import org.apache.tiles.core.prepare.factory.BasicPreparerFactory; @@ -28,36 +38,59 @@ import org.apache.tiles.request.ApplicationContext; import org.apache.tiles.request.ApplicationResource; import org.apache.tiles.request.locale.URLApplicationResource; +import org.apache.tiles.request.servlet.ServletApplicationContext; +import org.apache.tiles.request.servlet.ServletRequest; import org.apache.tiles.request.render.BasicRendererFactory; import org.apache.tiles.request.render.ChainedDelegateRenderer; import org.apache.tiles.request.render.Renderer; +import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockServletContext; import jakarta.servlet.ServletContext; +import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.jsp.JspFactory; +import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@SuppressWarnings("removal") public class StrutsTilesContainerFactoryTest { private StrutsTilesContainerFactory factory; private ApplicationContext applicationContext; + private JspFactory originalJspFactory; @Before public void setUp() throws Exception { + originalJspFactory = JspFactory.getDefaultFactory(); applicationContext = mock(ApplicationContext.class); factory = new StrutsTilesContainerFactory(); } + @After + public void tearDown() { + JspFactory.setDefaultFactory(originalJspFactory); + } + @Test public void getSources() { ApplicationResource pathResource = new URLApplicationResource( @@ -78,22 +111,170 @@ public void getSources() { } @Test - public void createAttributeEvaluatorFactory() { + public void createAttributeEvaluatorFactoryDefersOgnlConfigurationUntilEvaluation() { + TrackingFactory trackingFactory = new TrackingFactory(); + PropertyAccessor requestAccessorBefore = getRequestAccessorOrNull(); LocaleResolver resolver = factory.createLocaleResolver(applicationContext); // explicitly disables support for EL JspFactory.setDefaultFactory(null); - AttributeEvaluatorFactory attributeEvaluatorFactory = factory.createAttributeEvaluatorFactory(applicationContext, resolver); + AttributeEvaluatorFactory attributeEvaluatorFactory = trackingFactory.createAttributeEvaluatorFactory(applicationContext, resolver); assertTrue("The class of the evaluator is not correct", attributeEvaluatorFactory.getAttributeEvaluator((String) null) instanceof DirectAttributeEvaluator); assertTrue("The class of the evaluator is not correct", attributeEvaluatorFactory.getAttributeEvaluator("S2") instanceof StrutsAttributeEvaluator); - assertTrue("The class of the evaluator is not correct", - attributeEvaluatorFactory.getAttributeEvaluator("OGNL") instanceof OGNLAttributeEvaluator); + AttributeEvaluator ognlEvaluator = attributeEvaluatorFactory.getAttributeEvaluator("OGNL"); assertTrue("The class of the evaluator is not correct", attributeEvaluatorFactory.getAttributeEvaluator("I18N") instanceof I18NAttributeEvaluator); assertTrue("The class of the evaluator is not correct", attributeEvaluatorFactory.getAttributeEvaluator("EL") instanceof DirectAttributeEvaluator); + assertEquals("The raw evaluator construction path must not run", 0, trackingFactory.rawEvaluatorCreations); + assertEquals("Configuration must not be resolved during construction", 0, trackingFactory.configurationResolutions); + assertSame("The default path must not mutate the shared Tiles Request accessor", + requestAccessorBefore, getRequestAccessorOrNull()); + + EvaluationException exception = assertThrows(EvaluationException.class, + () -> ognlEvaluator.evaluate("ignored", mock(org.apache.tiles.request.Request.class))); + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, exception.getMessage()); + assertEquals(1, trackingFactory.configurationResolutions); + assertEquals(0, trackingFactory.rawEvaluatorCreations); + assertSame("The disabled path must not mutate the shared Tiles Request accessor", + requestAccessorBefore, getRequestAccessorOrNull()); + } + + @Test + public void createAttributeEvaluatorFactoryEnablesLegacyOgnlExplicitly() throws OgnlException { + PropertyAccessor originalAccessor = getRequestAccessorOrNull(); + try { + TrackingFactory trackingFactory = new TrackingFactory(true); + LocaleResolver resolver = trackingFactory.createLocaleResolver(applicationContext); + JspFactory.setDefaultFactory(null); + + AttributeEvaluatorFactory attributeEvaluatorFactory = trackingFactory.createAttributeEvaluatorFactory( + applicationContext, resolver); + + assertTrue(attributeEvaluatorFactory.getAttributeEvaluator("OGNL") instanceof OGNLAttributeEvaluator); + assertEquals(1, trackingFactory.rawEvaluatorCreations); + assertTrue(OgnlRuntime.getPropertyAccessor(org.apache.tiles.request.Request.class) + instanceof org.apache.tiles.ognl.DelegatePropertyAccessor); + } finally { + OgnlRuntime.setPropertyAccessor(org.apache.tiles.request.Request.class, originalAccessor); + } + } + + @Test + public void publicFalseConstructorSelectsDisabledEvaluatorWithoutRequestLookup() { + TrackingFactory trackingFactory = new TrackingFactory(false); + JspFactory.setDefaultFactory(null); + AttributeEvaluatorFactory evaluators = trackingFactory.createAttributeEvaluatorFactory( + applicationContext, trackingFactory.createLocaleResolver(applicationContext)); + + EvaluationException exception = assertThrows(EvaluationException.class, + () -> evaluators.getAttributeEvaluator("OGNL").evaluate( + "ignored", mock(org.apache.tiles.request.Request.class))); + + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, exception.getMessage()); + assertEquals(0, trackingFactory.configurationResolutions); + assertEquals(0, trackingFactory.rawEvaluatorCreations); + } + + @Test + public void lazyConfigurationIsResolvedOnceAndLegacyEvaluatorIsReused() throws OgnlException { + PropertyAccessor originalAccessor = getRequestAccessorOrNull(); + try { + TrackingFactory trackingFactory = new TrackingFactory(); + trackingFactory.configuredLegacyOgnlEnabled = true; + JspFactory.setDefaultFactory(null); + AttributeEvaluatorFactory evaluators = trackingFactory.createAttributeEvaluatorFactory( + applicationContext, trackingFactory.createLocaleResolver(applicationContext)); + AttributeEvaluator evaluator = evaluators.getAttributeEvaluator("OGNL"); + + assertEquals(0, trackingFactory.configurationResolutions); + assertEquals(0, trackingFactory.rawEvaluatorCreations); + assertEquals(1, evaluator.evaluate("1", mock(org.apache.tiles.request.Request.class))); + assertEquals(2, evaluator.evaluate("2", mock(org.apache.tiles.request.Request.class))); + + assertEquals(1, trackingFactory.configurationResolutions); + assertEquals(1, trackingFactory.rawEvaluatorCreations); + assertEquals(1, trackingFactory.legacyWarningCount); + } finally { + OgnlRuntime.setPropertyAccessor(org.apache.tiles.request.Request.class, originalAccessor); + } + } + + @Test + public void lazyConfigurationIsResolvedOnceUnderConcurrentFirstUse() throws Exception { + TrackingFactory trackingFactory = new TrackingFactory(); + trackingFactory.blockConfigurationResolution = true; + JspFactory.setDefaultFactory(null); + AttributeEvaluator evaluator = trackingFactory.createAttributeEvaluatorFactory( + applicationContext, trackingFactory.createLocaleResolver(applicationContext)) + .getAttributeEvaluator("OGNL"); + ExecutorService executor = Executors.newFixedThreadPool(4); + try { + Future first = executor.submit(() -> assertThrows(EvaluationException.class, + () -> evaluator.evaluate("ignored", mock(org.apache.tiles.request.Request.class)))); + assertTrue(trackingFactory.configurationResolutionEntered.await(10, TimeUnit.SECONDS)); + Future second = executor.submit(() -> assertThrows(EvaluationException.class, + () -> evaluator.evaluate("ignored", mock(org.apache.tiles.request.Request.class)))); + Future third = executor.submit(() -> assertThrows(EvaluationException.class, + () -> evaluator.evaluate("ignored", mock(org.apache.tiles.request.Request.class)))); + trackingFactory.continueConfigurationResolution.countDown(); + first.get(10, TimeUnit.SECONDS); + second.get(10, TimeUnit.SECONDS); + third.get(10, TimeUnit.SECONDS); + + assertEquals(1, trackingFactory.configurationResolutions); + assertEquals(0, trackingFactory.rawEvaluatorCreations); + } finally { + executor.shutdownNow(); + } + } + + @Test + public void requestScopedConfigurationUsesNormalBooleanParsing() { + assertResolvedConfiguration("false", false); + assertResolvedConfiguration("TrUe", true); + assertResolvedConfiguration("not-a-boolean", false); + } + + @Test + public void customInitializerBooleanConstructorsArePublic() throws NoSuchMethodException { + assertTrue(Modifier.isPublic(StrutsTilesContainerFactory.class.getConstructor(boolean.class).getModifiers())); + assertTrue(Modifier.isPublic(StrutsTilesInitializer.class.getConstructor(boolean.class).getModifiers())); + } + + @Test + public void noArgInitializerPreservesLazyWebApplicationConfiguration() throws OgnlException { + PropertyAccessor originalAccessor = getRequestAccessorOrNull(); + try { + StrutsTilesContainerFactory initializedFactory = new ExposedInitializer().createFactory(applicationContext); + JspFactory.setDefaultFactory(null); + AttributeEvaluator evaluator = initializedFactory.createAttributeEvaluatorFactory( + applicationContext, initializedFactory.createLocaleResolver(applicationContext)) + .getAttributeEvaluator("OGNL"); + + assertEquals(1, evaluator.evaluate("1", createRequestWithConfiguredValue("true"))); + } finally { + OgnlRuntime.setPropertyAccessor(org.apache.tiles.request.Request.class, originalAccessor); + } + } + + @Test + public void legacyWarningIsLoggedOncePerFactoryConstruction() throws OgnlException { + PropertyAccessor originalAccessor = getRequestAccessorOrNull(); + try { + TrackingFactory trackingFactory = new TrackingFactory(true); + JspFactory.setDefaultFactory(null); + LocaleResolver resolver = trackingFactory.createLocaleResolver(applicationContext); + trackingFactory.createAttributeEvaluatorFactory(applicationContext, resolver); + trackingFactory.createAttributeEvaluatorFactory(applicationContext, resolver); + + assertEquals(1, trackingFactory.legacyWarningCount); + assertEquals(StrutsTilesContainerFactory.LEGACY_OGNL_WARNING, trackingFactory.legacyWarningMessage); + } finally { + OgnlRuntime.setPropertyAccessor(org.apache.tiles.request.Request.class, originalAccessor); + } } @Test @@ -125,4 +306,87 @@ public void createDefaultAttributeRenderer() { verify(rendererFactory).getRenderer("freemarker"); } -} \ No newline at end of file + private static PropertyAccessor getRequestAccessorOrNull() { + try { + return OgnlRuntime.getPropertyAccessor(org.apache.tiles.request.Request.class); + } catch (OgnlException ignored) { + return null; + } + } + + private void assertResolvedConfiguration(String configuredValue, boolean expected) { + assertEquals(expected, factory.isLegacyOgnlEnabled(createRequestWithConfiguredValue(configuredValue))); + } + + private ServletRequest createRequestWithConfiguredValue(String configuredValue) { + MockServletContext servletContext = new MockServletContext(); + Dispatcher dispatcher = mock(Dispatcher.class); + ConfigurationManager configurationManager = mock(ConfigurationManager.class); + Configuration configuration = mock(Configuration.class); + Container container = mock(Container.class); + when(dispatcher.getConfigurationManager()).thenReturn(configurationManager); + when(configurationManager.getConfiguration()).thenReturn(configuration); + when(configuration.getContainer()).thenReturn(container); + when(container.getInstance(String.class, TilesConstants.STRUTS_TILES_OGNL_LEGACY_ENABLED)) + .thenReturn(configuredValue); + servletContext.setAttribute(StrutsStatics.SERVLET_DISPATCHER, dispatcher); + ServletApplicationContext servletApplicationContext = new ServletApplicationContext(servletContext); + return new ServletRequest(servletApplicationContext, + new MockHttpServletRequest(servletContext), mock(HttpServletResponse.class)); + } + + private static class ExposedInitializer extends StrutsTilesInitializer { + + private StrutsTilesContainerFactory createFactory(ApplicationContext context) { + return (StrutsTilesContainerFactory) createContainerFactory(context); + } + } + + private static class TrackingFactory extends StrutsTilesContainerFactory { + private int rawEvaluatorCreations; + private int legacyWarningCount; + private String legacyWarningMessage; + private final AtomicInteger resolutionCounter = new AtomicInteger(); + private final CountDownLatch configurationResolutionEntered = new CountDownLatch(1); + private final CountDownLatch continueConfigurationResolution = new CountDownLatch(1); + private boolean configuredLegacyOgnlEnabled; + private boolean blockConfigurationResolution; + private int configurationResolutions; + + private TrackingFactory() { + super(); + } + + private TrackingFactory(boolean legacyOgnlEnabled) { + super(legacyOgnlEnabled); + } + + @Override + boolean isLegacyOgnlEnabled(org.apache.tiles.request.Request request) { + configurationResolutions = resolutionCounter.incrementAndGet(); + configurationResolutionEntered.countDown(); + if (blockConfigurationResolution) { + try { + assertTrue(continueConfigurationResolution.await(10, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + } + return configuredLegacyOgnlEnabled; + } + + @Override + protected OGNLAttributeEvaluator createOGNLEvaluator() { + rawEvaluatorCreations++; + return super.createOGNLEvaluator(); + } + + @Override + void logLegacyOgnlWarning() { + legacyWarningCount++; + legacyWarningMessage = LEGACY_OGNL_WARNING; + } + } + +} diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesListenerTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesListenerTest.java new file mode 100644 index 0000000000..2c52bcc04c --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesListenerTest.java @@ -0,0 +1,31 @@ +/* + * 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.tiles; + +import org.apache.struts2.dispatcher.DispatcherListener; +import org.junit.Test; +import static org.junit.Assert.assertFalse; + +public class StrutsTilesListenerTest { + + @Test + public void doesNotParticipateInDispatcherLifecycle() { + assertFalse(DispatcherListener.class.isAssignableFrom(StrutsTilesListener.class)); + } +} diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesOgnlEvaluatorIntegrationTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesOgnlEvaluatorIntegrationTest.java new file mode 100644 index 0000000000..4373a25e08 --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesOgnlEvaluatorIntegrationTest.java @@ -0,0 +1,285 @@ +/* + * 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.tiles; + +import ognl.OgnlException; +import ognl.OgnlRuntime; +import ognl.PropertyAccessor; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.inject.Container; +import org.apache.struts2.util.StrutsTestCaseHelper; +import org.apache.struts2.util.ValueStack; +import org.apache.struts2.util.ValueStackFactory; +import org.apache.tiles.api.Attribute; +import org.apache.tiles.api.Expression; +import org.apache.tiles.core.evaluator.AttributeEvaluatorFactory; +import org.apache.tiles.core.evaluator.EvaluationException; +import org.apache.tiles.core.impl.BasicTilesContainer; +import org.apache.tiles.request.ApplicationContext; +import org.apache.tiles.request.Request; +import org.apache.tiles.request.servlet.ServletApplicationContext; +import org.apache.tiles.request.servlet.ServletRequest; +import org.apache.tiles.request.render.BasicRendererFactory; +import org.apache.tiles.request.render.StringRenderer; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockServletContext; + +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.jsp.JspFactory; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * Regression coverage for the two Tiles expression-language registrations. + */ +@SuppressWarnings("removal") +public class TilesOgnlEvaluatorIntegrationTest { + + private org.apache.struts2.dispatcher.Dispatcher dispatcher; + private Container strutsContainer; + private ValueStack valueStack; + private Request tilesRequest; + private BasicTilesContainer tilesContainer; + private BasicRendererFactory rendererFactory; + private StringWriter renderedOutput; + + @Before + public void setUp() throws Exception { + MockServletContext servletContext = new MockServletContext(); + dispatcher = StrutsTestCaseHelper.initDispatcher(servletContext, Map.of( + StrutsConstants.STRUTS_ALLOWLIST_ENABLE, Boolean.TRUE.toString() + )); + strutsContainer = dispatcher.getContainer(); + valueStack = strutsContainer.getInstance(ValueStackFactory.class).createValueStack(); + + MockHttpServletRequest servletRequest = new MockHttpServletRequest(servletContext); + servletRequest.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, valueStack); + ApplicationContext applicationContext = new ServletApplicationContext(servletContext); + HttpServletResponse servletResponse = mock(HttpServletResponse.class); + renderedOutput = new StringWriter(); + org.mockito.Mockito.when(servletResponse.getWriter()).thenReturn(new PrintWriter(renderedOutput)); + tilesRequest = new ServletRequest(applicationContext, servletRequest, servletResponse); + + StrutsTilesContainerFactory factory = new StrutsTilesContainerFactory(); + AttributeEvaluatorFactory evaluators = createAttributeEvaluatorFactoryWithoutEl(factory, applicationContext); + rendererFactory = new BasicRendererFactory(); + rendererFactory.registerRenderer("string", new StringRenderer()); + tilesContainer = new BasicTilesContainer(); + tilesContainer.setAttributeEvaluatorFactory(evaluators); + tilesContainer.setRendererFactory(rendererFactory); + } + + @After + public void tearDown() { + StrutsTestCaseHelper.tearDown(dispatcher); + } + + @Test + public void ognlLanguageFailsClosedWithoutEvaluatingExpression() { + Marker marker = new Marker(); + tilesRequest.getContext("request").put("marker", marker); + + Attribute attribute = expression("marker.touch()", StrutsTilesContainerFactory.OGNL); + + EvaluationException exception = assertThrows(EvaluationException.class, + () -> tilesContainer.evaluate(attribute, tilesRequest)); + + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, exception.getMessage()); + assertFalse("The disabled evaluator must not invoke a method from the expression", marker.touched); + assertFalse("The exception must not disclose the expression", exception.getMessage().contains("marker.touch()")); + } + + @Test + public void legacyOgnlLanguagePreservesRawEvaluationWhenExplicitlyEnabled() throws OgnlException { + PropertyAccessor originalAccessor = getRequestAccessorOrNull(); + org.apache.struts2.dispatcher.Dispatcher legacyDispatcher = null; + try { + EvaluationException disabled = assertThrows(EvaluationException.class, () -> tilesContainer.evaluate( + expression("marker.touch()", StrutsTilesContainerFactory.OGNL), tilesRequest)); + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, disabled.getMessage()); + + MockServletContext legacyServletContext = new MockServletContext(); + legacyDispatcher = StrutsTestCaseHelper.initDispatcher(legacyServletContext, Map.of( + "config", "struts-default.xml,org/apache/struts2/tiles/struts-tiles-legacy.xml", + StrutsConstants.STRUTS_ALLOWLIST_ENABLE, Boolean.TRUE.toString())); + assertEquals(Boolean.TRUE.toString(), legacyDispatcher.getConfigurationManager().getConfiguration() + .getContainer().getInstance( + String.class, TilesConstants.STRUTS_TILES_OGNL_LEGACY_ENABLED)); + ApplicationContext legacyApplicationContext = new ServletApplicationContext(legacyServletContext); + MockHttpServletRequest legacyServletRequest = new MockHttpServletRequest(legacyServletContext); + Request legacyTilesRequest = new ServletRequest( + legacyApplicationContext, legacyServletRequest, mock(HttpServletResponse.class)); + Marker marker = new Marker(); + legacyTilesRequest.getContext("request").put("marker", marker); + TrackingFactory legacyFactory = new TrackingFactory(); + BasicTilesContainer legacyTilesContainer = new BasicTilesContainer(); + legacyTilesContainer.setAttributeEvaluatorFactory(createAttributeEvaluatorFactoryWithoutEl( + legacyFactory, legacyApplicationContext)); + + assertEquals("The raw evaluator must not be built during Tiles construction", + 0, legacyFactory.rawEvaluatorCreations); + assertEquals("touched", legacyTilesContainer.evaluate( + expression("marker.touch()", StrutsTilesContainerFactory.OGNL), legacyTilesRequest)); + assertEquals("touched", legacyTilesContainer.evaluate( + expression("marker.touch()", StrutsTilesContainerFactory.OGNL), legacyTilesRequest)); + assertTrue("The explicitly enabled legacy evaluator must preserve existing behavior", marker.touched); + assertEquals("The compatibility constant must be resolved once per evaluator", 1, + legacyFactory.configurationResolutions); + assertEquals("The raw evaluator must be constructed once per evaluator", 1, + legacyFactory.rawEvaluatorCreations); + assertEquals("The migration warning must be emitted once per evaluator lifecycle", 1, + legacyFactory.legacyWarnings); + + EvaluationException stillDisabled = assertThrows(EvaluationException.class, () -> tilesContainer.evaluate( + expression("marker.touch()", StrutsTilesContainerFactory.OGNL), tilesRequest)); + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, stillDisabled.getMessage()); + } finally { + StrutsTestCaseHelper.tearDown(legacyDispatcher); + OgnlRuntime.setPropertyAccessor(Request.class, originalAccessor); + } + } + + @Test + public void missingDispatcherFailsClosedAndCachesTheDecision() { + MockServletContext servletContext = new MockServletContext(); + ApplicationContext applicationContext = new ServletApplicationContext(servletContext); + Request request = new ServletRequest( + applicationContext, new MockHttpServletRequest(servletContext), mock(HttpServletResponse.class)); + TrackingFactory factory = new TrackingFactory(); + AttributeEvaluatorFactory evaluators = createAttributeEvaluatorFactoryWithoutEl(factory, applicationContext); + + EvaluationException first = assertThrows(EvaluationException.class, + () -> evaluators.getAttributeEvaluator(StrutsTilesContainerFactory.OGNL).evaluate("ignored", request)); + EvaluationException second = assertThrows(EvaluationException.class, + () -> evaluators.getAttributeEvaluator(StrutsTilesContainerFactory.OGNL).evaluate("ignored", request)); + + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, first.getMessage()); + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, second.getMessage()); + assertEquals(1, factory.configurationResolutions); + assertEquals(0, factory.rawEvaluatorCreations); + } + + @Test + public void nonServletRequestFailsClosedWithoutExposingEnvironmentFailure() { + TrackingFactory factory = new TrackingFactory(); + AttributeEvaluatorFactory evaluators = createAttributeEvaluatorFactoryWithoutEl( + factory, tilesRequest.getApplicationContext()); + + EvaluationException exception = assertThrows(EvaluationException.class, + () -> evaluators.getAttributeEvaluator(StrutsTilesContainerFactory.OGNL) + .evaluate("sensitive-expression", mock(Request.class))); + + assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE, exception.getMessage()); + assertFalse(exception.getMessage().contains("sensitive-expression")); + assertEquals(1, factory.configurationResolutions); + assertEquals(0, factory.rawEvaluatorCreations); + } + + @Test + public void ordinaryDirectTilesAttributeRemainsUnaffected() throws Exception { + Attribute attribute = new Attribute("ordinary value"); + attribute.setRenderer("string"); + + assertEquals("ordinary value", tilesContainer.evaluate(attribute, tilesRequest)); + tilesContainer.render(attribute, tilesRequest); + assertEquals("ordinary value", renderedOutput.toString()); + } + + @Test + public void s2LanguageStillEvaluatesAgainstSecuredValueStack() { + Map model = new HashMap<>(); + model.put("title", "secured title"); + valueStack.push(model); + + assertEquals( + "secured title", + tilesContainer.evaluate(expression("title", StrutsTilesContainerFactory.S2), tilesRequest) + ); + } + + private static PropertyAccessor getRequestAccessorOrNull() { + try { + return OgnlRuntime.getPropertyAccessor(Request.class); + } catch (OgnlException ignored) { + return null; + } + } + + private static AttributeEvaluatorFactory createAttributeEvaluatorFactoryWithoutEl( + StrutsTilesContainerFactory factory, ApplicationContext applicationContext) { + JspFactory originalJspFactory = JspFactory.getDefaultFactory(); + try { + JspFactory.setDefaultFactory(null); + return factory.createAttributeEvaluatorFactory( + applicationContext, factory.createLocaleResolver(applicationContext)); + } finally { + JspFactory.setDefaultFactory(originalJspFactory); + } + } + + private Attribute expression(String value, String language) { + Attribute attribute = new Attribute(); + attribute.setExpressionObject(new Expression(value, language)); + return attribute; + } + + public static class Marker { + private boolean touched; + + public String touch() { + touched = true; + return "touched"; + } + } + + private static class TrackingFactory extends StrutsTilesContainerFactory { + private int configurationResolutions; + private int rawEvaluatorCreations; + private int legacyWarnings; + + @Override + boolean isLegacyOgnlEnabled(Request request) { + configurationResolutions++; + return super.isLegacyOgnlEnabled(request); + } + + @Override + protected org.apache.tiles.ognl.OGNLAttributeEvaluator createOGNLEvaluator() { + rawEvaluatorCreations++; + return super.createOGNLEvaluator(); + } + + @Override + void logLegacyOgnlWarning() { + legacyWarnings++; + } + } +} diff --git a/plugins/tiles/src/test/resources/org/apache/struts2/tiles/struts-tiles-legacy.xml b/plugins/tiles/src/test/resources/org/apache/struts2/tiles/struts-tiles-legacy.xml new file mode 100644 index 0000000000..a9af2d28cd --- /dev/null +++ b/plugins/tiles/src/test/resources/org/apache/struts2/tiles/struts-tiles-legacy.xml @@ -0,0 +1,28 @@ + + + + + + +