This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new ea2c3ee57ce restrict jsonp callback names in JsonpInInterceptor (#3628)
ea2c3ee57ce is described below
commit ea2c3ee57ceef29c6d79d609009cc153522e9136
Author: Naveed Khan <[email protected]>
AuthorDate: Thu Oct 29 14:22:54 2026 +0530
restrict jsonp callback names in JsonpInInterceptor (#3628)
* restrict jsonp callback names in JsonpInInterceptor
* validate the jsonp callback in handleMessage and harden the jsonp output
Move the pattern check into JsonpInInterceptor.handleMessage so subclasses
overriding getCallbackValue are covered, make the pattern configurable via
setCallbackPattern with a 128 character cap, prefix the padded output with
/**/ and set X-Content-Type-Options: nosniff on JSONP responses.
* make the jsonp padding start configurable and handle a null callback
pattern
* make the jsonp nosniff header configurable
---
.../jaxrs/provider/jsonp/JsonpInInterceptor.java | 25 ++++
.../jsonp/JsonpJaxrsWriterInterceptor.java | 24 +++-
.../provider/jsonp/JsonpPreStreamInterceptor.java | 24 +++-
.../jaxrs/provider/jsonp/JsonpInterceptorTest.java | 146 ++++++++++++++++++++-
.../jaxrs/JAXRSClientServerSpringBookTest.java | 5 +-
5 files changed, 217 insertions(+), 7 deletions(-)
diff --git
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java
index 8ae30d17d33..10179e74799 100644
---
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java
+++
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInInterceptor.java
@@ -19,6 +19,8 @@
package org.apache.cxf.jaxrs.provider.jsonp;
+import java.util.regex.Pattern;
+
import org.apache.cxf.common.util.StringUtils;
import org.apache.cxf.interceptor.Fault;
import org.apache.cxf.jaxrs.utils.JAXRSUtils;
@@ -35,9 +37,14 @@ public class JsonpInInterceptor extends
AbstractPhaseInterceptor<Message> {
public static final String CALLBACK_PARAM = "_jsonp";
public static final String CALLBACK_KEY = "JSONP.CALLBACK";
public static final String DEFAULT_CALLBACK_VALUE = "callback";
+ // The callback is written verbatim in front of the JSON, so only allow
+ // letters, digits, '_', '$' and '.' (enough for plain and dotted callback
+ // names such as angular.callbacks._0) up to a bounded length
+ public static final String DEFAULT_CALLBACK_PATTERN =
"[A-Za-z0-9_$.]{1,128}";
private String callbackParam = CALLBACK_PARAM;
private String defaultCallback = DEFAULT_CALLBACK_VALUE;
+ private Pattern callbackPattern =
Pattern.compile(DEFAULT_CALLBACK_PATTERN);
private String acceptType;
private String mediaType = JSONP_TYPE;
@@ -52,6 +59,10 @@ public class JsonpInInterceptor extends
AbstractPhaseInterceptor<Message> {
public void handleMessage(Message message) throws Fault {
String callbackValue = getCallbackValue(message);
if (!StringUtils.isEmpty(callbackValue)) {
+ if (!callbackPattern.matcher(callbackValue).matches()) {
+ // not a usable callback name, serve plain JSON
+ return;
+ }
if (getAcceptType() != null) {
// may be needed to enforce the selection of
// JSON-aware provider
@@ -104,4 +115,18 @@ public class JsonpInInterceptor extends
AbstractPhaseInterceptor<Message> {
public String getDefaultCallback() {
return defaultCallback;
}
+
+ /**
+ * Sets the regular expression a callback name must match in full.
+ * Values which do not match are ignored and the response is plain JSON.
+ * A null value restores {@link #DEFAULT_CALLBACK_PATTERN}.
+ */
+ public void setCallbackPattern(String callbackPattern) {
+ this.callbackPattern = Pattern.compile(callbackPattern == null
+ ? DEFAULT_CALLBACK_PATTERN : callbackPattern);
+ }
+
+ public String getCallbackPattern() {
+ return callbackPattern.pattern();
+ }
}
diff --git
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java
index cfa8c72d385..e403a0b1bdf 100644
---
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java
+++
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpJaxrsWriterInterceptor.java
@@ -36,7 +36,9 @@ import org.apache.cxf.message.Message;
public class JsonpJaxrsWriterInterceptor implements WriterInterceptor {
private String mediaType = JsonpInInterceptor.JSONP_TYPE;
+ private String paddingStart =
JsonpPreStreamInterceptor.DEFAULT_PADDING_START;
private String paddingEnd = "(";
+ private boolean addNosniffHeader = true;
public JsonpJaxrsWriterInterceptor() {
}
@@ -47,7 +49,11 @@ public class JsonpJaxrsWriterInterceptor implements
WriterInterceptor {
if (!StringUtils.isEmpty(callback)) {
context.getHeaders().putSingle(Message.CONTENT_TYPE,
JAXRSUtils.toMediaType(getMediaType()));
- context.getOutputStream().write((callback +
getPaddingEnd()).getBytes(StandardCharsets.UTF_8));
+ if (isAddNosniffHeader()) {
+ context.getHeaders().putSingle("X-Content-Type-Options",
"nosniff");
+ }
+ context.getOutputStream().write((getPaddingStart() + callback +
getPaddingEnd())
+
.getBytes(StandardCharsets.UTF_8));
}
context.proceed();
}
@@ -60,6 +66,14 @@ public class JsonpJaxrsWriterInterceptor implements
WriterInterceptor {
return mediaType;
}
+ public void setPaddingStart(String paddingStart) {
+ this.paddingStart = paddingStart;
+ }
+
+ public String getPaddingStart() {
+ return paddingStart;
+ }
+
public void setPaddingEnd(String paddingEnd) {
this.paddingEnd = paddingEnd;
}
@@ -68,6 +82,14 @@ public class JsonpJaxrsWriterInterceptor implements
WriterInterceptor {
return paddingEnd;
}
+ public void setAddNosniffHeader(boolean addNosniffHeader) {
+ this.addNosniffHeader = addNosniffHeader;
+ }
+
+ public boolean isAddNosniffHeader() {
+ return addNosniffHeader;
+ }
+
protected String getCallbackValue(Message message) {
Exchange exchange = message.getExchange();
return (String) exchange.get(JsonpInInterceptor.CALLBACK_KEY);
diff --git
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java
index 16f77ba7202..d9d3fcafca7 100644
---
a/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java
+++
b/rt/rs/extensions/providers/src/main/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpPreStreamInterceptor.java
@@ -40,9 +40,12 @@ import org.apache.cxf.phase.PhaseInterceptorChain;
* Appends the jsonp callback to json responses when the '_jsonp' parameter
has been set in the querystring.
*/
public class JsonpPreStreamInterceptor extends AbstractJsonpOutInterceptor
implements WriterInterceptor {
+ public static final String DEFAULT_PADDING_START = "/**/";
private String mediaType = JsonpInInterceptor.JSONP_TYPE;
+ private String paddingStart = DEFAULT_PADDING_START;
private String paddingEnd = "(";
+ private boolean addNosniffHeader = true;
public JsonpPreStreamInterceptor() {
super(Phase.PRE_STREAM);
@@ -52,7 +55,7 @@ public class JsonpPreStreamInterceptor extends
AbstractJsonpOutInterceptor imple
String callback = getCallbackValue(message);
if (!StringUtils.isEmpty(callback)) {
setContentType(message);
- writeValue(message, callback + getPaddingEnd());
+ writeValue(message, getPaddingStart() + callback +
getPaddingEnd());
}
}
@@ -64,6 +67,9 @@ public class JsonpPreStreamInterceptor extends
AbstractJsonpOutInterceptor imple
message.put(Message.PROTOCOL_HEADERS, headers);
}
headers.put(Message.CONTENT_TYPE,
Collections.singletonList(getMediaType()));
+ if (isAddNosniffHeader()) {
+ headers.put("X-Content-Type-Options",
Collections.singletonList("nosniff"));
+ }
}
public void setMediaType(String mediaType) {
@@ -74,6 +80,14 @@ public class JsonpPreStreamInterceptor extends
AbstractJsonpOutInterceptor imple
return mediaType;
}
+ public void setPaddingStart(String paddingStart) {
+ this.paddingStart = paddingStart;
+ }
+
+ public String getPaddingStart() {
+ return paddingStart;
+ }
+
public void setPaddingEnd(String paddingEnd) {
this.paddingEnd = paddingEnd;
}
@@ -82,6 +96,14 @@ public class JsonpPreStreamInterceptor extends
AbstractJsonpOutInterceptor imple
return paddingEnd;
}
+ public void setAddNosniffHeader(boolean addNosniffHeader) {
+ this.addNosniffHeader = addNosniffHeader;
+ }
+
+ public boolean isAddNosniffHeader() {
+ return addNosniffHeader;
+ }
+
@Override
public void aroundWriteTo(WriterInterceptorContext context) throws
IOException, WebApplicationException {
handleMessage(PhaseInterceptorChain.getCurrentMessage());
diff --git
a/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java
b/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java
index d75bf1cfb18..531b78dbe45 100644
---
a/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java
+++
b/rt/rs/extensions/providers/src/test/java/org/apache/cxf/jaxrs/provider/jsonp/JsonpInterceptorTest.java
@@ -21,6 +21,9 @@ package org.apache.cxf.jaxrs.provider.jsonp;
import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
import jakarta.ws.rs.core.MediaType;
import org.apache.cxf.message.ExchangeImpl;
@@ -31,6 +34,7 @@ import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
public class JsonpInterceptorTest {
@@ -61,7 +65,10 @@ public class JsonpInterceptorTest {
in.handleMessage(message);
preStream.handleMessage(message);
postStream.handleMessage(message);
- assertEquals("myCallback();", bos.toString());
+ assertEquals("/**/myCallback();", bos.toString());
+ @SuppressWarnings("unchecked")
+ Map<String, List<String>> headers = (Map<String,
List<String>>)message.get(Message.PROTOCOL_HEADERS);
+ assertEquals(Collections.singletonList("nosniff"),
headers.get("X-Content-Type-Options"));
}
@@ -80,13 +87,146 @@ public class JsonpInterceptorTest {
in.handleMessage(message);
preStream.handleMessage(message);
postStream.handleMessage(message);
- assertEquals("myCallback();", bos.toString());
+ assertEquals("/**/myCallback();", bos.toString());
} finally {
in.setCallbackParam("_jsonp");
}
}
+ @Test
+ public void testJsonWithoutPaddingStart() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "myCallback");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ preStream.setPaddingStart("");
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertEquals("myCallback();", bos.toString());
+ }
+
+ @Test
+ public void testJsonWithoutNosniffHeader() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "myCallback");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ preStream.setAddNosniffHeader(false);
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertEquals("/**/myCallback();", bos.toString());
+ @SuppressWarnings("unchecked")
+ Map<String, List<String>> headers = (Map<String,
List<String>>)message.get(Message.PROTOCOL_HEADERS);
+ assertNull(headers.get("X-Content-Type-Options"));
+ }
+
+ @Test
+ public void testNullCallbackPatternRestoresDefault() throws Exception {
+ in.setCallbackPattern("[a-z]+");
+ in.setCallbackPattern(null);
+ assertEquals(JsonpInInterceptor.DEFAULT_CALLBACK_PATTERN,
in.getCallbackPattern());
+ }
+
+ @Test
+ public void testJsonWithDottedCallback() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "angular.callbacks._0");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertEquals("/**/angular.callbacks._0();", bos.toString());
+ }
+
+ @Test
+ public void testJsonWithCustomCallbackPattern() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "angular.callbacks._0");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ try {
+ in.setCallbackPattern("[A-Za-z0-9_]{1,32}");
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+
assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY));
+ assertEquals("", bos.toString());
+ } finally {
+ in.setCallbackPattern(JsonpInInterceptor.DEFAULT_CALLBACK_PATTERN);
+ }
+ }
+
+ @Test
+ public void testJsonWithInvalidCallback() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "alert(document.domain)//");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY));
+ assertEquals("", bos.toString());
+ }
+
+ @Test
+ public void testJsonWithInvalidCallbackAndJsonpAccept() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.ACCEPT_CONTENT_TYPE,
JsonpInInterceptor.JSONP_TYPE);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "alert(document.domain)//");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message: a rejected callback must not fall back to the
default one
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY));
+ assertEquals("", bos.toString());
+ }
+
+ @Test
+ public void testJsonWithOverlongCallback() throws Exception {
+ Message message = new MessageImpl();
+ message.put(Message.CONTENT_TYPE, MediaType.APPLICATION_JSON);
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.QUERY_STRING, JsonpInInterceptor.CALLBACK_PARAM +
"=" + "a".repeat(129));
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, bos);
+
+ // Process the message
+ in.handleMessage(message);
+ preStream.handleMessage(message);
+ postStream.handleMessage(message);
+ assertNull(message.getExchange().get(JsonpInInterceptor.CALLBACK_KEY));
+ assertEquals("", bos.toString());
+ }
+
@Test
public void testJsonWithDefaultPadding() throws Exception {
Message message = new MessageImpl();
@@ -99,7 +239,7 @@ public class JsonpInterceptorTest {
in.handleMessage(message);
preStream.handleMessage(message);
postStream.handleMessage(message);
- assertEquals("callback();", bos.toString());
+ assertEquals("/**/callback();", bos.toString());
}
@Test
diff --git
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java
index 7e39104700a..98bfe27dcec 100644
---
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java
+++
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerSpringBookTest.java
@@ -601,7 +601,8 @@ public class JAXRSClientServerSpringBookTest extends
AbstractBusClientServerTest
client.query("_jsonp", "callback");
Response r = client.get();
assertEquals("application/x-javascript",
r.getMetadata().getFirst("Content-Type"));
- assertEquals("callback({\"Book\":{\"id\":123,\"name\":\"CXF in
Action\"}});",
+ assertEquals("nosniff",
r.getMetadata().getFirst("X-Content-Type-Options"));
+ assertEquals("/**/callback({\"Book\":{\"id\":123,\"name\":\"CXF in
Action\"}});",
IOUtils.readStringFromStream((InputStream)r.getEntity()));
}
@@ -615,7 +616,7 @@ public class JAXRSClientServerSpringBookTest extends
AbstractBusClientServerTest
Response r = client.get();
assertEquals("application/x-javascript",
r.getMetadata().getFirst("Content-Type"));
String response =
IOUtils.readStringFromStream((InputStream)r.getEntity());
-
assertTrue(response.startsWith("callback({\"class\":\"org.apache.cxf.systest.jaxrs.Book\","));
+
assertTrue(response.startsWith("/**/callback({\"class\":\"org.apache.cxf.systest.jaxrs.Book\","));
assertTrue(response.endsWith("});"));
assertTrue(response.contains("\"id\":123"));
assertTrue(response.contains("\"name\":\"CXF in Action\""));
