WW-5713 Fail closed for legacy Tiles OGNL evaluation

plugins/tiles/README.md

@@ -4,3 +4,17 @@ 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
+<constant name="struts.tiles.ognl.legacy.enabled" value="true"/>
+```
+
+Enabling the constant produces a startup warning. 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.

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);
+    }
+}

plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java

@@ -39,6 +39,7 @@
 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.AttributeEvaluator;
 import org.apache.tiles.core.evaluator.AttributeEvaluatorFactory;
 import org.apache.tiles.core.evaluator.BasicAttributeEvaluatorFactory;
 import org.apache.tiles.core.evaluator.impl.DirectAttributeEvaluator;
@@ -73,6 +74,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 +91,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 +122,14 @@ public class StrutsTilesContainerFactory extends BasicTilesContainerFactory {
     public static final String S2 = "S2";
     public static final String I18N = "I18N";
 
+    public StrutsTilesContainerFactory() {
+        this(false);
+    }
+
+    StrutsTilesContainerFactory(boolean legacyOgnlEnabled) {
+        this.legacyOgnlEnabled = legacyOgnlEnabled;
+    }
+
     @Override
     public TilesContainer createDecoratedContainer(TilesContainer originalContainer, ApplicationContext applicationContext) {
         return new CachingTilesContainer(originalContainer);
@@ -155,7 +172,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 +269,21 @@ protected I18NAttributeEvaluator createI18NEvaluator() {
         return new I18NAttributeEvaluator();
     }
 
+    private AttributeEvaluator createConfiguredOgnlEvaluator() {
+        if (legacyOgnlEnabled) {
+            if (legacyOgnlWarningLogged.compareAndSet(false, true)) {
+                logLegacyOgnlWarning();
+            }
+            return createOGNLEvaluator();
+        }
+        return new DisabledOgnlAttributeEvaluator();
+    }
+
+    void logLegacyOgnlWarning() {
+        LOG.warn(LEGACY_OGNL_WARNING);
+    }
+
+    @SuppressWarnings("removal")
     protected OGNLAttributeEvaluator createOGNLEvaluator() {
         try {
             PropertyAccessor objectPropertyAccessor = OgnlRuntime.getPropertyAccessor(Object.class);

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() {
+        this(false);
+    }
+
+    StrutsTilesInitializer(boolean legacyOgnlEnabled) {
+        this.legacyOgnlEnabled = legacyOgnlEnabled;
+    }
+
     @Override
     protected ApplicationContext createTilesApplicationContext(ApplicationContext preliminaryContext) {
         ServletContext servletContext = (ServletContext) preliminaryContext.getContext();
@@ -48,7 +58,7 @@ protected ApplicationContext createTilesApplicationContext(ApplicationContext pr
     @Override
     protected AbstractTilesContainerFactory createContainerFactory(ApplicationContext context) {
         LOG.trace("Creating dedicated Struts factory to create Tiles container");
-        return new StrutsTilesContainerFactory();
+        return new StrutsTilesContainerFactory(legacyOgnlEnabled);
     }
 
 }

plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesListener.java

@@ -18,23 +18,88 @@
  */
 package org.apache.struts2.tiles;
 
+import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletContextEvent;
+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.struts2.dispatcher.DispatcherListener;
 import org.apache.tiles.core.startup.TilesInitializer;
+import org.apache.tiles.request.servlet.ServletApplicationContext;
 import org.apache.tiles.web.startup.AbstractTilesListener;
 
 /**
  * Listener used to automatically tie Tiles support into Struts
  *
  * @since Struts 2.0.2
  */
-public class StrutsTilesListener extends AbstractTilesListener {
+public class StrutsTilesListener extends AbstractTilesListener implements DispatcherListener {
 
     private static final Logger LOG = LogManager.getLogger(StrutsTilesListener.class);
 
+    private ServletContext servletContext;
+    private boolean legacyOgnlEnabled;
+    private boolean listeningToDispatcher;
+
+    @Override
+    public void contextInitialized(ServletContextEvent event) {
+        servletContext = event.getServletContext();
+        Dispatcher dispatcher = Dispatcher.getInstance(servletContext);
+        if (dispatcher == null) {
+            Dispatcher.addDispatcherListener(this);
+            listeningToDispatcher = true;
+        } else {
+            initializeTiles(dispatcher);
+        }
+    }
+
+    @Override
+    public void contextDestroyed(ServletContextEvent event) {
+        if (listeningToDispatcher) {
+            Dispatcher.removeDispatcherListener(this);
+            listeningToDispatcher = false;
+        }
+        destroyTiles();
+        servletContext = null;
+    }
+
+    @Override
+    public void dispatcherInitialized(Dispatcher dispatcher) {
+        initializeTiles(dispatcher);
+    }
+
+    @Override
+    public void dispatcherDestroyed(Dispatcher dispatcher) {
+        destroyTiles();
+    }
+
     @Override
     protected TilesInitializer createTilesInitializer() {
         LOG.info("Starting Struts Tiles 3 integration ...");
-        return new StrutsTilesInitializer();
+        return new StrutsTilesInitializer(legacyOgnlEnabled);
+    }
+
+    boolean isLegacyOgnlEnabled() {
+        return legacyOgnlEnabled;
+    }
+
+    @SuppressWarnings("removal")
+    private synchronized void initializeTiles(Dispatcher dispatcher) {
+        if (initializer != null) {
+            return;
+        }
+        String configuredValue = dispatcher.getContainer().getInstance(
+            String.class, TilesConstants.STRUTS_TILES_OGNL_LEGACY_ENABLED);
+        legacyOgnlEnabled = BooleanUtils.toBoolean(configuredValue);
+        initializer = createTilesInitializer();
+        initializer.initialize(new ServletApplicationContext(servletContext));
+    }
+
+    private synchronized void destroyTiles() {
+        if (initializer != null) {
+            initializer.destroy();
+            initializer = null;
+        }
     }
 }

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() {
+    }
+}

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");

plugins/tiles/src/main/resources/struts-plugin.xml

@@ -24,6 +24,8 @@
 	"https://struts.apache.org/dtds/struts-6.0.dtd">
 
 <struts>
+    <constant name="struts.tiles.ognl.legacy.enabled" value="false"/>
+
     <package name="tiles-default" extends="struts-default">
         <result-types>
             <result-type name="tiles" class="org.apache.struts2.views.tiles.TilesResult"/>

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));
+    }
+}