You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2017/04/11 16:48:26 UTC

[1/2] tinkerpop git commit: TINKERPOP-1663 Validation for maximum number of parameters on a request

Repository: tinkerpop
Updated Branches:
  refs/heads/tp32 26c63028a -> e4372b2dc


TINKERPOP-1663 Validation for maximum number of parameters on a request

The default is set to 16 and there is a configuration option to allow it to be changed.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/a4c652ae
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/a4c652ae
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/a4c652ae

Branch: refs/heads/tp32
Commit: a4c652ae3c071c15c5a57524a797707f5ea318a4
Parents: f73d7ca
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Mon Apr 3 07:04:42 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Mon Apr 3 07:04:42 2017 -0400

----------------------------------------------------------------------
 .../src/reference/gremlin-applications.asciidoc | 21 +++++++
 .../upgrade/release-3.2.x-incubating.asciidoc   | 22 +++++++
 .../tinkerpop/gremlin/server/Settings.java      | 11 ++++
 .../server/op/AbstractEvalOpProcessor.java      | 23 +++++++-
 .../tinkerpop/gremlin/server/op/OpLoader.java   |  8 +++
 .../gremlin/server/op/session/Session.java      |  5 +-
 .../server/op/session/SessionOpProcessor.java   |  7 +++
 .../server/op/standard/StandardOpProcessor.java | 18 ++++++
 .../AbstractGremlinServerIntegrationTest.java   |  5 ++
 .../server/GremlinServerIntegrateTest.java      | 62 ++++++++++++++++++++
 10 files changed, 176 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/docs/src/reference/gremlin-applications.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc
index bfaa153..3a611a0 100644
--- a/docs/src/reference/gremlin-applications.asciidoc
+++ b/docs/src/reference/gremlin-applications.asciidoc
@@ -1136,6 +1136,7 @@ run Gremlin Server with Ganglia monitoring, download the `org.acplt:oncrpc` jar
 link:http://repo1.maven.org/maven2/org/acplt/oncrpc/1.0.7/[here] and copy it to the Gremlin Server `/lib` directory
 before starting the server.
 
+[[opprocessor-configurations]]
 OpProcessor Configurations
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -1161,9 +1162,23 @@ The `SessionOpProcessor` provides a way to interact with Gremlin Server over a <
 [width="100%",cols="3,10,^2",options="header"]
 |=========================================================
 |Name |Description |Default
+|maxParameters |Maximum number of parameters that can be passed on the request. |16
+|perGraphCloseTimeout |Time in milliseconds to wait for each configured graph to close any open transactions when the session is killed. |10000
 |sessionTimeout |Time in milliseconds before a session will time out. |28800000
 |=========================================================
 
+StandardOpProcessor
+++++++++++++++++++
+
+The `StandardOpProcessor` provides a way to interact with Gremlin Server without use of sessions and is the default
+method for processing script evaluation requests.
+
+[width="100%",cols="3,10,^2",options="header"]
+|=========================================================
+|Name |Description |Default
+|maxParameters |Maximum number of parameters that can be passed on the request. |16
+|=========================================================
+
 [[traversalopprocessor]]
 TraversalOpProcessor
 ++++++++++++++++++++
@@ -1636,6 +1651,12 @@ params.put("x",4);
 client.submit("[1,2,3,x]", params);
 ----
 
+The more parameters that are used in a script the more expensive the compilation step becomes. Gremlin Server has a
+`OpProcessor` setting called `maxParameters`, which is mentioned in the <<opprocess-configuration,OpProcessor Configuration>>
+section. It controls the maximum number of parameters that can be passed to the server for script evaluation purposes.
+Use of this setting can prevent accidental long run compilations, which individually are not terribly oppressive to
+the server, but taken as a group under high concurrency would be considered detrimental.
+
 Cache Management
 ^^^^^^^^^^^^^^^^
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/docs/src/upgrade/release-3.2.x-incubating.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
index fbe31bd..8079cdd 100644
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@ -32,6 +32,28 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.2.5/CHANGELOG.asc
 Upgrading for Users
 ~~~~~~~~~~~~~~~~~~~
 
