Skip to content

WW-5294 Add warning when JSP tags accessed directly #1569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lukaszlenart merged 1 commit into from
Feb 17, 2026
+426 −41
Merged

WW-5294 Add warning when JSP tags accessed directly #1569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,14 +45,21 @@ 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()
.withPageContext(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;
}

Expand Down
82 changes: 42 additions & 40 deletions core/src/test/java/org/apache/struts2/views/jsp/ActionTagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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("");
Expand All @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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));

}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading