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 a9ef39ea7 WW-5720 fix(rest): log any-setter dynamic-key rejections
once per request (#1948)
a9ef39ea7 is described below
commit a9ef39ea73c1179c4e9aac6d3b59faaa3dc3a937
Author: Lukasz Lenart <[email protected]>
AuthorDate: Tue Sep 15 19:48:44 2026 +0200
WW-5720 fix(rest): log any-setter dynamic-key rejections once per request
(#1948)
* WW-5720 fix(rest): log any-setter dynamic-key rejections once per request
An any-setter's key space is the request body, so a WARN per rejected key
let a single body write an unbounded number of log lines. The per-key
detail now goes to DEBUG and DynamicKeyRejections tallies the rejections
per any-setter and reason, writing one WARN for each when the request
state is cleared after the mapper read. Rejection itself is unchanged.
The tally lives in its own request-scoped holder rather than inside
DynamicKeyAuthorizationContext: that class is the nested-scope depth
stack, pushed and popped per accepted key, while the tally is per request
and flushed at a different point.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5720 fix(rest): name a creator-parameter any-setter by its index in
the rejection summary
AnnotatedParameter.getName() is empty, so the sink for a creator-parameter
any-setter read as "Foo#". Label it "Foo#creator[n]" instead, point the
WARN at the DEBUG lines that carry the rejected keys, and pin the wording
of the two reasons the tests did not yet assert on.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* WW-5720 test(rest): collect captured log messages with Stream.toList
Sonar S6204.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../jackson/AuthorizingSettableAnyProperty.java | 30 ++---
.../rest/handler/jackson/DynamicKeyRejections.java | 86 ++++++++++++++
.../jackson/ParameterAuthorizingModule.java | 9 +-
.../jackson/ParameterAuthorizingModuleTest.java | 127 ++++++++++++++++++++-
4 files changed, 234 insertions(+), 18 deletions(-)
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java
index e738e77a4..5072a6eb1 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java
@@ -26,10 +26,9 @@ import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.deser.SettableAnyProperty;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.util.TokenBuffer;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext;
import org.apache.struts2.interceptor.parameter.StrutsParameter;
+import org.apache.struts2.rest.handler.jackson.DynamicKeyRejections.Reason;
import java.io.IOException;
@@ -40,12 +39,12 @@ import java.io.IOException;
final class AuthorizingSettableAnyProperty extends SettableAnyProperty {
private static final long serialVersionUID = 1L;
- private static final Logger LOG =
LogManager.getLogger(AuthorizingSettableAnyProperty.class);
private static final Object REJECTED_VALUE = new Object();
private final SettableAnyProperty delegate;
private final StrutsParameter permission;
private final boolean creatorParameter;
+ private final String sink;
AuthorizingSettableAnyProperty(SettableAnyProperty delegate) {
super(delegate.getProperty(), memberOf(delegate.getProperty()),
delegate.getType(),
@@ -53,12 +52,23 @@ final class AuthorizingSettableAnyProperty extends
SettableAnyProperty {
this.delegate = delegate;
this.permission = permissionOf(delegate.getProperty());
this.creatorParameter = delegate.getParameterIndex() >= 0;
+ this.sink = sinkOf(delegate);
}
private static AnnotatedMember memberOf(BeanProperty property) {
return property == null ? null : property.getMember();
}
+ private static String sinkOf(SettableAnyProperty delegate) {
+ AnnotatedMember member = memberOf(delegate.getProperty());
+ if (member == null) {
+ return "<any-setter>";
+ }
+ int parameterIndex = delegate.getParameterIndex();
+ String memberName = parameterIndex >= 0 ? "creator[" + parameterIndex
+ "]" : member.getName();
+ return member.getDeclaringClass().getName() + "#" + memberName;
+ }
+
private static StrutsParameter permissionOf(BeanProperty property) {
AnnotatedMember member = memberOf(property);
return member == null ? null :
member.getAnnotation(StrutsParameter.class);
@@ -214,24 +224,18 @@ final class AuthorizingSettableAnyProperty extends
SettableAnyProperty {
}
private void rejectPermission(JsonParser parser, String path) throws
IOException {
- if (creatorParameter) {
- LOG.warn("REST body creator-parameter any-setter [{}] rejected;
dynamic-key consent "
- + "can only be declared on an any-setter method or field",
path);
- } else {
- LOG.warn("REST body any-setter parameter [{}] rejected; dynamic
keys require "
- + "@StrutsParameter(allowDynamicKeys = true) on a method
or field", path);
- }
+ DynamicKeyRejections.tally(creatorParameter ? Reason.CREATOR_PARAMETER
: Reason.CONSENT_MISSING,
+ sink, path);
redactAndSkip(parser);
}
private void rejectDepth(JsonParser parser, String path, int valueDepth,
int allowedDepth) throws IOException {
- LOG.warn("REST body any-setter parameter [{}] rejected; value depth
[{}] exceeds "
- + "@StrutsParameter depth [{}]", path, valueDepth,
allowedDepth);
+ DynamicKeyRejections.tally(Reason.DEPTH_EXCEEDED, sink, path,
valueDepth, allowedDepth);
redactAndSkip(parser);
}
private void rejectMissingPropertyName(JsonParser parser) throws
IOException {
- LOG.warn("REST body any-setter parameter rejected; dynamic property
name is unavailable");
+ DynamicKeyRejections.tally(Reason.PROPERTY_NAME_UNAVAILABLE, sink,
null);
redactAndSkip(parser);
}
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyRejections.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyRejections.java
new file mode 100644
index 000000000..4b15035d4
--- /dev/null
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyRejections.java
@@ -0,0 +1,86 @@
+/*
+ * 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.rest.handler.jackson;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+/**
+ * Tallies the dynamic keys an any-setter rejected while a body was read. The
key space of an
+ * any-setter is the request body itself, so the per-key detail stays at DEBUG
and a single WARN per
+ * sink and reason is written when the request state is cleared.
+ */
+final class DynamicKeyRejections {
+
+ private static final Logger LOG =
LogManager.getLogger(DynamicKeyRejections.class);
+ private static final ThreadLocal<Map<Entry, Integer>> TALLIES = new
ThreadLocal<>();
+
+ enum Reason {
+ CONSENT_MISSING("dynamic keys require
@StrutsParameter(allowDynamicKeys = true) on a method or field"),
+ CREATOR_PARAMETER("dynamic-key consent can only be declared on an
any-setter method or field"),
+ DEPTH_EXCEEDED("value depth exceeds the @StrutsParameter depth",
+ "value depth [{}] exceeds @StrutsParameter depth [{}]"),
+ PROPERTY_NAME_UNAVAILABLE("dynamic property name is unavailable");
+
+ private final String summary;
+ private final String detail;
+
+ Reason(String summary) {
+ this(summary, summary);
+ }
+
+ Reason(String summary, String detail) {
+ this.summary = summary;
+ this.detail = detail;
+ }
+ }
+
+ private DynamicKeyRejections() {
+ // utility
+ }
+
+ static void tally(Reason reason, String sink, String path, Object...
detailArguments) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("REST body any-setter parameter [{}] rejected by [{}];
{}", path, sink,
+ new ParameterizedMessage(reason.detail,
detailArguments).getFormattedMessage());
+ }
+ Map<Entry, Integer> tallies = TALLIES.get();
+ if (tallies == null) {
+ tallies = new LinkedHashMap<>();
+ TALLIES.set(tallies);
+ }
+ tallies.merge(new Entry(sink, reason), 1, Integer::sum);
+ }
+
+ static void reportAndClear() {
+ Map<Entry, Integer> tallies = TALLIES.get();
+ TALLIES.remove();
+ if (tallies == null) {
+ return;
+ }
+ tallies.forEach((entry, count) ->
+ LOG.warn("REST body any-setter [{}] rejected [{}] dynamic
key(s), logged at DEBUG; {}",
+ entry.sink, count, entry.reason.summary));
+ }
+
+ private record Entry(String sink, Reason reason) {
+ }
+}
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
index b9aeae4b6..e28689c6a 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
@@ -171,9 +171,11 @@ public class ParameterAuthorizingModule extends
SimpleModule {
/**
* Clears the request-scoped state this module keeps on the thread — the
dynamic-key scopes of
- * any-setters and the verdicts awaiting a forward reference — after a
mapper read. A handler that
- * registers this module on its own mapper must call it in a {@code
finally} around every read;
- * {@code ContentTypeInterceptor} clears the same state once more when it
unbinds the context.
+ * any-setters, the verdicts awaiting a forward reference, and the tally
of rejected dynamic keys,
+ * which is written to the log as one WARN per any-setter and reason —
after a mapper read. A
+ * handler that registers this module on its own mapper must call it in a
{@code finally} around
+ * every read; {@code ContentTypeInterceptor} clears the same state once
more when it unbinds the
+ * context.
*
* @since 7.4.0
*/
@@ -187,5 +189,6 @@ public class ParameterAuthorizingModule extends
SimpleModule {
public static void clearRequestState() {
DynamicKeyAuthorizationContext.clear();
AuthorizedForwardReferences.clear();
+ DynamicKeyRejections.reportAndClear();
}
}
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
index 55c2ab8ba..213b9edee 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
@@ -51,6 +51,12 @@ import com.fasterxml.jackson.dataformat.xml.JacksonXmlModule;
import com.fasterxml.jackson.dataformat.xml.XmlFactory;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import junit.framework.TestCase;
+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.logging.log4j.core.config.Configurator;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
import org.apache.struts2.interceptor.parameter.StrutsParameter;
@@ -70,17 +76,35 @@ import java.util.concurrent.atomic.AtomicReference;
public class ParameterAuthorizingModuleTest extends TestCase {
private ObjectMapper mapper;
+ private Logger rejectionLogger;
+ private Level rejectionLoggerLevel;
+ private RecordingAppender rejectionLog;
@Override
protected void setUp() {
mapper = new ObjectMapper().registerModule(new
ParameterAuthorizingModule());
+ rejectionLogger = (Logger)
LogManager.getLogger(DynamicKeyRejections.class);
+ rejectionLoggerLevel = rejectionLogger.getLevel();
+ Configurator.setLevel(rejectionLogger.getName(), Level.DEBUG);
+ rejectionLog = new RecordingAppender();
+ rejectionLog.start();
+ rejectionLogger.addAppender(rejectionLog);
}
@Override
protected void tearDown() {
+ rejectionLogger.removeAppender(rejectionLog);
+ rejectionLog.stop();
+ Configurator.setLevel(rejectionLogger.getName(), rejectionLoggerLevel);
ParameterAuthorizationContext.unbind();
- DynamicKeyAuthorizationContext.clear();
- AuthorizedForwardReferences.clear();
+ ParameterAuthorizingModule.clearRequestState();
+ }
+
+ private List<String> rejectionMessages(Level level) {
+ return rejectionLog.events.stream()
+ .filter(event -> event.getLevel() == level)
+ .map(event -> event.getMessage().getFormattedMessage())
+ .toList();
}
private void bind(ParameterAuthorizer authorizer, Object instance) {
@@ -198,6 +222,68 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
assertTrue(result.values.isEmpty());
}
+ public void testAnySetterRejectionsAreLoggedOncePerRequest() throws
Exception {
+ ObjectMapper enforcingMapper = enforcingMapper();
+ bind((path, t, a) -> true, new UnannotatedAnySetterBean());
+ enforcingMapper.readValue(
+ "{\"role\":\"admin\",\"level\":9,\"token\":\"x\"}",
UnannotatedAnySetterBean.class);
+
+ assertEquals(List.of(), rejectionMessages(Level.WARN));
+ List<String> detail = rejectionMessages(Level.DEBUG);
+ assertEquals(3, detail.size());
+ assertTrue(detail.get(0), detail.get(0).contains("[role]"));
+ assertTrue(detail.get(1), detail.get(1).contains("[level]"));
+ assertTrue(detail.get(2), detail.get(2).contains("[token]"));
+
+ ParameterAuthorizingModule.clearRequestState();
+
+ List<String> summary = rejectionMessages(Level.WARN);
+ assertEquals(1, summary.size());
+ String sink = UnannotatedAnySetterBean.class.getName() + "#put";
+ assertTrue(summary.get(0), summary.get(0).contains("[" + sink + "]"));
+ assertTrue(summary.get(0), summary.get(0).contains("[3] dynamic
key(s), logged at DEBUG;"));
+ assertTrue(summary.get(0),
summary.get(0).contains("@StrutsParameter(allowDynamicKeys = true)"));
+
+ ParameterAuthorizingModule.clearRequestState();
+ assertEquals(1, rejectionMessages(Level.WARN).size());
+ }
+
+ public void testDepthRejectionsAreSummarizedByReason() throws Exception {
+ ObjectMapper enforcingMapper = enforcingMapper();
+ bind((path, t, a) -> false, new DynamicDepthZeroAnySetterBean());
+ enforcingMapper.readValue(
+
"{\"home\":{\"city\":\"Warsaw\"},\"work\":{\"geo\":{\"country\":\"PL\"}}}",
+ DynamicDepthZeroAnySetterBean.class);
+
+ List<String> detail = rejectionMessages(Level.DEBUG);
+ assertEquals(2, detail.size());
+ assertTrue(detail.get(0), detail.get(0).contains("value depth [1]
exceeds @StrutsParameter depth [0]"));
+ assertTrue(detail.get(1), detail.get(1).contains("value depth [2]
exceeds @StrutsParameter depth [0]"));
+
+ ParameterAuthorizingModule.clearRequestState();
+
+ List<String> summary = rejectionMessages(Level.WARN);
+ assertEquals(1, summary.size());
+ assertTrue(summary.get(0), summary.get(0).contains("[2] dynamic
key(s), logged at DEBUG; value depth exceeds"));
+ }
+
+ public void testRejectionsAreSummarizedPerSink() throws Exception {
+ ObjectMapper enforcingMapper = enforcingMapper();
+ bind((path, t, a) -> "nested".equals(path), new
NestedAnySettersBean());
+ enforcingMapper.readValue(
+
"{\"role\":\"admin\",\"nested\":{\"role\":\"admin\",\"level\":9}}",
+ NestedAnySettersBean.class);
+
+ ParameterAuthorizingModule.clearRequestState();
+
+ List<String> summary = rejectionMessages(Level.WARN);
+ assertEquals(2, summary.size());
+ assertTrue(summary.get(0), summary.get(0).contains(
+ "[" + NestedAnySettersBean.class.getName() + "#put] rejected
[1]"));
+ assertTrue(summary.get(1), summary.get(1).contains(
+ "[" + UnannotatedAnySetterBean.class.getName() + "#put]
rejected [2]"));
+ }
+
public void testAnySetterWithoutDynamicKeyOptInRejected() throws Exception
{
ObjectMapper enforcingMapper = enforcingMapper();
bind((path, t, a) -> true, new AnnotatedAnySetterBean());
@@ -336,6 +422,13 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
CreatorAnySetterBean result = enforcingMapper.readValue(
"{\"role\":\"admin\"}", CreatorAnySetterBean.class);
assertTrue(result.values.isEmpty());
+
+ ParameterAuthorizingModule.clearRequestState();
+ List<String> summary = rejectionMessages(Level.WARN);
+ assertEquals(1, summary.size());
+ assertTrue(summary.get(0), summary.get(0).contains(
+ "[" + CreatorAnySetterBean.class.getName() + "#creator[0]]
rejected [1]"));
+ assertTrue(summary.get(0), summary.get(0).contains("can only be
declared on an any-setter method or field"));
}
public void testDeserializeWithoutCurrentNameRejectsAndClearsScope()
throws Exception {
@@ -368,6 +461,13 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
assertFalse(DynamicKeyAuthorizationContext.isActive());
assertEquals("", ParameterAuthorizationContext.currentPathPrefix());
+
+ ParameterAuthorizingModule.clearRequestState();
+ List<String> summary = rejectionMessages(Level.WARN);
+ assertEquals(1, summary.size());
+ assertTrue(summary.get(0), summary.get(0).contains(
+ "[" + PropertyCreatorWithAnySetterBean.class.getName() +
"#put] rejected [1]"));
+ assertTrue(summary.get(0), summary.get(0).contains("dynamic property
name is unavailable"));
}
public void testJacksonHandlerClearsDynamicScopeAfterReadFailure() throws
Exception {
@@ -1036,6 +1136,19 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
// --- Fixtures ---
+ private static final class RecordingAppender extends AbstractAppender {
+ private final List<LogEvent> events = new ArrayList<>();
+
+ private RecordingAppender() {
+ super("WW-5720", null, null, false, null);
+ }
+
+ @Override
+ public void append(LogEvent event) {
+ events.add(event.toImmutable());
+ }
+ }
+
private ObjectMapper enforcingMapper() {
return new ObjectMapper().registerModule(new
ParameterAuthorizingModule(true));
}
@@ -1420,6 +1533,16 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
}
}
+ public static class NestedAnySettersBean {
+ public final Map<String, Object> values = new LinkedHashMap<>();
+ public UnannotatedAnySetterBean nested;
+
+ @JsonAnySetter
+ public void put(String name, Object value) {
+ values.put(name, value);
+ }
+ }
+
public static class AnnotatedAnySetterBean {
public final Map<String, Object> values = new LinkedHashMap<>();