+Default Maximum Parameters
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+It was learned that compilation for scripts with large numbers of parameters is more expensive than those with less
+parameters. It therefore becomes possible to make some mistakes with how Gremlin Server is used. A new setting on
+the `StandardOpProcessor` and `SessionOpProcessor` called `maxParameters` controls the number of parameters that can
+be passed in on a request. This setting is defaulted to sixteen.
+
+Users upgrading to this version may notice errors in their applications if they use more than sixteen parameters. To
+fix this problem simply reconfigure Gremlin Server with a configuration as follows:
+
+[source,yaml]
+----
+processors:
+  - { className: org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor, config: { maxParameters: 64 }}
+  - { className: org.apache.tinkerpop.gremlin.server.op.standard.StandardOpProcessor, config: { maxParameters: 64 }}
+----
+
+The above configuration allows sixty-four parameters to be passed on each request.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1663[TINKERPOP-1663]
+
 GremlinScriptEngine Metrics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java
index e2f2ad5..f974848 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java
@@ -29,6 +29,7 @@ import org.apache.tinkerpop.gremlin.server.auth.AllowAllAuthenticator;
 import org.apache.tinkerpop.gremlin.server.auth.Authenticator;
 import org.apache.tinkerpop.gremlin.server.channel.WebSocketChannelizer;
 import info.ganglia.gmetric4j.gmetric.GMetric;
+import org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor;
 import org.apache.tinkerpop.gremlin.server.util.LifeCycleHook;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 import org.yaml.snakeyaml.TypeDescription;
