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