disable javascript execution by default (apache#3818)

api/src/main/java/io/druid/data/input/impl/JavaScriptParseSpec.java

@@ -65,7 +65,7 @@ public void verify(List<String> usedCols)
   @Override
   public Parser<String, Object> makeParser()
   {
-    if (config.isDisabled()) {
+    if (!config.isEnabled()) {
       throw new ISE("JavaScript is disabled");
     }
 

api/src/main/java/io/druid/js/JavaScriptConfig.java

@@ -19,31 +19,31 @@
 
 package io.druid.js;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import java.util.Objects;
-
 public class JavaScriptConfig
 {
   public static final int DEFAULT_OPTIMIZATION_LEVEL = 9;
 
-  private static final JavaScriptConfig DEFAULT = new JavaScriptConfig(false);
+  private static final JavaScriptConfig ENABLED_INSTANCE = new JavaScriptConfig(true);
 
   @JsonProperty
-  private boolean disabled = false;
-
-  public JavaScriptConfig()
-  {
-  }
+  private boolean enabled = false;
 
-  public JavaScriptConfig(boolean disabled)
+  @JsonCreator
+  public JavaScriptConfig(
+      @JsonProperty("enabled") Boolean enabled
+  )
   {
-    this.disabled = disabled;
+    if (enabled != null) {
+      this.enabled = enabled.booleanValue();
+    }
   }
 
-  public boolean isDisabled()
+  public boolean isEnabled()
   {
-    return disabled;
+    return enabled;
   }
 
   @Override
@@ -55,26 +55,29 @@ public boolean equals(Object o)
     if (o == null || getClass() != o.getClass()) {
       return false;
     }
-    JavaScriptConfig config = (JavaScriptConfig) o;
-    return disabled == config.disabled;
+
+    JavaScriptConfig that = (JavaScriptConfig) o;
+
+    return enabled == that.enabled;
+
   }
 
   @Override
   public int hashCode()
   {
-    return Objects.hash(disabled);
+    return (enabled ? 1 : 0);
   }
 
   @Override
   public String toString()
   {
     return "JavaScriptConfig{" +
-           "disabled=" + disabled +
+           "enabled=" + enabled +
            '}';
   }
 
-  public static JavaScriptConfig getDefault()
+  public static JavaScriptConfig getEnabledInstance()
   {
-    return DEFAULT;
+    return ENABLED_INSTANCE;
   }
 }

api/src/test/java/io/druid/data/input/impl/JavaScriptParseSpecTest.java

@@ -50,14 +50,14 @@ public void testSerde() throws IOException
     jsonMapper.setInjectableValues(
         new InjectableValues.Std().addValue(
             JavaScriptConfig.class,
-            JavaScriptConfig.getDefault()
+            JavaScriptConfig.getEnabledInstance()
         )
     );
     JavaScriptParseSpec spec = new JavaScriptParseSpec(
         new TimestampSpec("abc", "iso", null),
         new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),
         "abc",
-        JavaScriptConfig.getDefault()
+        JavaScriptConfig.getEnabledInstance()
     );
     final JavaScriptParseSpec serde = jsonMapper.readValue(
         jsonMapper.writeValueAsString(spec),
@@ -73,7 +73,7 @@ public void testSerde() throws IOException
   @Test
   public void testMakeParser()
   {
-    final JavaScriptConfig config = JavaScriptConfig.getDefault();
+    final JavaScriptConfig config = JavaScriptConfig.getEnabledInstance();
     JavaScriptParseSpec spec = new JavaScriptParseSpec(
         new TimestampSpec("abc", "iso", null),
         new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),
@@ -89,7 +89,7 @@ public void testMakeParser()
   @Test
   public void testMakeParserNotAllowed()
   {
-    final JavaScriptConfig config = new JavaScriptConfig(true);
+    final JavaScriptConfig config = new JavaScriptConfig(false);
     JavaScriptParseSpec spec = new JavaScriptParseSpec(
         new TimestampSpec("abc", "iso", null),
         new DimensionsSpec(DimensionsSpec.getDefaultSchemas(Arrays.asList("abc")), null, null),

api/src/test/java/io/druid/js/JavaScriptConfigTest.java

@@ -25,13 +25,39 @@
 
 public class JavaScriptConfigTest
 {
+  private static ObjectMapper mapper = new ObjectMapper();
+
   @Test
   public void testSerde() throws Exception
   {
-    final JavaScriptConfig config = new JavaScriptConfig(true);
-    final ObjectMapper mapper = new ObjectMapper();
-    final JavaScriptConfig config2 = mapper.readValue(mapper.writeValueAsBytes(config), JavaScriptConfig.class);
-    Assert.assertTrue(config2.isDisabled());
-    Assert.assertEquals(config, config2);
+    String json = "{\"enabled\":true}";
+
+    JavaScriptConfig config = mapper.readValue(
+        mapper.writeValueAsString(
+            mapper.readValue(
+                json,
+                JavaScriptConfig.class
+            )
+        ), JavaScriptConfig.class
+    );
+
+    Assert.assertTrue(config.isEnabled());
+  }
+
+  @Test
+  public void testSerdeWithDefaults() throws Exception
+  {
+    String json = "{}";
+
+    JavaScriptConfig config = mapper.readValue(
+        mapper.writeValueAsString(
+            mapper.readValue(
+                json,
+                JavaScriptConfig.class
+            )
+        ), JavaScriptConfig.class
+    );
+
+    Assert.assertFalse(config.isEnabled());
   }
 }

benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java

@@ -126,7 +126,7 @@ public class FilterPartitionBenchmark
   private BenchmarkSchemaInfo schemaInfo;
 
   private static String JS_FN = "function(str) { return 'super-' + str; }";
-  private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getDefault());
+  private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getEnabledInstance());
 
   static {
     JSON_MAPPER = new DefaultObjectMapper();

benchmarks/src/main/java/io/druid/benchmark/FilteredAggregatorBenchmark.java

@@ -126,7 +126,7 @@ public class FilteredAggregatorBenchmark
   private TimeseriesQuery query;
 
   private static String JS_FN = "function(str) { return 'super-' + str; }";
-  private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getDefault());
+  private static ExtractionFn JS_EXTRACTION_FN = new JavaScriptExtractionFn(JS_FN, false, JavaScriptConfig.getEnabledInstance());
 
   static {
     JSON_MAPPER = new DefaultObjectMapper();
@@ -167,7 +167,7 @@ public void setup() throws IOException
     filter = new OrDimFilter(
         Arrays.asList(
             new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC),
-            new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()),
+            new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getEnabledInstance()),
             new RegexDimFilter("dimSequential", "X", null),
             new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null),
             new InDimFilter("dimSequential", Arrays.asList("X"), null)

benchmarks/src/main/java/io/druid/benchmark/indexing/IncrementalIndexReadBenchmark.java

@@ -171,7 +171,7 @@ public void readWithFilters(Blackhole blackhole) throws Exception
     DimFilter filter = new OrDimFilter(
         Arrays.asList(
             new BoundDimFilter("dimSequential", "-1", "-1", true, true, null, null, StringComparators.ALPHANUMERIC),
-            new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getDefault()),
+            new JavaScriptDimFilter("dimSequential", "function(x) { return false }", null, JavaScriptConfig.getEnabledInstance()),
             new RegexDimFilter("dimSequential", "X", null),
             new SearchQueryDimFilter("dimSequential", new ContainsSearchQuerySpec("X", false), null),
             new InDimFilter("dimSequential", Arrays.asList("X"), null)

docs/content/configuration/index.md

@@ -384,9 +384,8 @@ the following properties.
 
 |Property|Description|Default|
 |--------|-----------|-------|
-|`druid.javascript.disabled`|Set to "true" to disable JavaScript functionality. This affects the JavaScript parser, filter, extractionFn, aggregator, post-aggregator, router strategy, and worker selection strategy.|false|
+|`druid.javascript.enabled`|Set to "true" to enable JavaScript functionality. This affects the JavaScript parser, filter, extractionFn, aggregator, post-aggregator, router strategy, and worker selection strategy.|false|
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>

docs/content/configuration/indexing-service.md

@@ -257,8 +257,7 @@ Example: a function that sends batch_index_task to workers 10.0.0.1 and 10.0.0.2
 ```
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 #### Autoscaler

docs/content/development/javascript.md

@@ -22,6 +22,14 @@ without needing to write and deploy Druid extensions.
 
 Druid uses the Mozilla Rhino engine at optimization level 9 to compile and execute JavaScript.
 
+## Security
+
+Druid does not execute JavaScript functions in a sandbox, so they have full access to the machine. So Javascript
+functions allow users to execute arbutrary code inside druid process. So, by default, Javascript is disabled.
+However, on dev/staging environments or secured production environments you can enable those by setting
+the [configuration property](../configuration/index.html)
+`druid.javascript.enabled = true`.
+
 ## Global variables
 
 Avoid using global variables. Druid may share the global scope between multiple threads, which can lead to
@@ -36,13 +44,6 @@ You may need to pay special attention to garbage collection when making heavy us
 garbage collection of the compiled classes themselves. Be sure to use a garbage collector configuration that supports
 timely collection of unused classes (this is generally easier on JDK8 with the Metaspace than it is on JDK7).
 
-## Security
-
-Druid does not execute JavaScript functions in a sandbox, so they have full access to the machine. If you are running
-a cluster where users that can submit queries should not be allowed to execute arbitrary code, we recommend disabling
-JavaScript functionality by setting the [configuration property](../configuration/index.html)
-`druid.javascript.disabled = true`.
-
 ## JavaScript vs. Native Extensions
 
 Generally we recommend using JavaScript when security is not an issue, and when speed of development is more important

docs/content/development/router.md

@@ -117,8 +117,7 @@ Allows defining arbitrary routing rules using a JavaScript function. The functio
 ```
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 HTTP Endpoints

docs/content/ingestion/data-formats.md

@@ -147,8 +147,7 @@ Note with the JavaScript parser that data must be fully parsed and returned as a
 This means any flattening or parsing multi-dimensional values must be done here.
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 ### Multi-value dimensions

docs/content/querying/aggregations.md

@@ -162,8 +162,7 @@ JavaScript functions are expected to return floating-point values.
 ```
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 ## Approximate Aggregations

docs/content/querying/dimensionspecs.md

@@ -308,8 +308,7 @@ Example for the `__time` dimension:
 ```
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 ### Lookup extraction function

docs/content/querying/filters.md

@@ -89,8 +89,7 @@ The following matches any dimension values for the dimension `name` between `'ba
 The JavaScript filter supports the use of extraction functions, see [Filtering with Extraction Functions](#filtering-with-extraction-functions) for details.
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 ### Extraction filter
@@ -440,4 +439,4 @@ Filtering on a set of ISO 8601 intervals:
       "2014-11-15T00:00:00.000Z/2014-11-16T00:00:00.000Z"
     ]
 }
-```
+```

docs/content/querying/post-aggregations.md

@@ -99,8 +99,7 @@ Example JavaScript aggregator:
 ```
 
 <div class="note info">
-Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines
-about using Druid's JavaScript functionality.
+JavaScript-based functionality is disabled by default. Please refer to the Druid <a href="../development/javascript.html">JavaScript programming guide</a> for guidelines about using Druid's JavaScript functionality, including instructions on how to enable it.
 </div>
 
 ### HyperUnique Cardinality post-aggregator

indexing-service/src/main/java/io/druid/indexing/overlord/setup/JavaScriptWorkerSelectStrategy.java

@@ -57,7 +57,7 @@ public JavaScriptWorkerSelectStrategy(
   {
     Preconditions.checkNotNull(fn, "function must not be null");
 
-    if (config.isDisabled()) {
+    if (!config.isEnabled()) {
       throw new ISE("JavaScript is disabled");
     }
 

indexing-service/src/test/java/io/druid/indexing/overlord/setup/JavaScriptWorkerSelectStrategyTest.java

@@ -69,7 +69,7 @@ public class JavaScriptWorkerSelectStrategyTest
       + "}\n"
       + "return null;\n"
       + "}",
-      JavaScriptConfig.getDefault()
+      JavaScriptConfig.getEnabledInstance()
   );
 
   @Test
@@ -79,7 +79,7 @@ public void testSerde() throws Exception
     mapper.setInjectableValues(
         new InjectableValues.Std().addValue(
             JavaScriptConfig.class,
-            JavaScriptConfig.getDefault()
+            JavaScriptConfig.getEnabledInstance()
         )
     );
 
@@ -99,7 +99,7 @@ public void testDisabled() throws Exception
     mapper.setInjectableValues(
         new InjectableValues.Std().addValue(
             JavaScriptConfig.class,
-            new JavaScriptConfig(true)
+            new JavaScriptConfig(false)
         )
     );
 

processing/src/main/java/io/druid/query/aggregation/JavaScriptAggregatorFactory.java

@@ -85,10 +85,10 @@ public JavaScriptAggregatorFactory(
     this.fnCombine = fnCombine;
     this.config = config;
 
-    if (config.isDisabled()) {
-      this.compiledScript = null;
-    } else {
+    if (config.isEnabled()) {
       this.compiledScript = compileScript(fnAggregate, fnReset, fnCombine);
+    } else {
+      this.compiledScript = null;
     }
   }
 

processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java

@@ -97,7 +97,7 @@ public JavaScriptPostAggregator(
     Preconditions.checkNotNull(name, "Must have a valid, non-null post-aggregator name");
     Preconditions.checkNotNull(fieldNames, "Must have a valid, non-null fieldNames");
     Preconditions.checkNotNull(function, "Must have a valid, non-null function");
-    Preconditions.checkState(!config.isDisabled(), "JavaScript is disabled");
+    Preconditions.checkState(config.isEnabled(), "JavaScript is disabled.");
 
     this.name = name;
     this.fieldNames = fieldNames;

processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java

@@ -80,10 +80,10 @@ public JavaScriptExtractionFn(
     this.function = function;
     this.injective = injective;
 
-    if (config.isDisabled()) {
-      this.fn = null;
-    } else {
+    if (config.isEnabled()) {
       this.fn = compile(function);
+    } else {
+      this.fn = null;
     }
   }