@@ -218,6 +219,16 @@ public class Settings {
      */
     public List<ProcessorSettings> processors = new ArrayList<>();
 
+    /**
+     * Find the {@link ProcessorSettings} related to the specified class. If there are multiple entries then only the
+     * first is returned.
+     */
+    public Optional<ProcessorSettings> optionalProcessor(final Class<? extends OpProcessor> clazz) {
+        return processors.stream()
+                .filter(p -> p.className.equals(clazz.getCanonicalName()))
+                .findFirst();
+    }
+
     public Optional<ServerMetrics> optionalMetrics() {
         return Optional.ofNullable(metrics);
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index 1ba6e36..edc484c 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@ -41,13 +41,11 @@ import org.apache.tinkerpop.gremlin.server.util.MetricManager;
 import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 import io.netty.channel.ChannelHandlerContext;
-import org.codehaus.groovy.control.ErrorCollector;
 import org.codehaus.groovy.control.MultipleCompilationErrorsException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.script.Bindings;
-import javax.script.ScriptException;
 import javax.script.SimpleBindings;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -73,6 +71,18 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
     public static final Timer evalOpTimer = MetricManager.INSTANCE.getTimer(name(GremlinServer.class, "op", "eval"));
 
     /**
+     * The maximum number of parameters that can be passed on a script evaluation request.
+     */
+    public static final String CONFIG_MAX_PARAMETERS = "maxParameters";
+
+    /**
+     * Default number of parameters allowed on a script evaluation request.
+     */
+    public static final int DEFAULT_MAX_PARAMETERS = 16;
+
+    protected int maxParameters = DEFAULT_MAX_PARAMETERS;
+
+    /**
      * Captures the "error" count as a reportable metric for Gremlin Server.
      *
      * @deprecated As of release 3.1.1-incubating, not replaced. Direct usage is discouraged with sub-classes as
@@ -178,7 +188,7 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
 
         if (message.optionalArgs(Tokens.ARGS_BINDINGS).isPresent()) {
             final Map bindings = (Map) message.getArgs().get(Tokens.ARGS_BINDINGS);
-            if (bindings.keySet().stream().anyMatch(k -> null == k || !(k instanceof String))) {
+            if (IteratorUtils.anyMatch(bindings.keySet().iterator(), k -> null == k || !(k instanceof String))) {
                 final String msg = String.format("The [%s] message is using one or more invalid binding keys - they must be of type String and cannot be null", Tokens.OPS_EVAL);
                 throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create());
             }
@@ -188,6 +198,13 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
                 final String msg = String.format("The [%s] message supplies one or more invalid parameters key of [%s] - these are reserved names.", Tokens.OPS_EVAL, badBindings);
                 throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create());
             }
+
+            // ignore control bindings that get passed in with the "#jsr223" prefix - those aren't used in compilation
+            if (IteratorUtils.count(IteratorUtils.filter(bindings.keySet().iterator(), k -> !k.toString().startsWith("#jsr223"))) > maxParameters) {
+                final String msg = String.format("The [%s] message contains %s bindings which is more than is allowed by the server %s configuration",
+                        Tokens.OPS_EVAL, bindings.size(), maxParameters);
+                throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create());
+            }
         }
 
         return Optional.empty();

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/OpLoader.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/OpLoader.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/OpLoader.java
index 3ef805f..5b69a64 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/OpLoader.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/OpLoader.java
@@ -78,4 +78,12 @@ public final class OpLoader {
     public static Map<String, OpProcessor> getProcessors() {
         return Collections.unmodifiableMap(processors);
     }
+
+    /**
+     * Reset the processors so that they can be re-initialized with different settings which is useful in testing
+     * scenarios.
+     */
+    public synchronized static void reset() {
+        initialized = false;
+    }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
index 6961339..f43b6e4 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
@@ -89,9 +89,8 @@ public class Session {
         this.scheduledExecutorService = context.getScheduledExecutorService();
         this.sessions = sessions;
 
-        final Settings.ProcessorSettings processorSettings = this.settings.processors.stream()
-                .filter(p -> p.className.equals(SessionOpProcessor.class.getCanonicalName()))
-                .findAny().orElse(SessionOpProcessor.DEFAULT_SETTINGS);
+        final Settings.ProcessorSettings processorSettings = this.settings.optionalProcessor(SessionOpProcessor.class).
+                orElse(SessionOpProcessor.DEFAULT_SETTINGS);
         this.configuredSessionTimeout = Long.parseLong(processorSettings.config.getOrDefault(
                 SessionOpProcessor.CONFIG_SESSION_TIMEOUT, SessionOpProcessor.DEFAULT_SESSION_TIMEOUT).toString());
         this.configuredPerGraphCloseTimeout = Long.parseLong(processorSettings.config.getOrDefault(

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
index fe3cc85..9485405 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
@@ -94,6 +94,7 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
         DEFAULT_SETTINGS.config = new HashMap<String, Object>() {{
             put(CONFIG_SESSION_TIMEOUT, DEFAULT_SESSION_TIMEOUT);
             put(CONFIG_PER_GRAPH_CLOSE_TIMEOUT, DEFAULT_PER_GRAPH_CLOSE_TIMEOUT);
+            put(CONFIG_MAX_PARAMETERS, DEFAULT_MAX_PARAMETERS);
         }};
     }
 
@@ -106,6 +107,12 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
         return OP_PROCESSOR_NAME;
     }
 
+    @Override
+    public void init(final Settings settings) {
+        this.maxParameters = (int) settings.optionalProcessor(SessionOpProcessor.class).orElse(DEFAULT_SETTINGS).config.
+                getOrDefault(CONFIG_MAX_PARAMETERS, DEFAULT_MAX_PARAMETERS);
+    }
+
     /**
      * Session based requests accept a "close" operator in addition to "eval".  A close will trigger the session to be
      * killed and any uncommitted transaction to be rolled-back.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/standard/StandardOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/standard/StandardOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/standard/StandardOpProcessor.java
index 893ae75..c0e0f76 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/standard/StandardOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/standard/StandardOpProcessor.java
@@ -25,8 +25,10 @@ import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
 import org.apache.tinkerpop.gremlin.server.Context;
 import org.apache.tinkerpop.gremlin.server.OpProcessor;
+import org.apache.tinkerpop.gremlin.server.Settings;
 import org.apache.tinkerpop.gremlin.server.op.AbstractEvalOpProcessor;
 import org.apache.tinkerpop.gremlin.server.op.OpProcessorException;
+import org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer;
 import org.slf4j.Logger;
@@ -34,6 +36,7 @@ import org.slf4j.LoggerFactory;
 
 import javax.script.Bindings;
 import javax.script.SimpleBindings;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
@@ -51,6 +54,15 @@ public class StandardOpProcessor extends AbstractEvalOpProcessor {
 
     protected final Function<Context, BindingSupplier> bindingMaker;
 
+    static final Settings.ProcessorSettings DEFAULT_SETTINGS = new Settings.ProcessorSettings();
+
+    static {
+        DEFAULT_SETTINGS.className = StandardOpProcessor.class.getCanonicalName();
+        DEFAULT_SETTINGS.config = new HashMap<String, Object>() {{
+            put(CONFIG_MAX_PARAMETERS, DEFAULT_MAX_PARAMETERS);
+        }};
+    }
+
     public StandardOpProcessor() {
         super(true);
         bindingMaker = getBindingMaker();
@@ -62,6 +74,12 @@ public class StandardOpProcessor extends AbstractEvalOpProcessor {
     }
 
     @Override
+    public void init(final Settings settings) {
+        this.maxParameters = (int) settings.optionalProcessor(StandardOpProcessor.class).orElse(DEFAULT_SETTINGS).config.
+                getOrDefault(CONFIG_MAX_PARAMETERS, DEFAULT_MAX_PARAMETERS);
+    }
+
+    @Override
     public ThrowingConsumer<Context> getEvalOp() {
         return this::evalOp;
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
index 12fadb6..b8bf51f 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
@@ -18,6 +18,7 @@
  */
 package org.apache.tinkerpop.gremlin.server;
 
+import org.apache.tinkerpop.gremlin.server.op.OpLoader;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -82,6 +83,10 @@ public abstract class AbstractGremlinServerIntegrationTest {
 
     public void stopServer() throws Exception {
         server.stop().join();
+
+        // reset the OpLoader processors so that they can get reconfigured on startup - Settings may have changed
+        // between tests
+        OpLoader.reset();
     }
 
     public static boolean deleteDirectory(final File directory) {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a4c652ae/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
index af25be5..8f90462 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
@@ -52,6 +52,8 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet;
+import org.apache.tinkerpop.gremlin.server.op.AbstractEvalOpProcessor;
+import org.apache.tinkerpop.gremlin.server.op.standard.StandardOpProcessor;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.server.channel.NioChannelizer;
@@ -142,6 +144,13 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
         switch (nameOfTest) {
             case "shouldProvideBetterExceptionForMethodCodeTooLarge":
                 settings.maxContentLength = 4096000;
+                final Settings.ProcessorSettings processorSettingsBig = new Settings.ProcessorSettings();
+                processorSettingsBig.className = StandardOpProcessor.class.getName();
+                processorSettingsBig.config = new HashMap<String,Object>() {{
+                    put(AbstractEvalOpProcessor.CONFIG_MAX_PARAMETERS, Integer.MAX_VALUE);
+                }};
+                settings.processors.clear();
+                settings.processors.add(processorSettingsBig);
                 break;
             case "shouldRespectHighWaterMarkSettingAndSucceed":
                 settings.writeBufferHighWaterMark = 64;
@@ -219,6 +228,15 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
             case "shouldUseBaseScript":
                 settings.scriptEngines.get("gremlin-groovy").config = getScriptEngineConfForBaseScript();
                 break;
+            case "shouldReturnInvalidRequestArgsWhenBindingCountExceedsAllowable":
+                final Settings.ProcessorSettings processorSettingsSmall = new Settings.ProcessorSettings();
+                processorSettingsSmall.className = StandardOpProcessor.class.getName();
+                processorSettingsSmall.config = new HashMap<String,Object>() {{
+                    put(AbstractEvalOpProcessor.CONFIG_MAX_PARAMETERS, 1);
+                }};
+                settings.processors.clear();
+                settings.processors.add(processorSettingsSmall);
+                break;
         }
 
         return settings;
@@ -587,6 +605,50 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
     }
 
     @Test
+    public void shouldReturnInvalidRequestArgsWhenBindingCountExceedsAllowable() throws Exception {
+        try (SimpleClient client = TestClientFactory.createWebSocketClient()) {
+            final Map<Object, Object> bindings = new HashMap<>();
+            bindings.put("x", 123);
+            bindings.put("y", 123);
+            final RequestMessage request = RequestMessage.build(Tokens.OPS_EVAL)
+                    .addArg(Tokens.ARGS_GREMLIN, "x+y")
+                    .addArg(Tokens.ARGS_BINDINGS, bindings).create();
+            final CountDownLatch latch = new CountDownLatch(1);
+            final AtomicBoolean pass = new AtomicBoolean(false);
+            client.submit(request, result -> {
+                if (result.getStatus().getCode() != ResponseStatusCode.PARTIAL_CONTENT) {
+                    pass.set(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS == result.getStatus().getCode());
+                    latch.countDown();
+                }
+            });
+
+            if (!latch.await(3000, TimeUnit.MILLISECONDS))
+                fail("Request should have returned error, but instead timed out");
+            assertThat(pass.get(), is(true));
+        }
+
+        try (SimpleClient client = TestClientFactory.createWebSocketClient()) {
+            final Map<Object, Object> bindings = new HashMap<>();
+            bindings.put("x", 123);
+            final RequestMessage request = RequestMessage.build(Tokens.OPS_EVAL)
+                    .addArg(Tokens.ARGS_GREMLIN, "x+123")
+                    .addArg(Tokens.ARGS_BINDINGS, bindings).create();
+            final CountDownLatch latch = new CountDownLatch(1);
+            final AtomicBoolean pass = new AtomicBoolean(false);
+            client.submit(request, result -> {
+                if (result.getStatus().getCode() != ResponseStatusCode.PARTIAL_CONTENT) {
+                    pass.set(ResponseStatusCode.SUCCESS == result.getStatus().getCode() && (((int) ((List) result.getResult().getData()).get(0) == 246)));
+                    latch.countDown();
+                }
+            });
+
+            if (!latch.await(3000, TimeUnit.MILLISECONDS))
+                fail("Request should have returned error, but instead timed out");
+            assertThat(pass.get(), is(true));
+        }
+    }
+
+    @Test
     public void shouldReturnInvalidRequestArgsWhenInvalidNullBindingKeyIsUsed() throws Exception {
         try (SimpleClient client = TestClientFactory.createWebSocketClient()) {
             final Map<String, Object> bindings = new HashMap<>();


[2/2] tinkerpop git commit: Merge branch 'TINKERPOP-1663' into tp32

Posted by sp...@apache.org.
Merge branch 'TINKERPOP-1663' into tp32

Conflicts:
	docs/src/upgrade/release-3.2.x-incubating.asciidoc


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/e4372b2d
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/e4372b2d
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/e4372b2d

Branch: refs/heads/tp32
Commit: e4372b2dce84cbbb57583af7e807185e8ff4c43d
Parents: 26c6302 a4c652a
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Tue Apr 11 08:22:58 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Apr 11 08:22:58 2017 -0400

----------------------------------------------------------------------
 .../src/reference/gremlin-applications.asciidoc | 21 +++++++
 .../upgrade/release-3.2.x-incubating.asciidoc   | 22 +++++++
 .../tinkerpop/gremlin/server/Settings.java      | 11 ++++
 .../server/op/AbstractEvalOpProcessor.java      | 23 +++++++-
 .../tinkerpop/gremlin/server/op/OpLoader.java   |  8 +++
 .../gremlin/server/op/session/Session.java      |  5 +-
 .../server/op/session/SessionOpProcessor.java   |  7 +++
 .../server/op/standard/StandardOpProcessor.java | 18 ++++++
 .../AbstractGremlinServerIntegrationTest.java   |  5 ++
 .../server/GremlinServerIntegrateTest.java      | 62 ++++++++++++++++++++
 10 files changed, 176 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e4372b2d/docs/src/reference/gremlin-applications.asciidoc
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e4372b2d/docs/src/upgrade/release-3.2.x-incubating.asciidoc
----------------------------------------------------------------------
diff --cc docs/src/upgrade/release-3.2.x-incubating.asciidoc
index f0bbba2,8079cdd..c85cc3c
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@@ -32,16 -32,28 +32,38 @@@ Please see the link:https://github.com/
  Upgrading for Users
  ~~~~~~~~~~~~~~~~~~~
  
 +Authentication Configuration
 +^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 +
 +The server settings previously used `authentication.className` to set an authenticator for the the two provided
 +authentication handler and channelizer classes to use. This has been deprecated in favor of `authentication.authenticator`.
 +A class that extends `AbstractAuthenticationHandler` may also now be provided as `authentication.authenticationHandler`
 +to be used in either of the provided channelizer classes to handle the provided authenticator
 +
 +See: https://issues.apache.org/jira/browse/TINKERPOP-1657[TINKERPOP-1657]
 +
+ Default Maximum Parameters
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^
+ 
+ It was learned that compilation for scripts with large numbers of parameters is more expensive than those with less
+ parameters. It therefore becomes possible to make some mistakes with how Gremlin Server is used. A new setting on
+ the `StandardOpProcessor` and `SessionOpProcessor` called `maxParameters` controls the number of parameters that can
+ be passed in on a request. This setting is defaulted to sixteen.
+ 
+ Users upgrading to this version may notice errors in their applications if they use more than sixteen parameters. To
+ fix this problem simply reconfigure Gremlin Server with a configuration as follows:
+ 
+ [source,yaml]
+ ----
+ processors:
+   - { className: org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor, config: { maxParameters: 64 }}
+   - { className: org.apache.tinkerpop.gremlin.server.op.standard.StandardOpProcessor, config: { maxParameters: 64 }}
+ ----
+ 
+ The above configuration allows sixty-four parameters to be passed on each request.
+ 
+ See: link:https://issues.apache.org/jira/browse/TINKERPOP-1663[TINKERPOP-1663]
+ 
  GremlinScriptEngine Metrics
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e4372b2d/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java
----------------------------------------------------------------------