WW-5352 Implement auto-allowlisting for Iterator component

This commit is contained in:
Kusal Kithul-Godage
2024-01-09 19:24:18 +11:00
parent a57c2882e7
commit 0a71e2c3b9
5 changed files with 34 additions and 22 deletions
@@ -38,7 +38,6 @@
<constant name="struts.parameters.requireAnnotations" value="true" />
<constant name="struts.allowlist.classes"
value="
org.apache.struts2.showcase.conversion.OperationsEnum,
org.apache.struts2.showcase.conversion.Person,
org.apache.struts2.showcase.conversion.Address
"/>
@@ -18,9 +18,11 @@
*/
package org.apache.struts2.components;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.util.ValueStack;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.ognl.ThreadAllowlist;
import org.apache.struts2.util.MakeIterator;
import org.apache.struts2.views.annotations.StrutsTag;
import org.apache.struts2.views.annotations.StrutsTagAttribute;
@@ -188,8 +190,8 @@ import java.util.List;
* <!-- START SNIPPET: example6description -->
*
* <p>Another way to create a simple loop, similar to JSTL's
* &lt;c:forEach begin="..." end="..." ...&gt; is to use some
* OGNL magic, which provides some under-the-covers magic to
* &lt;c:forEach begin="..." end="..." ...&gt; is to use some
* OGNL magic, which provides some under-the-covers magic to
* make 0-n loops trivial. This example also loops five times.</p>
*
* <!-- END SNIPPET: example6description -->
@@ -237,11 +239,17 @@ public class IteratorComponent extends ContextBean {
protected Integer end;
protected String stepStr;
protected Integer step;
private ThreadAllowlist threadAllowlist;
public IteratorComponent(ValueStack stack) {
super(stack);
}
@Inject
public void setThreadAllowlist(ThreadAllowlist threadAllowlist) {
this.threadAllowlist = threadAllowlist;
}
public boolean start(Writer writer) {
//Create an iterator status if the status attribute was set.
if (statusAttr != null) {
@@ -298,6 +306,7 @@ public class IteratorComponent extends ContextBean {
if ((iterator != null) && iterator.hasNext()) {
Object currentValue = iterator.next();
stack.push(currentValue);
threadAllowlist.allowClass(currentValue.getClass());
String var = getVar();
@@ -719,8 +719,6 @@ public class Dispatcher {
} else {
throw new ServletException(e);
}
} finally {
threadAllowlist.clearAllowlist();
}
}
@@ -1051,6 +1049,7 @@ public class Dispatcher {
*/
public void cleanUpRequest(HttpServletRequest request) {
ContainerHolder.clear();
threadAllowlist.clearAllowlist();
if (!(request instanceof MultiPartRequestWrapper)) {
return;
}
@@ -18,16 +18,14 @@
*/
package org.apache.struts2.views.jsp;
import com.opensymphony.xwork2.inject.Container;
import com.opensymphony.xwork2.util.ValueStack;
import org.apache.struts2.components.Component;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.jsp.JspException;
import com.opensymphony.xwork2.ActionContext;
import org.apache.struts2.components.Component;
import com.opensymphony.xwork2.inject.Container;
import com.opensymphony.xwork2.util.ValueStack;
/**
*/
public abstract class ComponentTagSupport extends StrutsBodyTagSupport {
@@ -39,8 +37,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport {
public int doEndTag() throws JspException {
component.end(pageContext.getOut(), getBody());
component = null; // Always clear component reference (since clearTagStateForTagPoolingServers() is conditional).
clearTagStateForTagPoolingServers();
return EVAL_PAGE;
return super.doEndTag();
}
@Override
@@ -49,7 +46,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport {
component = getBean(stack, (HttpServletRequest) pageContext.getRequest(), (HttpServletResponse) pageContext.getResponse());
Container container = stack.getActionContext().getContainer();
container.inject(component);
populateParams();
boolean evalBody = component.start(pageContext.getOut());
@@ -62,7 +59,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport {
/**
* Define method to populate component state based on the Tag parameters.
*
* <p>
* Descendants should override this method for custom behaviour, but should <em>always</em> call the ancestor method when doing so.
*/
protected void populateParams() {
@@ -71,7 +68,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport {
/**
* Specialized method to populate the performClearTagStateForTagPoolingServers state of the Component to match the value set in the Tag.
*
* <p>
* Generally only unit tests would call this method directly, to avoid calling the whole populateParams() chain again after doStartTag()
* has been called. Doing that can break tag / component state behaviour, but unit tests still need a way to set the
* performClearTagStateForTagPoolingServers state for the component (which only comes into being after doStartTag() is called).
@@ -21,6 +21,7 @@ package org.apache.struts2.components;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.util.ValueStack;
import org.apache.struts2.StrutsInternalTestCase;
import org.apache.struts2.ognl.ThreadAllowlist;
import java.io.StringWriter;
import java.util.Arrays;
@@ -28,14 +29,25 @@ import java.util.List;
public class IteratorComponentTest extends StrutsInternalTestCase {
private ValueStack stack;
private IteratorComponent ic;
private ThreadAllowlist threadAllowlist;
@Override
public void setUp() throws Exception {
super.setUp();
stack = ActionContext.getContext().getValueStack();
ic = new IteratorComponent(stack);
threadAllowlist = new ThreadAllowlist();
ic.setThreadAllowlist(threadAllowlist);
}
public void testIterator() throws Exception {
// given
final ValueStack stack = ActionContext.getContext().getValueStack();
stack.push(new FooAction());
StringWriter out = new StringWriter();
IteratorComponent ic = new IteratorComponent(stack);
ic.setValue("items");
ic.setVar("val");
@@ -64,12 +76,10 @@ public class IteratorComponentTest extends StrutsInternalTestCase {
public void testIteratorWithBegin() throws Exception {
// given
final ValueStack stack = ActionContext.getContext().getValueStack();
stack.push(new FooAction());
StringWriter out = new StringWriter();
IteratorComponent ic = new IteratorComponent(stack);
ic.setValue("items");
ic.setVar("val");
ic.setBegin("1");
@@ -96,7 +106,6 @@ public class IteratorComponentTest extends StrutsInternalTestCase {
public void testIteratorWithNulls() throws Exception {
// given
final ValueStack stack = ActionContext.getContext().getValueStack();
stack.push(new FooAction() {
private List items = Arrays.asList("1", "2", null, "4");
@@ -107,7 +116,6 @@ public class IteratorComponentTest extends StrutsInternalTestCase {
StringWriter out = new StringWriter();
IteratorComponent ic = new IteratorComponent(stack);
ic.setValue("items");
ic.setVar("val");
Property prop = new Property(stack);