This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 8f3b4abde WW-5789 fix(rest): keep params-bound values when the body
handler binds nothing
8f3b4abde is described below
commit 8f3b4abde5672dee11f0467fc5145fd65420a466
Author: Lukasz Lenart <[email protected]>
AuthorDate: Thu Oct 22 21:58:35 2026 +0200
WW-5789 fix(rest): keep params-bound values when the body handler binds
nothing
Since WW-5626 (7.2.0), ContentTypeInterceptor runs a two-phase copy for
every
handler that is not AuthorizationAwareContentTypeHandler when
struts.parameters.requireAnnotations=true: a fresh instance is built with
the
no-arg constructor, the handler fills it, and every non-null property is
copied
onto the real target. FormUrlEncodedHandler, MultipartFormDataHandler and
HtmlHandler never touch the body, so the "fresh" instance holds only
constructor defaults, and those overwrote the values ParametersInterceptor
had
bound one step earlier. Null defaults were skipped, which is why the report
reads "works unless the field is null by default".
The three handlers bind nothing, so they now declare the marker and take the
direct path. The marker Javadoc admits that case and names the subclass
caveat.
Co-Authored-By: Claude Mythos 5.1 <[email protected]>
---
.../AuthorizationAwareContentTypeHandler.java | 6 ++
.../rest/handler/FormUrlEncodedHandler.java | 4 +-
.../apache/struts2/rest/handler/HtmlHandler.java | 4 +-
.../rest/handler/MultipartFormDataHandler.java | 4 +-
.../ContentTypeInterceptorIntegrationTest.java | 68 ++++++++++++++++++++--
5 files changed, 79 insertions(+), 7 deletions(-)
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/AuthorizationAwareContentTypeHandler.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/AuthorizationAwareContentTypeHandler.java
index 0e01c507e..a7840b648 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/AuthorizationAwareContentTypeHandler.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/AuthorizationAwareContentTypeHandler.java
@@ -38,6 +38,12 @@ package org.apache.struts2.rest.handler;
* interface but its parser does not honor the context, authorization will
silently do nothing —
* a serious security bug. The marker interface is the contract;
implementations must uphold it.</p>
*
+ * <p>A handler whose {@code toObject} binds nothing from the body, such as
{@link FormUrlEncodedHandler},
+ * upholds the contract trivially and declares the interface so that the
two-phase copy, which would
+ * write a fresh instance's constructor defaults over values already bound,
does not run for it. A
+ * subclass that overrides {@code toObject} to deserialize inherits the
declaration and must honor the
+ * context itself.</p>
+ *
* @since 7.2.0
*/
public interface AuthorizationAwareContentTypeHandler extends
ContentTypeHandler {
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/FormUrlEncodedHandler.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/FormUrlEncodedHandler.java
index e73d336a3..076970427 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/FormUrlEncodedHandler.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/FormUrlEncodedHandler.java
@@ -31,10 +31,12 @@ import java.io.Writer;
*
* This handler is intended for requests only, not for responses
*
+ * Binds nothing from the body, so it is trivially {@link
AuthorizationAwareContentTypeHandler}.
+ *
* @see <a
href="http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4";>http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4</a>
*
*/
-public class FormUrlEncodedHandler implements ContentTypeHandler {
+public class FormUrlEncodedHandler implements
AuthorizationAwareContentTypeHandler {
public static final String CONTENT_TYPE =
"application/x-www-form-urlencoded";
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/HtmlHandler.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/HtmlHandler.java
index d7d6e08ed..c86a9b8d6 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/HtmlHandler.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/HtmlHandler.java
@@ -26,8 +26,10 @@ import java.io.Writer;
/**
* Handles HTML content, usually just a simple passthrough to the framework
+ *
+ * Binds nothing from the body, so it is trivially {@link
AuthorizationAwareContentTypeHandler}.
*/
-public class HtmlHandler implements ContentTypeHandler {
+public class HtmlHandler implements AuthorizationAwareContentTypeHandler {
@Override
public String fromObject(ActionInvocation invocation, Object obj, String
resultCode, Writer out) throws IOException {
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/MultipartFormDataHandler.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/MultipartFormDataHandler.java
index 6a4a42b89..3b77ebe6d 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/MultipartFormDataHandler.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/MultipartFormDataHandler.java
@@ -32,9 +32,11 @@ import java.io.Writer;
*
* This handler is intended for requests only, not for responses
*
+ * Binds nothing from the body, so it is trivially {@link
AuthorizationAwareContentTypeHandler}.
+ *
* @see <a
href="http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4";>http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4</a>
*/
-public class MultipartFormDataHandler implements ContentTypeHandler {
+public class MultipartFormDataHandler implements
AuthorizationAwareContentTypeHandler {
public static final String CONTENT_TYPE = "multipart/form-data";
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
index f2a973596..68a875e61 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
@@ -26,6 +26,7 @@ import junit.framework.TestCase;
import org.apache.struts2.ActionContext;
import org.apache.struts2.ActionInvocation;
import org.apache.struts2.ActionSupport;
+import org.apache.struts2.ModelDriven;
import org.apache.struts2.action.Action;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.interceptor.parameter.StrutsParameter;
@@ -37,9 +38,13 @@ import org.apache.struts2.ognl.StrutsOgnlGuard;
import org.apache.struts2.ognl.StrutsProxyCacheFactory;
import org.apache.struts2.rest.handler.AuthorizationAwareContentTypeHandler;
import org.apache.struts2.rest.handler.ContentTypeHandler;
+import org.apache.struts2.rest.handler.FormUrlEncodedHandler;
+import org.apache.struts2.rest.handler.HtmlHandler;
import org.apache.struts2.rest.handler.JacksonJsonHandler;
+import org.apache.struts2.rest.handler.MultipartFormDataHandler;
import org.apache.struts2.rest.handler.jackson.ForwardReferenceStateProbe;
import org.apache.struts2.util.StrutsProxyService;
+import org.apache.struts2.util.ValueStack;
import org.springframework.mock.web.MockHttpServletRequest;
import java.io.Reader;
@@ -58,6 +63,7 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
private ContentTypeInterceptor interceptor;
private SecureRestAction action;
+ private Object configuredAction;
private Mock mockActionInvocation;
private Mock mockSelector;
@@ -79,6 +85,7 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
}
private void setupInterceptorWithHandler(Object actionInstance,
ContentTypeHandler handler) {
+ configuredAction = actionInstance;
var ognlUtil = new OgnlUtil(
new DefaultOgnlExpressionCacheFactory<>("1000",
LRU.toString()),
new DefaultOgnlBeanInfoCacheFactory<>("1000", LRU.toString()),
@@ -108,14 +115,23 @@ public class ContentTypeInterceptorIntegrationTest
extends TestCase {
}
private void runWithBody(String body) throws Exception {
+ runWithBody(body, "application/json");
+ }
+
+ private void runWithBody(String body, String contentType) throws Exception
{
MockHttpServletRequest request = new MockHttpServletRequest();
request.setContent(body.getBytes());
- request.setContentType("application/json");
+ request.setContentType(contentType);
- ActionContext.of()
+ ActionContext context = ActionContext.of()
.withActionMapping(new ActionMapping())
- .withServletRequest(request)
- .bind();
+ .withServletRequest(request);
+ if (configuredAction instanceof ModelDriven<?> modelDriven) {
+ Mock mockStack = new Mock(ValueStack.class);
+ mockStack.matchAndReturn("peek", modelDriven.getModel());
+ context.withValueStack((ValueStack) mockStack.proxy());
+ }
+ context.bind();
interceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
mockSelector.verify();
@@ -159,6 +175,32 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
action.getShallowAddress().getZip());
}
+ // --- WW-5789: a handler that binds nothing must leave the model as the
params interceptor left it ---
+
+ public void testFormUrlEncodedBodyKeepsValuesBoundOnTheModel() throws
Exception {
+ assertNoOpHandlerKeepsBoundModelValues(new FormUrlEncodedHandler());
+ }
+
+ public void testMultipartFormDataBodyKeepsValuesBoundOnTheModel() throws
Exception {
+ assertNoOpHandlerKeepsBoundModelValues(new MultipartFormDataHandler());
+ }
+
+ public void testHtmlBodyKeepsValuesBoundOnTheModel() throws Exception {
+ assertNoOpHandlerKeepsBoundModelValues(new HtmlHandler());
+ }
+
+ private void assertNoOpHandlerKeepsBoundModelValues(ContentTypeHandler
handler) throws Exception {
+ ModelDrivenRestAction modelDriven = new ModelDrivenRestAction();
+ DefaultedModel model = modelDriven.getModel();
+ model.setStatus("bound-by-params");
+ setupInterceptorWithHandler(modelDriven, handler);
+
+ runWithBody("status=bound-by-params", handler.getContentType());
+
+ assertEquals("a constructor default must not overwrite a value the
params interceptor bound",
+ "bound-by-params", model.getStatus());
+ }
+
// --- Tests proving the new Jackson authorization path is in use ---
public void testJacksonHandlerDoesNotRequireNoArgConstructor() throws
Exception {
@@ -373,4 +415,22 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
@JsonProperty("name")
public void setAdmin(String admin) { this.admin = admin; }
}
+
+ /**
+ * WW-5789: a model whose constructor sets a default, the shape on which a
fresh-instance copy shows up.
+ */
+ public static class DefaultedModel {
+ private String status = "new";
+
+ public String getStatus() { return status; }
+
+ public void setStatus(String status) { this.status = status; }
+ }
+
+ public static class ModelDrivenRestAction extends ActionSupport implements
ModelDriven<DefaultedModel> {
+ private final DefaultedModel model = new DefaultedModel();
+
+ @Override
+ public DefaultedModel getModel() { return model; }
+ }
}
