From c2293ea408c6264d873626ebb9de96d7e07c849f Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 6 Feb 2026 08:42:10 +0100 Subject: [PATCH] feat(security): WW-5294 add warning when JSP tags accessed directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add security warning to TagUtils.getStack() that logs when JSP tags are rendered outside of action scope (direct JSP access). This helps developers identify potential security issues where JSPs are accessed directly without going through the Struts action flow. The warning message includes a link to the security documentation at https://struts.apache.org/security/#never-expose-jsp-files-directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../apache/struts2/views/jsp/TagUtils.java | 10 +- .../struts2/views/jsp/ActionTagTest.java | 82 ++++---- .../struts2/views/jsp/TagUtilsTest.java | 189 ++++++++++++++++++ .../2026-02-06-WW-5294-textfield-warning.md | 186 +++++++++++++++++ 4 files changed, 426 insertions(+), 41 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/views/jsp/TagUtilsTest.java create mode 100644 thoughts/shared/research/2026-02-06-WW-5294-textfield-warning.md diff --git a/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java b/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java index e8dc60c811..a3bf1e1ddf 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java @@ -19,6 +19,7 @@ package org.apache.struts2.views.jsp; import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionInvocation; import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.util.ValueStack; import org.apache.logging.log4j.LogManager; @@ -44,7 +45,7 @@ public static ValueStack getStack(PageContext pageContext) { if (stack == null) { LOG.warn("No ValueStack in ActionContext!"); throw new ConfigurationException("Rendering tag out of Action scope, accessing directly JSPs is not recommended! " + - "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly"); + "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly"); } else { LOG.trace("Adds the current PageContext to ActionContext"); stack.getActionContext() @@ -52,6 +53,13 @@ public static ValueStack getStack(PageContext pageContext) { .with(ATTRIBUTES, new AttributeMap(stack.getContext())); } + // Check for direct JSP access (stack exists but no action invocation) + ActionInvocation ai = ActionContext.getContext().getActionInvocation(); + if (ai == null || ai.getAction() == null) { + LOG.warn("Rendering tag out of Action scope, accessing directly JSPs is not recommended! " + + "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly"); + } + return stack; } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ActionTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ActionTagTest.java index 3b60dca090..1853494043 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ActionTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ActionTagTest.java @@ -71,8 +71,8 @@ public void testActionTagWithNamespace() { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testActionTagWithNamespace_clearTagStateSet() { @@ -105,8 +105,8 @@ public void testActionTagWithNamespace_clearTagStateSet() { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testSimple() { @@ -144,8 +144,8 @@ public void testSimple() { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -187,8 +187,8 @@ public void testSimple_clearTagStateSet() { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -233,8 +233,8 @@ public void testSimpleWithActionMethodInOriginalURI() { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -276,8 +276,8 @@ public void testSimpleWithctionMethodInOriginalURI_clearTagStateSet() { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -305,8 +305,8 @@ public void testActionWithExecuteResult() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -337,8 +337,8 @@ public void testActionWithExecuteResult_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -366,8 +366,8 @@ public void testActionWithoutExecuteResult() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testActionWithoutExecuteResult_clearTagStateSet() throws Exception { @@ -397,13 +397,14 @@ public void testActionWithoutExecuteResult_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testExecuteButResetReturnSameInvocation() throws Exception { Mock mockActionInv = new Mock(ActionInvocation.class); mockActionInv.matchAndReturn("invoke", "TEST"); + mockActionInv.matchAndReturn("getAction", new Object()); ActionTag tag = new ActionTag(); tag.setPageContext(pageContext); tag.setNamespace(""); @@ -426,14 +427,15 @@ public void testExecuteButResetReturnSameInvocation() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testExecuteButResetReturnSameInvocation_clearTagStateSet() throws Exception { Mock mockActionInv = new Mock(ActionInvocation.class); mockActionInv.matchAndReturn("invoke", "TEST"); + mockActionInv.matchAndReturn("getAction", new Object()); ActionTag tag = new ActionTag(); tag.setPerformClearTagStateForTagPoolingServers(true); // Explicitly request tag state clearing. tag.setPageContext(pageContext); @@ -459,8 +461,8 @@ public void testExecuteButResetReturnSameInvocation_clearTagStateSet() throws Ex freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testIngoreContextParamsFalse() throws Exception { @@ -491,8 +493,8 @@ public void testIngoreContextParamsFalse() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -527,8 +529,8 @@ public void testIngoreContextParamsFalse_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -560,8 +562,8 @@ public void testIngoreContextParamsTrue() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testIngoreContextParamsTrue_clearTagStateSet() throws Exception { @@ -595,8 +597,8 @@ public void testIngoreContextParamsTrue_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testNoNameDefined() throws Exception { @@ -633,8 +635,8 @@ public void testUnknownNameDefined() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } // FIXME: Logging the error seems to cause the standard Maven build to fail @@ -656,8 +658,8 @@ public void testUnknownNameDefined_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testActionMethodWithExecuteResult() throws Exception { @@ -685,8 +687,8 @@ public void testActionMethodWithExecuteResult() throws Exception { ActionTag freshTag = new ActionTag(); 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)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testActionMethodWithExecuteResult_clearTagStateSet() throws Exception { @@ -717,8 +719,8 @@ public void testActionMethodWithExecuteResult_clearTagStateSet() throws Exceptio freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", - strutsBodyTagsAreReflectionEqual(tag, freshTag)); + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @Override diff --git a/core/src/test/java/org/apache/struts2/views/jsp/TagUtilsTest.java b/core/src/test/java/org/apache/struts2/views/jsp/TagUtilsTest.java new file mode 100644 index 0000000000..0c997e27a5 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/views/jsp/TagUtilsTest.java @@ -0,0 +1,189 @@ +/* + * 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.views.jsp; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.struts2.ActionContext; +import org.apache.struts2.config.ConfigurationException; +import org.apache.struts2.mock.MockActionInvocation; +import org.apache.struts2.util.ValueStack; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.TestAction; + +import java.util.ArrayList; +import java.util.List; + +/** + * Tests for {@link TagUtils} class. + * Verifies security warning behavior when JSP tags are accessed directly without action flow. + */ +public class TagUtilsTest extends StrutsInternalTestCase { + + private StrutsMockHttpServletRequest request; + private StrutsMockPageContext pageContext; + private StrutsMockHttpServletResponse response; + private TestAppender testAppender; + private Logger tagUtilsLogger; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + request = new StrutsMockHttpServletRequest(); + response = new StrutsMockHttpServletResponse(); + request.setSession(new StrutsMockHttpSession()); + + pageContext = new StrutsMockPageContext(servletContext, request, response); + + // Setup log appender to capture warnings + testAppender = new TestAppender(); + tagUtilsLogger = (Logger) LogManager.getLogger(TagUtils.class); + tagUtilsLogger.addAppender(testAppender); + testAppender.start(); + } + + @Override + protected void tearDown() throws Exception { + testAppender.stop(); + tagUtilsLogger.removeAppender(testAppender); + ActionContext.clear(); + super.tearDown(); + } + + public void testGetStack_withNullValueStack_throwsConfigurationException() { + // Setup: no ValueStack in request or ActionContext + ActionContext.of().bind(); + + try { + TagUtils.getStack(pageContext); + fail("Expected ConfigurationException to be thrown"); + } catch (ConfigurationException e) { + assertTrue("Exception message should contain 'Rendering tag out of Action scope'", + e.getMessage().contains("Rendering tag out of Action scope")); + assertTrue("Exception message should contain security warning", + e.getMessage().contains("accessing directly JSPs is not recommended")); + } + } + + public void testGetStack_withNullActionInvocation_logsWarning() { + // Setup: ValueStack exists but no ActionInvocation + ValueStack stack = ActionContext.getContext().getValueStack(); + request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, stack); + + // Ensure ActionInvocation is null + ActionContext.of(stack.getContext()) + .withActionInvocation(null) + .bind(); + + // Execute + ValueStack result = TagUtils.getStack(pageContext); + + // Verify + assertNotNull("ValueStack should be returned", result); + assertTrue("Warning about direct JSP access should be logged", + hasWarningLogMessage("Rendering tag out of Action scope")); + } + + public void testGetStack_withNullAction_logsWarning() { + // Setup: ValueStack and ActionInvocation exist but action is null + ValueStack stack = ActionContext.getContext().getValueStack(); + request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, stack); + + MockActionInvocation actionInvocation = new MockActionInvocation(); + actionInvocation.setAction(null); + + ActionContext.of(stack.getContext()) + .withActionInvocation(actionInvocation) + .bind(); + + // Execute + ValueStack result = TagUtils.getStack(pageContext); + + // Verify + assertNotNull("ValueStack should be returned", result); + assertTrue("Warning about direct JSP access should be logged", + hasWarningLogMessage("Rendering tag out of Action scope")); + } + + public void testGetStack_withValidAction_noWarning() { + // Setup: normal action flow with valid action + ValueStack stack = ActionContext.getContext().getValueStack(); + request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, stack); + + TestAction action = new TestAction(); + MockActionInvocation actionInvocation = new MockActionInvocation(); + actionInvocation.setAction(action); + + ActionContext.of(stack.getContext()) + .withActionInvocation(actionInvocation) + .bind(); + + // Execute + ValueStack result = TagUtils.getStack(pageContext); + + // Verify + assertNotNull("ValueStack should be returned", result); + assertFalse("Warning should NOT be logged when action is present", + hasWarningLogMessage("Rendering tag out of Action scope")); + } + + public void testGetStack_warningMessageContainsSecurityUrl() { + // Setup: ValueStack exists but no ActionInvocation + ValueStack stack = ActionContext.getContext().getValueStack(); + request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, stack); + + ActionContext.of(stack.getContext()) + .withActionInvocation(null) + .bind(); + + // Execute + TagUtils.getStack(pageContext); + + // Verify warning contains security documentation URL + assertTrue("Warning should contain security documentation URL", + hasWarningLogMessage("https://struts.apache.org/security/#never-expose-jsp-files-directly")); + } + + private boolean hasWarningLogMessage(String messageSubstring) { + return testAppender.logEvents.stream() + .anyMatch(event -> event.getLevel() == Level.WARN + && event.getMessage().getFormattedMessage().contains(messageSubstring)); + } + + /** + * Test appender to capture log events for verification. + */ + static class TestAppender extends AbstractAppender { + List logEvents = new ArrayList<>(); + + TestAppender() { + super("TestAppender", null, null, false, null); + } + + @Override + public void append(LogEvent logEvent) { + logEvents.add(logEvent.toImmutable()); + } + } +} diff --git a/thoughts/shared/research/2026-02-06-WW-5294-textfield-warning.md b/thoughts/shared/research/2026-02-06-WW-5294-textfield-warning.md new file mode 100644 index 0000000000..f964f9cc06 --- /dev/null +++ b/thoughts/shared/research/2026-02-06-WW-5294-textfield-warning.md @@ -0,0 +1,186 @@ +--- +date: 2026-02-06T10:00:00-08:00 +topic: "WW-5294: TextField tag not showing warning when JSP exposed directly" +tags: [research, security, components, templates, direct-jsp-access] +status: complete +jira: WW-5294 +--- + +# Research: WW-5294 - TextField Warning for Direct JSP Access + +**Date**: 2026-02-06 + +## Research Question + +Investigate why `` tag does not show the security warning when JSP pages are accessed directly (bypassing Struts action flow), while tags like `` and `` do show warnings. + +## Summary + +The warning mechanism for "direct JSP access" is **inconsistently implemented** across Struts components. The warning exists in some places but not others, and the detection conditions vary. The bug is that `` (and potentially other UIBean components) should trigger the same warning as other tags but doesn't in certain scenarios. + +## Detailed Findings + +### Warning Mechanism Locations + +There are **two locations** where warnings about direct JSP access are implemented: + +#### 1. TagUtils.getStack() - Exception when ValueStack is null + +**File**: `core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java:44-47` + +```java +if (stack == null) { + LOG.warn("No ValueStack in ActionContext!"); + throw new ConfigurationException("Rendering tag out of Action scope, accessing directly JSPs is not recommended! " + + "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly"); +} +``` + +- **Behavior**: Logs warning AND throws exception (stops rendering) +- **Trigger**: ValueStack is null +- **Affects**: ALL tags that use `getStack()` + +#### 2. FreemarkerTemplateEngine.renderTemplate() - Warning when action is null + +**File**: `core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java:119-125` + +```java +ActionInvocation ai = ActionContext.getContext().getActionInvocation(); +Object action = (ai == null) ? null : ai.getAction(); +if (action == null) { + LOG.warn("Rendering tag {} out of Action scope, accessing directly JSPs is not recommended! " + + "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly", templateName); +} +``` + +- **Behavior**: Logs warning only (rendering continues) +- **Trigger**: ActionInvocation is null OR action is null +- **Affects**: Only UIBean components using FreeMarker templates + +### Missing Warning Locations + +#### 1. JspTemplateEngine - NO warning check + +**File**: `core/src/main/java/org/apache/struts2/components/template/JspTemplateEngine.java` + +The JSP template engine does NOT have any warning for direct JSP access. If someone uses JSP templates instead of FreeMarker, they get no warning. + +#### 2. ServletUrlRenderer - NO warning check + +**File**: `core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java` + +The URL renderer used by `` tag does NOT have any warning mechanism. It uses `ActionContext.getContext().getActionInvocation()` but only for URL generation logic, not for warning. + +### Component Class Hierarchy + +``` +Component (base) +├── ContextBean +│ └── URL (extends ContextBean) → Uses ServletUrlRenderer, NO template +└── UIBean (extends Component) + ├── TextField → Uses FreeMarker template "text.ftl" + ├── ClosingUIBean + │ └── Anchor → Uses FreeMarker template "a.ftl" + └── (other UI components) +``` + +### Tag Processing Flow + +1. **JSP tag doStartTag()** calls `ComponentTagSupport.doStartTag()` +2. `getStack()` is called → delegates to `TagUtils.getStack()` +3. If stack is null → **ConfigurationException thrown** (all tags fail) +4. Component is created and rendered + +For UIBean components (TextField, Anchor, etc.): +5. `UIBean.end()` calls `mergeTemplate()` +6. `TemplateEngineManager.getTemplateEngine()` returns appropriate engine +7. `FreemarkerTemplateEngine.renderTemplate()` logs warning if action is null + +For URL component: +5. `URL.end()` calls `urlRenderer.renderUrl()` +6. No warning check exists in this path + +### Root Cause Analysis + +**Scenario 1: No ValueStack at all** +- First tag to render triggers `ConfigurationException` in TagUtils +- ALL tags fail with visible error +- This is NOT the bug scenario + +**Scenario 2: ValueStack exists but no ActionInvocation** +This is the likely bug scenario: +- TagUtils check passes (stack is not null) +- `` renders via ServletUrlRenderer → **NO warning** +- `` renders via FreemarkerTemplateEngine → **Logs warning** +- `` renders via FreemarkerTemplateEngine → **Should log warning** + +The issue is that: +1. The FreemarkerTemplateEngine warning only goes to the **log**, not the user +2. The warning condition `action == null` might not be triggered if there's a "stub" ActionInvocation +3. The warning is inconsistent between template engines (FreeMarker has it, JSP doesn't) + +### Key Inconsistencies Found + +| Component | Template Engine | Warning Location | Warning Type | +|-----------|----------------|------------------|--------------| +| `` | None (direct render) | None | **No warning** | +| `` | FreeMarker | FreemarkerTemplateEngine | Log warning | +| `` | FreeMarker | FreemarkerTemplateEngine | Log warning | +| Any UIBean with JSP template | JSP | None | **No warning** | + +## Code References + +- `core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java:38-56` - getStack() with ConfigurationException +- `core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java:119-125` - Warning for FreeMarker +- `core/src/main/java/org/apache/struts2/components/template/JspTemplateEngine.java:48-83` - renderTemplate() without warning +- `core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java:75-134` - renderUrl() without warning +- `core/src/main/java/org/apache/struts2/components/UIBean.java:565-578` - end() calls mergeTemplate() +- `core/src/main/java/org/apache/struts2/components/URL.java:141-144` - end() uses urlRenderer + +## Recommendations + +### Option 1: Centralize Warning in TagUtils (Recommended) + +Add the ActionInvocation check to `TagUtils.getStack()` so ALL tags get the warning consistently: + +```java +public static ValueStack getStack(PageContext pageContext) { + // ... existing stack check ... + + // Add warning for missing ActionInvocation + ActionInvocation ai = ActionContext.getContext().getActionInvocation(); + if (ai == null || ai.getAction() == null) { + LOG.warn("Rendering tag out of Action scope, accessing directly JSPs is not recommended! " + + "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly"); + } + + return stack; +} +``` + +### Option 2: Add Warning to JspTemplateEngine + +Add the same warning check that exists in FreemarkerTemplateEngine to JspTemplateEngine for consistency. + +### Option 3: Add Warning to Component.start() + +Add the warning check to the base `Component.start()` method so ALL components (not just template-based ones) get the warning. + +## Security Implications + +This is a **security issue** because: +1. Directly accessing JSPs bypasses Struts interceptors (validation, security, etc.) +2. Form input tags like `` pose potentially greater risks than link tags +3. The inconsistent warning behavior may give developers false confidence + +## Related Issues + +- The JIRA ticket was reopened because the fix was incomplete +- Title updated to clarify: "not showing the warning" for `` +- Fix version: 7.2.0 + +## Open Questions + +1. Should the warning also be added to `Component.start()` for non-template components? +2. Should the warning be elevated to a thrown exception (like TagUtils does for null stack)? +3. Are there other components/paths that also need the warning? \ No newline at end of file