This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new e213ef6c8 WW-5713 refactor(tiles): resolve the legacy OGNL flag from
the container's own ServletContext (#1961)
e213ef6c8 is described below
commit e213ef6c8b831cd460eab71db7ddd8a57f7b8110
Author: Lukasz Lenart <[email protected]>
AuthorDate: Thu Sep 17 07:50:53 2026 +0200
WW-5713 refactor(tiles): resolve the legacy OGNL flag from the container's
own ServletContext (#1961)
* WW-5713 refactor(tiles): resolve the legacy OGNL flag from the
container's own ServletContext
The lazy OGNL: evaluator read struts.tiles.ognl.legacy.enabled from the
ServletContext of whichever Tiles Request reached it first, walking the
request wrappers and catching NotAServletEnvironmentException. The Tiles
container is built for exactly one ApplicationContext, so capture that
context's ServletContext at construction and key the Dispatcher lookup to
it. A non-servlet ApplicationContext cannot carry a Dispatcher and now gets
the disabled evaluator outright.
Removal-version mentions are dropped from the Javadoc, the runtime
messages and the README; forRemoval = true and WW-5714 track the removal.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5713 test(tiles): keep a single throwing call inside the assertThrows
lambda
Sonar S5778 on nonServletApplicationContextFailsClosedWithoutLookup.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
plugins/tiles/README.md | 8 +--
.../tiles/DisabledOgnlAttributeEvaluator.java | 3 +-
.../struts2/tiles/StrutsTilesContainerFactory.java | 46 +++++++--------
.../org/apache/struts2/tiles/TilesConstants.java | 3 +-
.../apache/tiles/ognl/OGNLAttributeEvaluator.java | 6 +-
.../tiles/StrutsTilesContainerFactoryTest.java | 65 +++++++++++++++++-----
.../tiles/TilesOgnlEvaluatorIntegrationTest.java | 7 ++-
7 files changed, 89 insertions(+), 49 deletions(-)
diff --git a/plugins/tiles/README.md b/plugins/tiles/README.md
index 7c2a98f39..47dff6a38 100644
--- a/plugins/tiles/README.md
+++ b/plugins/tiles/README.md
@@ -16,7 +16,7 @@ Applications that temporarily require the legacy raw
evaluator can set the follo
<constant name="struts.tiles.ognl.legacy.enabled" value="true"/>
```
-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.
+The plugin resolves this constant from the Struts configuration of the web
application that owns the Tiles
+container, 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 and the
+legacy evaluator are deprecated for removal.
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
index 775f6bdfa..42a7db6e3 100644
---
a/plugins/tiles/src/main/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluator.java
+++
b/plugins/tiles/src/main/java/org/apache/struts2/tiles/DisabledOgnlAttributeEvaluator.java
@@ -28,8 +28,7 @@ import org.apache.tiles.request.Request;
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.";
+ + "or temporarily enable struts.tiles.ognl.legacy.enabled; legacy
Tiles OGNL support is deprecated for removal.";
@Override
public Object evaluate(String expression, Request request) {
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 636a2cfab..056f754cf 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
@@ -71,7 +71,7 @@ import org.apache.tiles.request.Request;
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.ServletApplicationContext;
import org.apache.tiles.request.servlet.ServletUtil;
import java.util.ArrayList;
@@ -99,7 +99,7 @@ public class StrutsTilesContainerFactory extends
BasicTilesContainerFactory {
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.";
+ + "compatibility flag and the legacy evaluator are deprecated for
removal.";
private final Boolean legacyOgnlEnabled;
private final AtomicBoolean legacyOgnlWarningLogged = new AtomicBoolean();
@@ -178,7 +178,7 @@ public class StrutsTilesContainerFactory extends
BasicTilesContainerFactory {
BasicAttributeEvaluatorFactory attributeEvaluatorFactory = new
BasicAttributeEvaluatorFactory(new DirectAttributeEvaluator());
attributeEvaluatorFactory.registerAttributeEvaluator(S2,
createStrutsEvaluator());
attributeEvaluatorFactory.registerAttributeEvaluator(I18N,
createI18NEvaluator());
- attributeEvaluatorFactory.registerAttributeEvaluator(OGNL,
createConfiguredOgnlEvaluator());
+ attributeEvaluatorFactory.registerAttributeEvaluator(OGNL,
createConfiguredOgnlEvaluator(applicationContext));
ELAttributeEvaluator elEvaluator =
createELEvaluator(applicationContext);
if (elEvaluator != null) {
@@ -275,11 +275,14 @@ public class StrutsTilesContainerFactory extends
BasicTilesContainerFactory {
return new I18NAttributeEvaluator();
}
- private AttributeEvaluator createConfiguredOgnlEvaluator() {
- if (legacyOgnlEnabled == null) {
- return new ConfiguredOgnlAttributeEvaluator();
+ private AttributeEvaluator
createConfiguredOgnlEvaluator(ApplicationContext applicationContext) {
+ if (legacyOgnlEnabled != null) {
+ return createOgnlEvaluator(legacyOgnlEnabled);
}
- return createOgnlEvaluator(legacyOgnlEnabled);
+ if (applicationContext instanceof ServletApplicationContext) {
+ return new
ConfiguredOgnlAttributeEvaluator(ServletUtil.getServletContext(applicationContext));
+ }
+ return new DisabledOgnlAttributeEvaluator();
}
private AttributeEvaluator createOgnlEvaluator(boolean enabled) {
@@ -293,20 +296,14 @@ public class StrutsTilesContainerFactory extends
BasicTilesContainerFactory {
}
@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) {
+ boolean isLegacyOgnlEnabled(ServletContext servletContext) {
+ 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);
}
void logLegacyOgnlWarning() {
@@ -334,20 +331,25 @@ public class StrutsTilesContainerFactory extends
BasicTilesContainerFactory {
private final class ConfiguredOgnlAttributeEvaluator extends
AbstractAttributeEvaluator {
+ private final ServletContext servletContext;
private volatile AttributeEvaluator delegate;
+ private ConfiguredOgnlAttributeEvaluator(ServletContext
servletContext) {
+ this.servletContext = servletContext;
+ }
+
@Override
public Object evaluate(String expression, Request request) {
- return getDelegate(request).evaluate(expression, request);
+ return getDelegate().evaluate(expression, request);
}
- private AttributeEvaluator getDelegate(Request request) {
+ private AttributeEvaluator getDelegate() {
AttributeEvaluator result = delegate;
if (result == null) {
synchronized (this) {
result = delegate;
if (result == null) {
- result =
createOgnlEvaluator(isLegacyOgnlEnabled(request));
+ result =
createOgnlEvaluator(isLegacyOgnlEnabled(servletContext));
delegate = result;
}
}
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
index 44383047d..3812713ed 100644
--- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/TilesConstants.java
+++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/TilesConstants.java
@@ -26,8 +26,7 @@ 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, migrate Tiles expressions to {@code S2:} or
ordinary Tiles mechanisms.
*/
@Deprecated(since = "7.4.0", forRemoval = true)
public static final String STRUTS_TILES_OGNL_LEGACY_ENABLED =
"struts.tiles.ognl.legacy.enabled";
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 59aaff522..0cd6e21e3 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,9 +28,9 @@ import org.apache.tiles.request.Request;
* 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, 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.
*/
@Deprecated(since = "7.4.0", forRemoval = true)
public class OGNLAttributeEvaluator extends AbstractAttributeEvaluator {
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 aa92da82d..882fefe04 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
@@ -77,12 +77,14 @@ public class StrutsTilesContainerFactoryTest {
private StrutsTilesContainerFactory factory;
private ApplicationContext applicationContext;
+ private ApplicationContext servletApplicationContext;
private JspFactory originalJspFactory;
@Before
public void setUp() throws Exception {
originalJspFactory = JspFactory.getDefaultFactory();
applicationContext = mock(ApplicationContext.class);
+ servletApplicationContext = new ServletApplicationContext(new
MockServletContext());
factory = new StrutsTilesContainerFactory();
}
@@ -118,7 +120,8 @@ public class StrutsTilesContainerFactoryTest {
// explicitly disables support for EL
JspFactory.setDefaultFactory(null);
- AttributeEvaluatorFactory attributeEvaluatorFactory =
trackingFactory.createAttributeEvaluatorFactory(applicationContext, resolver);
+ AttributeEvaluatorFactory attributeEvaluatorFactory =
trackingFactory.createAttributeEvaluatorFactory(
+ servletApplicationContext, 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",
@@ -186,7 +189,7 @@ public class StrutsTilesContainerFactoryTest {
trackingFactory.configuredLegacyOgnlEnabled = true;
JspFactory.setDefaultFactory(null);
AttributeEvaluatorFactory evaluators =
trackingFactory.createAttributeEvaluatorFactory(
- applicationContext,
trackingFactory.createLocaleResolver(applicationContext));
+ servletApplicationContext,
trackingFactory.createLocaleResolver(servletApplicationContext));
AttributeEvaluator evaluator =
evaluators.getAttributeEvaluator("OGNL");
assertEquals(0, trackingFactory.configurationResolutions);
@@ -208,7 +211,7 @@ public class StrutsTilesContainerFactoryTest {
trackingFactory.blockConfigurationResolution = true;
JspFactory.setDefaultFactory(null);
AttributeEvaluator evaluator =
trackingFactory.createAttributeEvaluatorFactory(
- applicationContext,
trackingFactory.createLocaleResolver(applicationContext))
+ servletApplicationContext,
trackingFactory.createLocaleResolver(servletApplicationContext))
.getAttributeEvaluator("OGNL");
ExecutorService executor = Executors.newFixedThreadPool(4);
try {
@@ -232,12 +235,48 @@ public class StrutsTilesContainerFactoryTest {
}
@Test
- public void requestScopedConfigurationUsesNormalBooleanParsing() {
+ public void configuredValueUsesNormalBooleanParsing() {
assertResolvedConfiguration("false", false);
assertResolvedConfiguration("TrUe", true);
assertResolvedConfiguration("not-a-boolean", false);
}
+ @Test
+ public void nonServletApplicationContextFailsClosedWithoutLookup() {
+ TrackingFactory trackingFactory = new TrackingFactory();
+ JspFactory.setDefaultFactory(null);
+ AttributeEvaluator evaluator =
trackingFactory.createAttributeEvaluatorFactory(
+ applicationContext,
trackingFactory.createLocaleResolver(applicationContext))
+ .getAttributeEvaluator("OGNL");
+ org.apache.tiles.request.Request request =
mock(org.apache.tiles.request.Request.class);
+
+ EvaluationException exception = assertThrows(EvaluationException.class,
+ () -> evaluator.evaluate("ignored", request));
+
+ assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE,
exception.getMessage());
+ assertEquals(0, trackingFactory.configurationResolutions);
+ assertEquals(0, trackingFactory.rawEvaluatorCreations);
+ }
+
+ @Test
+ public void flagIsReadFromTheOwningWebApplicationNotFromTheRequest() {
+ ApplicationContext owningApplicationContext = new
ServletApplicationContext(
+ createServletContextWithConfiguredValue("false"));
+ ServletContext neighbourServletContext =
createServletContextWithConfiguredValue("true");
+ org.apache.tiles.request.Request neighbourRequest = new ServletRequest(
+ new ServletApplicationContext(neighbourServletContext),
+ new MockHttpServletRequest(neighbourServletContext),
mock(HttpServletResponse.class));
+ JspFactory.setDefaultFactory(null);
+ AttributeEvaluator evaluator = factory.createAttributeEvaluatorFactory(
+ owningApplicationContext,
factory.createLocaleResolver(owningApplicationContext))
+ .getAttributeEvaluator("OGNL");
+
+ EvaluationException exception = assertThrows(EvaluationException.class,
+ () -> evaluator.evaluate("1", neighbourRequest));
+
+ assertEquals(DisabledOgnlAttributeEvaluator.DISABLED_MESSAGE,
exception.getMessage());
+ }
+
@Test
public void customInitializerBooleanConstructorsArePublic() throws
NoSuchMethodException {
assertTrue(Modifier.isPublic(StrutsTilesContainerFactory.class.getConstructor(boolean.class).getModifiers()));
@@ -248,13 +287,15 @@ public class StrutsTilesContainerFactoryTest {
public void noArgInitializerPreservesLazyWebApplicationConfiguration()
throws OgnlException {
PropertyAccessor originalAccessor = getRequestAccessorOrNull();
try {
- StrutsTilesContainerFactory initializedFactory = new
ExposedInitializer().createFactory(applicationContext);
+ ApplicationContext legacyApplicationContext = new
ServletApplicationContext(
+ createServletContextWithConfiguredValue("true"));
+ StrutsTilesContainerFactory initializedFactory = new
ExposedInitializer().createFactory(legacyApplicationContext);
JspFactory.setDefaultFactory(null);
AttributeEvaluator evaluator =
initializedFactory.createAttributeEvaluatorFactory(
- applicationContext,
initializedFactory.createLocaleResolver(applicationContext))
+ legacyApplicationContext,
initializedFactory.createLocaleResolver(legacyApplicationContext))
.getAttributeEvaluator("OGNL");
- assertEquals(1, evaluator.evaluate("1",
createRequestWithConfiguredValue("true")));
+ assertEquals(1, evaluator.evaluate("1",
mock(org.apache.tiles.request.Request.class)));
} finally {
OgnlRuntime.setPropertyAccessor(org.apache.tiles.request.Request.class,
originalAccessor);
}
@@ -315,10 +356,10 @@ public class StrutsTilesContainerFactoryTest {
}
private void assertResolvedConfiguration(String configuredValue, boolean
expected) {
- assertEquals(expected,
factory.isLegacyOgnlEnabled(createRequestWithConfiguredValue(configuredValue)));
+ assertEquals(expected,
factory.isLegacyOgnlEnabled(createServletContextWithConfiguredValue(configuredValue)));
}
- private ServletRequest createRequestWithConfiguredValue(String
configuredValue) {
+ private ServletContext createServletContextWithConfiguredValue(String
configuredValue) {
MockServletContext servletContext = new MockServletContext();
Dispatcher dispatcher = mock(Dispatcher.class);
ConfigurationManager configurationManager =
mock(ConfigurationManager.class);
@@ -330,9 +371,7 @@ public class StrutsTilesContainerFactoryTest {
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));
+ return servletContext;
}
private static class ExposedInitializer extends StrutsTilesInitializer {
@@ -362,7 +401,7 @@ public class StrutsTilesContainerFactoryTest {
}
@Override
- boolean isLegacyOgnlEnabled(org.apache.tiles.request.Request request) {
+ boolean isLegacyOgnlEnabled(ServletContext servletContext) {
configurationResolutions = resolutionCounter.incrementAndGet();
configurationResolutionEntered.countDown();
if (blockConfigurationResolution) {
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
index 4373a25e0..0ffc3b7d7 100644
---
a/plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesOgnlEvaluatorIntegrationTest.java
+++
b/plugins/tiles/src/test/java/org/apache/struts2/tiles/TilesOgnlEvaluatorIntegrationTest.java
@@ -44,6 +44,7 @@ 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.io.PrintWriter;
@@ -188,7 +189,7 @@ public class TilesOgnlEvaluatorIntegrationTest {
}
@Test
- public void
nonServletRequestFailsClosedWithoutExposingEnvironmentFailure() {
+ public void nonServletRequestFailsClosedWithoutExposingExpression() {
TrackingFactory factory = new TrackingFactory();
AttributeEvaluatorFactory evaluators =
createAttributeEvaluatorFactoryWithoutEl(
factory, tilesRequest.getApplicationContext());
@@ -266,9 +267,9 @@ public class TilesOgnlEvaluatorIntegrationTest {
private int legacyWarnings;
@Override
- boolean isLegacyOgnlEnabled(Request request) {
+ boolean isLegacyOgnlEnabled(ServletContext servletContext) {
configurationResolutions++;
- return super.isLegacyOgnlEnabled(request);
+ return super.isLegacyOgnlEnabled(servletContext);
}
@Override
