diff --git a/conf/defaults.yaml b/conf/defaults.yaml index 9682cf8bcaf..dff2ff7bc7a 100644 --- a/conf/defaults.yaml +++ b/conf/defaults.yaml @@ -117,6 +117,7 @@ ui.http.creds.plugin: org.apache.storm.security.auth.DefaultHttpCredentialsPlugi ui.pagination: 20 ui.disable.http.binding: true ui.disable.spout.lag.monitoring: true +ui.enable.jsonp: false logviewer.port: 8000 logviewer.childopts: "-Xmx128m" diff --git a/docs/STORM-UI-REST-API.md b/docs/STORM-UI-REST-API.md index c035e76d41f..4dd6c501c88 100644 --- a/docs/STORM-UI-REST-API.md +++ b/docs/STORM-UI-REST-API.md @@ -13,6 +13,7 @@ metrics data and configuration information as well as management operations such The REST API returns JSON responses and supports JSONP. Clients can pass a callback query parameter to wrap JSON in the callback function. +JSONP is disabled by default; the callback parameter is ignored unless `ui.enable.jsonp` is set to true. # Using the UI REST API diff --git a/storm-server/src/main/java/org/apache/storm/DaemonConfig.java b/storm-server/src/main/java/org/apache/storm/DaemonConfig.java index 6dcc2eb37f2..3b42abd49d5 100644 --- a/storm-server/src/main/java/org/apache/storm/DaemonConfig.java +++ b/storm-server/src/main/java/org/apache/storm/DaemonConfig.java @@ -395,6 +395,14 @@ public class DaemonConfig implements Validated { @IsBoolean public static final String UI_DISABLE_SPOUT_LAG_MONITORING = "ui.disable.spout.lag.monitoring"; + /** + * This controls whether the Storm UI and Logviewer REST APIs wrap their response in the + * JSONP callback named by the "callback" query parameter. It is disabled by default, since + * a JSONP response can be read by any page that is able to include it with a script tag. + */ + @IsBoolean + public static final String UI_ENABLE_JSONP = "ui.enable.jsonp"; + /** * This controls wheather Storm Logviewer should bind to http port even if logviewer.port is > 0. */ diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java index 2e4f64f2f1b..3f50e9f0405 100644 --- a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java +++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java @@ -18,6 +18,7 @@ package org.apache.storm.daemon.ui; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -89,6 +90,7 @@ import org.apache.storm.scheduler.resource.normalization.NormalizedResourceRequest; import org.apache.storm.stats.StatsUtil; import org.apache.storm.thrift.TException; +import org.apache.storm.utils.ConfigUtils; import org.apache.storm.utils.IVersionInfo; import org.apache.storm.utils.ObjectReader; import org.apache.storm.utils.Time; @@ -447,10 +449,27 @@ public static void stormRunJetty(Integer port, Integer headerBufferSize, private static final Pattern JSONP_CALLBACK_PATTERN = Pattern.compile("^[A-Za-z_$][A-Za-z0-9_$]*(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)*$"); + /** + * Whether the "callback" query parameter is honored, see {@link DaemonConfig#UI_ENABLE_JSONP}. + * It is read once, like the rest of the daemon configuration, so a change needs a restart. + */ + private static boolean jsonpEnabled = + ObjectReader.getBoolean(ConfigUtils.readStormConfig().get(DaemonConfig.UI_ENABLE_JSONP), false); + + @VisibleForTesting + static void setJsonpEnabled(boolean enabled) { + jsonpEnabled = enabled; + } + private static String sanitizeJsonpCallback(String callback) { if (callback == null) { return null; } + if (!jsonpEnabled) { + LOG.warn("Ignoring JSONP callback parameter, set {} to true to enable JSONP responses", + DaemonConfig.UI_ENABLE_JSONP); + return null; + } if (callback.length() > 128 || !JSONP_CALLBACK_PATTERN.matcher(callback).matches()) { LOG.warn("Ignoring invalid JSONP callback parameter"); return null; diff --git a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java index 852af5cb972..d2fef3aa684 100644 --- a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java +++ b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java @@ -102,6 +102,8 @@ void setup() { void cleanup() { // Stop simulating time mockTime.close(); + // Restore the default of ui.enable.jsonp + UIHelpers.setJsonpEnabled(false); } /** @@ -603,6 +605,7 @@ public void testGetJsonResponseBodyNoCallbackReturnsJson() { @Test public void testGetJsonResponseBodyValidCallbackIsWrapped() { + UIHelpers.setJsonpEnabled(true); Map data = new HashMap<>(); data.put("a", 1); String body = UIHelpers.getJsonResponseBody(data, "myCb", true); @@ -611,12 +614,14 @@ public void testGetJsonResponseBodyValidCallbackIsWrapped() { @Test public void testGetJsonResponseBodyValidDottedCallbackIsWrapped() { + UIHelpers.setJsonpEnabled(true); String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "foo.bar.$baz_0", false); assertEquals("foo.bar.$baz_0({\"x\":1});", body); } @Test public void testGetJsonResponseBodyInvalidCallbackFallsBackToJson() { + UIHelpers.setJsonpEnabled(true); Map data = new HashMap<>(); data.put("a", 1); String body = UIHelpers.getJsonResponseBody(data, "alert(document.cookie)//", true); @@ -625,12 +630,14 @@ public void testGetJsonResponseBodyInvalidCallbackFallsBackToJson() { @Test public void testGetJsonResponseBodyEmptyCallbackFallsBackToJson() { + UIHelpers.setJsonpEnabled(true); String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "", false); assertEquals("{\"x\":1}", body); } @Test public void testGetJsonResponseBodyTooLongCallbackFallsBackToJson() { + UIHelpers.setJsonpEnabled(true); StringBuilder sb = new StringBuilder("cb"); for (int i = 0; i < 200; i++) { sb.append('x'); @@ -641,10 +648,20 @@ public void testGetJsonResponseBodyTooLongCallbackFallsBackToJson() { @Test public void testGetJsonResponseBodyRejectsCallbacksStartingWithDigit() { + UIHelpers.setJsonpEnabled(true); String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "1cb", false); assertEquals("{\"x\":1}", body); } + @Test + public void testGetJsonResponseBodyCallbackIgnoredWhenJsonpDisabled() { + UIHelpers.setJsonpEnabled(false); + Map data = new HashMap<>(); + data.put("a", 1); + String body = UIHelpers.getJsonResponseBody(data, "myCb", true); + assertEquals("{\"a\":1}", body); + } + @Test public void testGetJsonResponseHeadersNoCallbackUsesJsonContentType() { Map headers = UIHelpers.getJsonResponseHeaders(null, null); @@ -654,6 +671,7 @@ public void testGetJsonResponseHeadersNoCallbackUsesJsonContentType() { @Test public void testGetJsonResponseHeadersValidCallbackUsesJavaScriptContentType() { + UIHelpers.setJsonpEnabled(true); Map headers = UIHelpers.getJsonResponseHeaders("myCb", null); assertEquals("application/javascript;charset=utf-8", headers.get("Content-Type")); assertEquals("nosniff", headers.get("X-Content-Type-Options")); @@ -661,8 +679,17 @@ public void testGetJsonResponseHeadersValidCallbackUsesJavaScriptContentType() { @Test public void testGetJsonResponseHeadersInvalidCallbackFallsBackToJsonContentType() { + UIHelpers.setJsonpEnabled(true); Map headers = UIHelpers.getJsonResponseHeaders("alert(1)//", null); assertEquals("application/json;charset=utf-8", headers.get("Content-Type")); assertEquals("nosniff", headers.get("X-Content-Type-Options")); } + + @Test + public void testGetJsonResponseHeadersCallbackIgnoredWhenJsonpDisabled() { + UIHelpers.setJsonpEnabled(false); + Map headers = UIHelpers.getJsonResponseHeaders("myCb", null); + assertEquals("application/json;charset=utf-8", headers.get("Content-Type")); + assertEquals("nosniff", headers.get("X-Content-Type-Options")); + } } \ No newline at end of file