From 428db20b3c2037b72e0efbfde79d932eac7b0847 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=B3=95=E2=B2=9B=CF=84=E2=B2=89=E2=B2=85=E2=B2=A5?=
 =?UTF-8?q?=E2=B2=89=E2=B3=8F=CF=84=E2=B2=9F=E2=B2=85=20=F0=9F=95=B5?=
 =?UTF-8?q?=F0=9F=8F=BB?= <192411347+g0w6y@users.noreply.github.com>
Date: Sat, 7 Nov 2026 17:35:14 +0530
Subject: [PATCH 1/2] 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.
---
 .../apache/struts2/json/JSONInterceptor.java  |  8 ++++-
 .../org/apache/struts2/json/JSONResult.java   |  5 ++++
 .../org/apache/struts2/json/JSONUtil.java     | 24 +++++++++++++++
 .../apache/struts2/json/JSONResultTest.java   | 29 +++++++++++++++++++
 .../org/apache/struts2/json/JSONUtilTest.java | 22 ++++++++++++++
 5 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 a2002c9aba..af4596e250 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,14 @@ private boolean checkSMDMethodSignature(Method method, String name, int paramete
     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("Rejecting JSONP callback parameter '{}' -- not a valid (dotted) "
+                            + "JS identifier, refusing to wrap the response with it", 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 f021a61ecf..e194def48a 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,11 @@ protected String addCallbackIfApplicable(HttpServletRequest request, String json
         if ((callbackParameter != null) && (callbackParameter.length() > 0)) {
             String callbackName = request.getParameter(callbackParameter);
             if (StringUtils.isNotEmpty(callbackName)) {
+                if (!JSONUtil.isValidJsonpCallbackName(callbackName)) {
+                    LOG.warn("Rejecting JSONP callback parameter '{}' -- not a valid (dotted) "
+                            + "JS identifier, refusing to wrap the response with it", 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 e1c20f7b1c..82bd49953e 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
@@ -565,6 +565,30 @@ public static boolean isGzipInRequest(HttpServletRequest request) {
         return StringUtils.contains(request.getHeader("Accept-Encoding"), "gzip");
     }
 
+    /**
+     * A JSONP {@code callback} request parameter value must be a (dotted) JavaScript
+     * identifier, matching every real-world JSONP server implementation's own validation.
+     * {@link JSONResult#addCallbackIfApplicable(HttpServletRequest, String)} and
+     * {@link JSONInterceptor#addCallbackIfApplicable(HttpServletRequest, String)} both
+     * concatenate the callback parameter's raw value directly around the serialized JSON
+     * ({@code callbackName + "(" + json + ")"}) with no check at all -- any character,
+     * including {@code )}, {@code ;}, and further script text, passes straight into the
+     * response body that a consumer loads as a {@code <script src="...">}. An unvalidated
+     * value there lets the parameter inject and execute arbitrary script in whatever page
+     * includes the JSONP endpoint, not merely a differently-named callback function.
+     *
+     * @param callbackName the raw {@code request.getParameter(callbackParameter)} value
+     * @return true if {@code callbackName} is safe to use as-is as a JS callback identifier
+     * @since 7.5.0
+     */
+    public static boolean isValidJsonpCallbackName(String callbackName) {
+        return callbackName != null && callbackName.length() <= 128
+                && JSONP_CALLBACK_PATTERN.matcher(callbackName).matches();
+    }
+
+    private static final Pattern JSONP_CALLBACK_PATTERN =
+            Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*(\\.[A-Za-z_$][A-Za-z0-9_$]*)*");
+
     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/JSONResultTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONResultTest.java
index a5c938f0aa..e89d0fe70c 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,35 @@ public void testJSONP() throws Exception {
         assertEquals("application/json;charset=UTF-8", response.getContentType());
     }
 
+    /**
+     * The callback parameter was concatenated into the response with no identifier check,
+     * so any character the client put in the callback query parameter -- not just a
+     * different function name -- landed directly in front of the JSON body. Benign probe
+     * only: "cb);STRUTS_PROBE_MARKER;(" is a marker string, not a working exploit payload;
+     * nothing here is executed as script.
+     */
+    public void testJSONPCallbackMustBeValidIdentifier() throws Exception {
+        JSONResult result = new JSONResult();
+        result.setCallbackParameter("callback");
+        JSONUtil jsonUtil = new JSONUtil();
+        jsonUtil.setWriter(new StrutsJSONWriter());
+        result.setJsonUtil(jsonUtil);
+
+        // Benign marker value standing in for arbitrary script text; a real identifier-only
+        // callback ("exec") is covered by testJSONP() above as the control case.
+        String notAnIdentifier = "cb);STRUTS_PROBE_MARKER;(";
+        request.addParameter("callback", notAnIdentifier);
+
+        executeTest2Action(result);
+        String json = response.getContentAsString();
+
+        // Fix: an invalid callback name must not be used to wrap the JSON -- the response
+        // must not contain the raw, non-identifier parameter value at all.
+        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 b2dcf8bff6..16c3387f7a 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,26 @@ public void testSerializeListOfList() throws Exception {
         assertEquals("{\"listOfLists\":[[\"1\",\"2\"],[\"3\",\"4\"],[\"5\",\"6\"],[\"7\",\"8\"],[\"9\",\"0\"]]}", actual);
     }
 
+    /**
+     * Shared helper used by both JSONP sink sites ({@link JSONResult#addCallbackIfApplicable}
+     * and {@link JSONInterceptor#addCallbackIfApplicable}) to reject a callback parameter
+     * value that is not a plain (dotted) JS identifier -- the check both call sites were
+     * missing.
+     */
+    public void testIsValidJsonpCallbackName() {
+        // control: identifiers real JSONP clients actually send
+        assertTrue(JSONUtil.isValidJsonpCallbackName("exec"));
+        assertTrue(JSONUtil.isValidJsonpCallbackName("_cb$1"));
+        assertTrue(JSONUtil.isValidJsonpCallbackName("jQuery123.ajaxCallback"));
+
+        // benign marker values standing in for "the callback parameter carries more than a
+        // function name" -- none of these are real exploit payloads
+        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)));
+    }
+
 }

From 89ea1696fcf5ca91f1bde10731d65ff38f67cc66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=B3=95=E2=B2=9B=CF=84=E2=B2=89=E2=B2=85=E2=B2=A5?=
 =?UTF-8?q?=E2=B2=89=E2=B3=8F=CF=84=E2=B2=9F=E2=B2=85=20=F0=9F=95=B5?=
 =?UTF-8?q?=F0=9F=8F=BB?= <192411347+g0w6y@users.noreply.github.com>
Date: Sun, 8 Nov 2026 08:55:42 +0530
Subject: [PATCH 2/2] 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
---
 .../apache/struts2/json/JSONInterceptor.java  |  3 +-
 .../org/apache/struts2/json/JSONResult.java   |  3 +-
 .../org/apache/struts2/json/JSONUtil.java     | 22 +++++---------
 .../struts2/json/JSONInterceptorTest.java     | 29 +++++++++++++++++++
 .../apache/struts2/json/JSONResultTest.java   | 12 +-------
 .../org/apache/struts2/json/JSONUtilTest.java |  9 ------
 6 files changed, 39 insertions(+), 39 deletions(-)

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 af4596e250..53d16dea96 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
@@ -543,8 +543,7 @@ protected String addCallbackIfApplicable(HttpServletRequest request, String json
             String callbackName = request.getParameter(callbackParameter);
             if ((callbackName != null) && (callbackName.length() > 0)) {
                 if (!JSONUtil.isValidJsonpCallbackName(callbackName)) {
-                    LOG.warn("Rejecting JSONP callback parameter '{}' -- not a valid (dotted) "
-                            + "JS identifier, refusing to wrap the response with it", callbackName);
+                    LOG.warn("JSONP callback parameter [{}] is not a valid identifier; rejected", normalizeSpace(callbackName));
                     return json;
                 }
                 json = callbackName + "(" + 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 e194def48a..6c17e82e8f 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
@@ -283,8 +283,7 @@ protected String addCallbackIfApplicable(HttpServletRequest request, String json
             String callbackName = request.getParameter(callbackParameter);
             if (StringUtils.isNotEmpty(callbackName)) {
                 if (!JSONUtil.isValidJsonpCallbackName(callbackName)) {
-                    LOG.warn("Rejecting JSONP callback parameter '{}' -- not a valid (dotted) "
-                            + "JS identifier, refusing to wrap the response with it", 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 82bd49953e..c0c8ff0069 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;
 
@@ -566,19 +569,11 @@ public static boolean isGzipInRequest(HttpServletRequest request) {
     }
 
     /**
-     * A JSONP {@code callback} request parameter value must be a (dotted) JavaScript
-     * identifier, matching every real-world JSONP server implementation's own validation.
-     * {@link JSONResult#addCallbackIfApplicable(HttpServletRequest, String)} and
-     * {@link JSONInterceptor#addCallbackIfApplicable(HttpServletRequest, String)} both
-     * concatenate the callback parameter's raw value directly around the serialized JSON
-     * ({@code callbackName + "(" + json + ")"}) with no check at all -- any character,
-     * including {@code )}, {@code ;}, and further script text, passes straight into the
-     * response body that a consumer loads as a {@code <script src="...">}. An unvalidated
-     * value there lets the parameter inject and execute arbitrary script in whatever page
-     * includes the JSONP endpoint, not merely a differently-named callback function.
+     * 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 raw {@code request.getParameter(callbackParameter)} value
-     * @return true if {@code callbackName} is safe to use as-is as a JS callback identifier
+     * @param callbackName the requested callback name
+     * @return true if {@code callbackName} has that shape
      * @since 7.5.0
      */
     public static boolean isValidJsonpCallbackName(String callbackName) {
@@ -586,9 +581,6 @@ public static boolean isValidJsonpCallbackName(String callbackName) {
                 && JSONP_CALLBACK_PATTERN.matcher(callbackName).matches();
     }
 
-    private static final Pattern JSONP_CALLBACK_PATTERN =
-            Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*(\\.[A-Za-z_$][A-Za-z0-9_$]*)*");
-
     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 e7508fee41..1dcfeafb02 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 void testSMDPrimitivesNoResult() throws Exception {
         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 e89d0fe70c..7f9c8c5368 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,13 +539,7 @@ public void testJSONP() throws Exception {
         assertEquals("application/json;charset=UTF-8", response.getContentType());
     }
 
-    /**
-     * The callback parameter was concatenated into the response with no identifier check,
-     * so any character the client put in the callback query parameter -- not just a
-     * different function name -- landed directly in front of the JSON body. Benign probe
-     * only: "cb);STRUTS_PROBE_MARKER;(" is a marker string, not a working exploit payload;
-     * nothing here is executed as script.
-     */
+    // control case: testJSONP()
     public void testJSONPCallbackMustBeValidIdentifier() throws Exception {
         JSONResult result = new JSONResult();
         result.setCallbackParameter("callback");
@@ -553,16 +547,12 @@ public void testJSONPCallbackMustBeValidIdentifier() throws Exception {
         jsonUtil.setWriter(new StrutsJSONWriter());
         result.setJsonUtil(jsonUtil);
 
-        // Benign marker value standing in for arbitrary script text; a real identifier-only
-        // callback ("exec") is covered by testJSONP() above as the control case.
         String notAnIdentifier = "cb);STRUTS_PROBE_MARKER;(";
         request.addParameter("callback", notAnIdentifier);
 
         executeTest2Action(result);
         String json = response.getContentAsString();
 
-        // Fix: an invalid callback name must not be used to wrap the JSON -- the response
-        // must not contain the raw, non-identifier parameter value at all.
         assertFalse("Non-identifier callback value must not be concatenated into the response",
                 json.contains(notAnIdentifier));
         assertFalse(json.contains("STRUTS_PROBE_MARKER"));
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 16c3387f7a..d9dca84d9e 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,20 +79,11 @@ public void testSerializeListOfList() throws Exception {
         assertEquals("{\"listOfLists\":[[\"1\",\"2\"],[\"3\",\"4\"],[\"5\",\"6\"],[\"7\",\"8\"],[\"9\",\"0\"]]}", actual);
     }
 
-    /**
-     * Shared helper used by both JSONP sink sites ({@link JSONResult#addCallbackIfApplicable}
-     * and {@link JSONInterceptor#addCallbackIfApplicable}) to reject a callback parameter
-     * value that is not a plain (dotted) JS identifier -- the check both call sites were
-     * missing.
-     */
     public void testIsValidJsonpCallbackName() {
-        // control: identifiers real JSONP clients actually send
         assertTrue(JSONUtil.isValidJsonpCallbackName("exec"));
         assertTrue(JSONUtil.isValidJsonpCallbackName("_cb$1"));
         assertTrue(JSONUtil.isValidJsonpCallbackName("jQuery123.ajaxCallback"));
 
-        // benign marker values standing in for "the callback parameter carries more than a
-        // function name" -- none of these are real exploit payloads
         assertFalse(JSONUtil.isValidJsonpCallbackName("cb);STRUTS_PROBE_MARKER;("));
         assertFalse(JSONUtil.isValidJsonpCallbackName("cb\nSTRUTS_PROBE_MARKER"));
         assertFalse(JSONUtil.isValidJsonpCallbackName("</script><STRUTS_PROBE_MARKER>"));