Add ui.enable.jsonp to control JSONP callback wrapping in UI and Logviewer API responses

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"

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

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.
      */

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;
@@ -202,7 +204,7 @@
         for (int i = 0; i < args.length; i++) {
             argsEncoded[i] = Utils.urlEncodeUtf8(String.valueOf(args[i]));
         }
         return String.format(fmt, argsEncoded);
     }
 
     /**
@@ -314,8 +316,8 @@
      * @return corsFilterHandle
      */
     public static FilterHolder corsFilterHandle() {
         FilterHolder filterHolder = new FilterHolder(new CrossOriginFilter());
         filterHolder.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, "*");
         filterHolder.setInitParameter(CrossOriginFilter.ALLOWED_METHODS_PARAM, "GET, POST, PUT");
         filterHolder.setInitParameter(
                 CrossOriginFilter.ALLOWED_HEADERS_PARAM,
@@ -447,10 +449,27 @@
     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;

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<String, Object> 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<String, Object> 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<String, Object> 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,15 +671,25 @@ 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"));
     }
 
     @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"));
+    }
 }