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 91e7815f3 WW-5845 Validate JSONP callback names (#2121)
91e7815f3 is described below
commit 91e7815f32251359a174b802ae37ae0bdc468a67
Author: g0w6y <[email protected]>
AuthorDate: Sun Nov 8 20:30:51 2026 +0530
WW-5845 Validate JSONP callback names (#2121)
* WW-5845 Validate the JSONP callback parameter name before wrapping the
response in it
JSONResult.addCallbackIfApplicable and
JSONInterceptor.addCallbackIfApplicable
built the JSONP wrapper as callbackName + "(" + json + ")", where
callbackName is the raw, unvalidated callbackParameter query value -- no
character restriction. Since the response is meant to be loaded as
<script src=...>, an unvalidated value there is more than a
differently-named callback function.
Adds a shared JSONUtil.isValidJsonpCallbackName() check
(^[A-Za-z_$][A-Za-z0-9_$]*(\.[A-Za-z_$][A-Za-z0-9_$]*)*$, length capped
at 128) at both call sites. An invalid callback name falls back to the
unwrapped JSON response, logged at WARN.
Tests: JSONUtilTest#testIsValidJsonpCallbackName,
JSONResultTest#testJSONPCallbackMustBeValidIdentifier (new, confirmed to
fail without the fix with a genuine assertion mismatch). Existing
JSONResultTest, JSONUtilTest, JSONInterceptorTest unaffected. Full
plugins/json suite: 179/0 failures.
* WW-5845 Address review: normalize the logged callback name, cover the SMD
arm, trim comments
- log the rejected callback name through normalizeSpace in JSONInterceptor
and JSONResult, like the other request-derived values in these classes
- JSONInterceptorTest: add a rejection and a control test for the SMD arm
- reduce the JSONUtil Javadoc and the new test comments to the contract
- move the callback pattern next to the other static final fields
---
.../org/apache/struts2/json/JSONInterceptor.java | 7 +++++-
.../java/org/apache/struts2/json/JSONResult.java | 4 +++
.../java/org/apache/struts2/json/JSONUtil.java | 16 ++++++++++++
.../apache/struts2/json/JSONInterceptorTest.java | 29 ++++++++++++++++++++++
.../org/apache/struts2/json/JSONResultTest.java | 19 ++++++++++++++
.../java/org/apache/struts2/json/JSONUtilTest.java | 13 ++++++++++
6 files changed, 87 insertions(+), 1 deletion(-)
diff --git
a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
index a2002c9ab..53d16dea9 100644
--- a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
+++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
@@ -541,8 +541,13 @@ public class JSONInterceptor extends AbstractInterceptor {
protected String addCallbackIfApplicable(HttpServletRequest request,
String json) {
if ((callbackParameter != null) && (callbackParameter.length() > 0)) {
String callbackName = request.getParameter(callbackParameter);
- if ((callbackName != null) && (callbackName.length() > 0))
+ if ((callbackName != null) && (callbackName.length() > 0)) {
+ if (!JSONUtil.isValidJsonpCallbackName(callbackName)) {
+ LOG.warn("JSONP callback parameter [{}] is not a valid
identifier; rejected", normalizeSpace(callbackName));
+ return json;
+ }
json = callbackName + "(" + json + ")";
+ }
}
return json;
}
diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java
b/plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java
index f021a61ec..6c17e82e8 100644
--- a/plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java
+++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java
@@ -282,6 +282,10 @@ public class JSONResult implements Result {
if ((callbackParameter != null) && (callbackParameter.length() > 0)) {
String callbackName = request.getParameter(callbackParameter);
if (StringUtils.isNotEmpty(callbackName)) {
+ if (!JSONUtil.isValidJsonpCallbackName(callbackName)) {
+ LOG.warn("JSONP callback parameter [{}] is not a valid
identifier; rejected", StringUtils.normalizeSpace(callbackName));
+ return json;
+ }
json = callbackName + "(" + json + ")";
}
}
diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java
b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java
index e1c20f7b1..c0c8ff006 100644
--- a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java
+++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java
@@ -62,6 +62,9 @@ public class JSONUtil {
/** Chunk size used to read input incrementally while applying the length
limit. */
private static final int READ_CHUNK_SIZE = 8192;
+ private static final Pattern JSONP_CALLBACK_PATTERN =
+
Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*(\\.[A-Za-z_$][A-Za-z0-9_$]*)*");
+
private JSONReader reader;
private JSONWriter writer;
@@ -565,6 +568,19 @@ public class JSONUtil {
return StringUtils.contains(request.getHeader("Accept-Encoding"),
"gzip");
}
+ /**
+ * Checks a JSONP callback name: a dotted ASCII identifier ({@code
[A-Za-z_$][A-Za-z0-9_$]*},
+ * segments joined by {@code .}) of at most 128 characters.
+ *
+ * @param callbackName the requested callback name
+ * @return true if {@code callbackName} has that shape
+ * @since 7.5.0
+ */
+ public static boolean isValidJsonpCallbackName(String callbackName) {
+ return callbackName != null && callbackName.length() <= 128
+ && JSONP_CALLBACK_PATTERN.matcher(callbackName).matches();
+ }
+
public static final String REGEXP_PATTERN = "regexp";
public static final String WILDCARD_PATTERN = "wildcard";
/* package */ static final String SPLIT_PATTERN = "split";
diff --git
a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
index e7508fee4..1dcfeafb0 100644
---
a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
+++
b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
@@ -240,6 +240,35 @@ public class JSONInterceptorTest extends StrutsTestCase {
assertEquals("application/json;charset=UTF-8",
response.getContentType());
}
+ public void testSMDInvalidJSONPCallbackLeavesJsonUnwrapped() throws
Exception {
+ String json = executeSMDWithCallback("cb);STRUTS_PROBE_MARKER;(");
+
+
assertEquals(TestUtils.normalize(JSONResultTest.class.getResource("smd-11.txt")),
TestUtils.normalize(json, true));
+ assertFalse(json.contains("STRUTS_PROBE_MARKER"));
+ }
+
+ public void testSMDValidJSONPCallbackWrapsJson() throws Exception {
+ String json = executeSMDWithCallback("exec");
+
+ assertTrue(json.startsWith("exec(") && json.endsWith(")"));
+
assertEquals(TestUtils.normalize(JSONResultTest.class.getResource("smd-11.txt")),
+ TestUtils.normalize(json.substring("exec(".length(),
json.length() - 1), true));
+ }
+
+ private String executeSMDWithCallback(String callback) throws Exception {
+ setRequestContent("smd-6.txt");
+ this.request.addHeader("Content-Type", "application/json-rpc");
+ this.request.addParameter("callback", callback);
+
+ JSONInterceptor interceptor = createInterceptor();
+ interceptor.setEnableSMD(true);
+ interceptor.setCallbackParameter("callback");
+ this.invocation.setAction(new SMDActionTest1());
+
+ interceptor.intercept(this.invocation);
+ return response.getContentAsString();
+ }
+
public void testSMDReturnObject() throws Exception {
// request
setRequestContent("smd-10.txt");
diff --git
a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java
b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java
index a5c938f0a..7f9c8c536 100644
--- a/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java
+++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java
@@ -539,6 +539,25 @@ public class JSONResultTest extends StrutsTestCase {
assertEquals("application/json;charset=UTF-8",
response.getContentType());
}
+ // control case: testJSONP()
+ public void testJSONPCallbackMustBeValidIdentifier() throws Exception {
+ JSONResult result = new JSONResult();
+ result.setCallbackParameter("callback");
+ JSONUtil jsonUtil = new JSONUtil();
+ jsonUtil.setWriter(new StrutsJSONWriter());
+ result.setJsonUtil(jsonUtil);
+
+ String notAnIdentifier = "cb);STRUTS_PROBE_MARKER;(";
+ request.addParameter("callback", notAnIdentifier);
+
+ executeTest2Action(result);
+ String json = response.getContentAsString();
+
+ assertFalse("Non-identifier callback value must not be concatenated
into the response",
+ json.contains(notAnIdentifier));
+ assertFalse(json.contains("STRUTS_PROBE_MARKER"));
+ }
+
public void testNoCache() throws Exception {
JSONResult result = new JSONResult();
result.setNoCache(true);
diff --git
a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java
b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java
index b2dcf8bff..d9dca84d9 100644
--- a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java
+++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilTest.java
@@ -79,4 +79,17 @@ public class JSONUtilTest extends TestCase {
assertEquals("{\"listOfLists\":[[\"1\",\"2\"],[\"3\",\"4\"],[\"5\",\"6\"],[\"7\",\"8\"],[\"9\",\"0\"]]}",
actual);
}
+ public void testIsValidJsonpCallbackName() {
+ assertTrue(JSONUtil.isValidJsonpCallbackName("exec"));
+ assertTrue(JSONUtil.isValidJsonpCallbackName("_cb$1"));
+
assertTrue(JSONUtil.isValidJsonpCallbackName("jQuery123.ajaxCallback"));
+
+
assertFalse(JSONUtil.isValidJsonpCallbackName("cb);STRUTS_PROBE_MARKER;("));
+
assertFalse(JSONUtil.isValidJsonpCallbackName("cb\nSTRUTS_PROBE_MARKER"));
+
assertFalse(JSONUtil.isValidJsonpCallbackName("</script><STRUTS_PROBE_MARKER>"));
+ assertFalse(JSONUtil.isValidJsonpCallbackName(""));
+ assertFalse(JSONUtil.isValidJsonpCallbackName(null));
+ assertFalse(JSONUtil.isValidJsonpCallbackName("a".repeat(129)));
+ }
+
}
