[tinkerpop] 01/03: Return a 597 error code for lambda compilation problems.

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 79ce81933843280589813c2e482f7df4c4662d15
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Fri Sep 20 08:32:08 2019 -0400

    Return a 597 error code for lambda compilation problems.
    
    This is a more fine grained and accurate error now. Lambda compilation issues aren't really deserialization problems. CTR
---
 CHANGELOG.asciidoc                                 |  1 +
 .../server/op/traversal/TraversalOpProcessor.java  |  7 +++++
 .../gremlin/server/GremlinServerIntegrateTest.java | 32 ++++++++++++++++++++--
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ccc4899..bf7ef39 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -35,6 +35,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Fixed potential for `NullPointerException` with empty identifiers in `GraphStep`.
 * Postponed the timing of transport creation to `connection.write` in Gremlin Python.
 * Made `EventStrategy` compatible with multi-valued properties.
+* Changed `TraversalOpProcessor` to throw a `SERVER_ERROR_SCRIPT_EVALUATION` (597) if lambdas don't compile.
 
 [[release-3-3-8]]
 === TinkerPop 3.3.8 (Release Date: August 5, 2019)
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
index 36c6856..7b7dbdb 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
@@ -55,6 +55,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.script.Bindings;
+import javax.script.ScriptException;
 import javax.script.SimpleBindings;
 import java.lang.reflect.UndeclaredThrowableException;
 import java.util.ArrayList;
@@ -392,6 +393,12 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
                 traversal = JavaTranslator.of(g).translate(bytecode);
             else
                 traversal = context.getGremlinExecutor().eval(bytecode, EMPTY_BINDINGS, lambdaLanguage.get(), traversalSourceName);
+        } catch (ScriptException ex) {
+            logger.error("Traversal contains a lambda that cannot be compiled", ex);
+            throw new OpProcessorException("Traversal contains a lambda that cannot be compiled",
+                    ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION)
+                            .statusMessage(ex.getMessage())
+                            .statusAttributeException(ex).create());
         } catch (Exception ex) {
             logger.error("Could not deserialize the Traversal instance", ex);
             throw new OpProcessorException("Could not deserialize the Traversal instance",
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 82e4f67..a8e7b3e 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
@@ -362,6 +362,34 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
     }
 
     @Test
+    public void shouldScriptEvaluationErrorForRemoteTraversal() throws Exception {
+        final GraphTraversalSource g = traversal().withRemote(conf);
+
+        try {
+            // tests bad lambda
+            g.inject(1).sideEffect(Lambda.consumer("(")).iterate();
+            fail("This traversal should not have executed since lambda can't be compiled");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION, ((ResponseException) t).getResponseStatusCode());
+        }
+
+        // make a graph with a cycle in it to force a long run traversal
+        graphGetter.get().traversal().addV("person").as("p").addE("self").to("p").iterate();
+
+        try {
+            // tests an "unending" traversal
+            g.V().repeat(__.out()).until(__.outE().count().is(0)).iterate();
+            fail("This traversal should have timed out");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, ((ResponseException) t).getResponseStatusCode());
+        }
+    }
+
+    @Test
     public void shouldCloseChannelIfClientDoesntRespond() throws Exception {
         final SimpleClient client = TestClientFactory.createWebSocketClient();
         client.submit("1+1");
@@ -649,7 +677,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
             cluster.close();
         }
     }
-    
+
     @Test
     public void shouldEnableSslAndClientCertificateAuthWithPkcs12() {
         final Cluster cluster = TestClientFactory.build().enableSsl(true).keyStore(P12_CLIENT_KEY).keyStorePassword(KEY_PASS)
@@ -709,7 +737,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
             cluster.close();
         }
     }
-    
+
     @Test
     public void shouldEnableSslAndFailIfProtocolsDontMatch() {
         final Cluster cluster = TestClientFactory.build().enableSsl(true).keyStore(JKS_SERVER_KEY).keyStorePassword(KEY_PASS)