This is an automated email from the ASF dual-hosted git repository.
valentyn pushed a commit to branch master-http
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master-http by this push:
new 3b09b8b3ea driver error handling (#2594)
3b09b8b3ea is described below
commit 3b09b8b3eafd1aa570a8db5f8a11a9ada56d45f2
Author: Valentyn Kahamlyk <vkagam...@users.noreply.github.com>
AuthorDate: Tue May 7 13:47:53 2024 -0700
driver error handling (#2594)
---
.../handler/HttpGremlinResponseStreamDecoder.java | 38 +++++++++++++++-------
.../handler/HttpBasicAuthenticationHandler.java | 37 +++++++++------------
.../gremlin/server/handler/HttpHandlerUtil.java | 9 +++--
.../server/GremlinServerAuthIntegrateTest.java | 4 +--
.../server/GremlinServerAuthzIntegrateTest.java | 14 ++++----
5 files changed, 57 insertions(+), 45 deletions(-)
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinResponseStreamDecoder.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinResponseStreamDecoder.java
index 0b4cce749b..4ebfb5c172 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinResponseStreamDecoder.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinResponseStreamDecoder.java
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.driver.handler;
+import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.MessageToMessageDecoder;
import io.netty.handler.codec.http.DefaultHttpObject;
@@ -29,9 +30,12 @@ import io.netty.handler.codec.http.LastHttpContent;
import io.netty.util.Attribute;
import io.netty.util.AttributeKey;
import io.netty.util.AttributeMap;
+import io.netty.util.CharsetUtil;
import org.apache.tinkerpop.gremlin.util.MessageSerializerV4;
import org.apache.tinkerpop.gremlin.util.message.ResponseMessageV4;
import org.apache.tinkerpop.gremlin.util.ser.SerializationException;
+import org.apache.tinkerpop.shaded.jackson.databind.JsonNode;
+import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
import java.util.List;
import java.util.Objects;
@@ -40,8 +44,10 @@ public class HttpGremlinResponseStreamDecoder extends
MessageToMessageDecoder<De
// todo: move out
public static final AttributeKey<Boolean> IS_FIRST_CHUNK =
AttributeKey.valueOf("isFirstChunk");
+ public static final AttributeKey<HttpResponseStatus> RESPONSE_STATUS =
AttributeKey.valueOf("responseStatus");
private final MessageSerializerV4<?> serializer;
+ private final ObjectMapper mapper = new ObjectMapper();
public HttpGremlinResponseStreamDecoder(MessageSerializerV4<?> serializer)
{
this.serializer = serializer;
@@ -50,18 +56,15 @@ public class HttpGremlinResponseStreamDecoder extends
MessageToMessageDecoder<De
@Override
protected void decode(ChannelHandlerContext ctx, DefaultHttpObject msg,
List<Object> out) throws Exception {
final Attribute<Boolean> isFirstChunk = ((AttributeMap)
ctx).attr(IS_FIRST_CHUNK);
+ final Attribute<HttpResponseStatus> responseStatus = ((AttributeMap)
ctx).attr(RESPONSE_STATUS);
System.out.println("HttpGremlinResponseStreamDecoder start");
if (msg instanceof HttpResponse) {
- if (((HttpResponse) msg).status() != HttpResponseStatus.OK &&
((HttpResponse) msg).status() != HttpResponseStatus.NO_CONTENT) {
- System.out.println("Error: " + ((HttpResponse) msg).status());
-
- final ResponseMessageV4 response = ResponseMessageV4.build()
- .code(((HttpResponse) msg).status())
- .create();
+ responseStatus.set(((HttpResponse) msg).status());
- out.add(response);
+ if (isError(((HttpResponse) msg).status())) {
+ System.out.println("Error: " + ((HttpResponse) msg).status());
return;
}
@@ -70,6 +73,19 @@ public class HttpGremlinResponseStreamDecoder extends
MessageToMessageDecoder<De
if (msg instanceof HttpContent) {
try {
+ // with error status we can get json in response
+ // no more chunks expected
+ if (isError(responseStatus.get())) {
+ final JsonNode node = mapper.readTree(((HttpContent)
msg).content().toString(CharsetUtil.UTF_8));
+ final String message = node.get("message").asText();
+ final ResponseMessageV4 response =
ResponseMessageV4.build()
+ .code(responseStatus.get()).statusMessage(message)
+ .create();
+
+ out.add(response);
+ return;
+ }
+
if (isFirstChunk.get()) {
System.out.println("first chunk");
} else {
@@ -79,10 +95,6 @@ public class HttpGremlinResponseStreamDecoder extends
MessageToMessageDecoder<De
System.out.println("payload size in bytes: " + ((HttpContent)
msg).content().readableBytes());
System.out.println(((HttpContent) msg).content());
- if (((HttpContent) msg).content().readableBytes() < 300) {
- System.out.println("---");
- }
-
final ResponseMessageV4 chunk =
serializer.readChunk(((HttpContent) msg).content(), isFirstChunk.get());
if (chunk.getResult().getData() != null) {
@@ -111,4 +123,8 @@ public class HttpGremlinResponseStreamDecoder extends
MessageToMessageDecoder<De
System.out.println("----------------------------");
}
+
+ private static boolean isError(final HttpResponseStatus status) {
+ return status != HttpResponseStatus.OK && status !=
HttpResponseStatus.NO_CONTENT;
+ }
}
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
index 55d4ee17ff..db6d1b77f2 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
@@ -18,9 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.server.handler;
-import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.http.DefaultFullHttpResponse;
import io.netty.handler.codec.http.FullHttpMessage;
import io.netty.util.ReferenceCountUtil;
import org.apache.tinkerpop.gremlin.server.GremlinServer;
@@ -32,16 +30,16 @@ import org.apache.tinkerpop.gremlin.server.authz.Authorizer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.HashMap;
import java.util.Map;
import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED;
-import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import static
org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens.PROPERTY_ADDRESS;
import static
org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens.PROPERTY_PASSWORD;
import static
org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens.PROPERTY_USERNAME;
+import static
org.apache.tinkerpop.gremlin.server.handler.HttpHandlerUtil.sendError;
/**
* Implements basic HTTP authentication for use with the {@link
HttpGremlinEndpointHandler} and HTTP based API calls.
@@ -49,6 +47,7 @@ import static
org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.Credenti
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
public class HttpBasicAuthenticationHandler extends
AbstractAuthenticationHandler {
+ private static final String INCORRECT_CREDENTIALS_MESSAGE = "Missing or
incorrect credentials";
private static final Logger auditLogger =
LoggerFactory.getLogger(GremlinServer.AUDIT_LOGGER_NAME);
private final Settings settings;
@@ -64,7 +63,8 @@ public class HttpBasicAuthenticationHandler extends
AbstractAuthenticationHandle
if (msg instanceof FullHttpMessage) {
final FullHttpMessage request = (FullHttpMessage) msg;
if (!request.headers().contains("Authorization")) {
- sendError(ctx, msg);
+ sendError(ctx, UNAUTHORIZED, INCORRECT_CREDENTIALS_MESSAGE);
+ ReferenceCountUtil.release(msg);
return;
}
@@ -72,24 +72,24 @@ public class HttpBasicAuthenticationHandler extends
AbstractAuthenticationHandle
final String basic = "Basic ";
final String authorizationHeader =
request.headers().get("Authorization");
if (!authorizationHeader.startsWith(basic)) {
- sendError(ctx, msg);
+ sendError(ctx, UNAUTHORIZED, INCORRECT_CREDENTIALS_MESSAGE);
+ ReferenceCountUtil.release(msg);
return;
}
- byte[] decodedUserPass = null;
+ byte[] decodedUserPass;
try {
final String encodedUserPass =
authorizationHeader.substring(basic.length());
decodedUserPass = decoder.decode(encodedUserPass);
- } catch (IndexOutOfBoundsException iae) {
- sendError(ctx, msg);
- return;
- } catch (IllegalArgumentException iae) {
- sendError(ctx, msg);
+ } catch (IndexOutOfBoundsException | IllegalArgumentException ex) {
+ sendError(ctx, UNAUTHORIZED, ex.getMessage());
+ ReferenceCountUtil.release(msg);
return;
}
- final String authorization = new String(decodedUserPass,
Charset.forName("UTF-8"));
+ final String authorization = new String(decodedUserPass,
StandardCharsets.UTF_8);
final String[] split = authorization.split(":");
if (split.length != 2) {
- sendError(ctx, msg);
+ sendError(ctx, UNAUTHORIZED, INCORRECT_CREDENTIALS_MESSAGE);
+ ReferenceCountUtil.release(msg);
return;
}
@@ -112,14 +112,9 @@ public class HttpBasicAuthenticationHandler extends
AbstractAuthenticationHandle
credentials.get(PROPERTY_USERNAME), address,
authClassParts[authClassParts.length - 1]);
}
} catch (AuthenticationException ae) {
- sendError(ctx, msg);
+ sendError(ctx, UNAUTHORIZED, ae.getMessage());
+ ReferenceCountUtil.release(msg);
}
}
}
-
- private void sendError(final ChannelHandlerContext ctx, final Object msg) {
- // Close the connection as soon as the error message is sent.
- ctx.writeAndFlush(new DefaultFullHttpResponse(HTTP_1_1,
UNAUTHORIZED)).addListener(ChannelFutureListener.CLOSE);
- ReferenceCountUtil.release(msg);
- }
}
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java
index 70182e566d..ce59c872f6 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java
@@ -21,6 +21,7 @@ package org.apache.tinkerpop.gremlin.server.handler;
import com.codahale.metrics.Meter;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http.DefaultFullHttpResponse;
import io.netty.handler.codec.http.DefaultHttpContent;
@@ -44,6 +45,7 @@ import org.slf4j.LoggerFactory;
import static com.codahale.metrics.MetricRegistry.name;
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
+import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
/**
@@ -64,9 +66,9 @@ public class HttpHandlerUtil {
* Helper method to send errors back as JSON. Only to be used when the
RequestMessage couldn't be parsed, because
* a proper serialized ResponseMessage should be sent in that case.
*
- * @param ctx The netty channel context.
- * @param status The HTTP error status code.
- * @param message The error message to contain the body.
+ * @param ctx The netty channel context.
+ * @param status The HTTP error status code.
+ * @param message The error message to contain the body.
*/
public static void sendError(final ChannelHandlerContext ctx, final
HttpResponseStatus status, final String message) {
logger.warn(String.format("Invalid request - responding with %s and
%s", status, message));
@@ -79,6 +81,7 @@ public class HttpHandlerUtil {
HTTP_1_1, status, Unpooled.copiedBuffer(node.toString(),
CharsetUtil.UTF_8));
response.headers().set(CONTENT_TYPE, "application/json");
HttpUtil.setContentLength(response,
response.content().readableBytes());
+
ctx.writeAndFlush(response);
}
diff --git
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
index 2bcbe6dbe6..c33e42ba29 100644
---
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
+++
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
@@ -137,7 +137,7 @@ public class GremlinServerAuthIntegrateTest extends
AbstractGremlinServerIntegra
final Throwable root = ExceptionHelper.getRootCause(ex);
assertEquals(ResponseException.class, root.getClass());
// server do not send error message now
- // assertEquals("Username and/or password are incorrect",
root.getMessage());
+ assertEquals("Username and/or password are incorrect",
root.getMessage());
} finally {
cluster.close();
}
@@ -153,7 +153,7 @@ public class GremlinServerAuthIntegrateTest extends
AbstractGremlinServerIntegra
} catch (Exception ex) {
final Throwable root = ExceptionHelper.getRootCause(ex);
assertEquals(ResponseException.class, root.getClass());
- // assertEquals("Username and/or password are incorrect",
root.getMessage());
+ assertEquals("Username and/or password are incorrect",
root.getMessage());
} finally {
cluster.close();
}
diff --git
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java
index 76a1b392c5..8cdb5a32ff 100644
---
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java
+++
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthzIntegrateTest.java
@@ -34,7 +34,6 @@ import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSo
import org.apache.tinkerpop.gremlin.server.auth.AllowAllAuthenticator;
import org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator;
import org.apache.tinkerpop.gremlin.server.authz.AllowListAuthorizer;
-import org.apache.tinkerpop.gremlin.server.channel.HttpChannelizer;
import
org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthenticationHandler;
import org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTokens;
import org.apache.tinkerpop.gremlin.util.function.Lambda;
@@ -51,8 +50,6 @@ import java.util.HashMap;
import java.util.Objects;
import static org.apache.tinkerpop.gremlin.driver.Auth.basic;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -131,12 +128,12 @@ public class GremlinServerAuthzIntegrateTest extends
AbstractGremlinServerIntegr
return settings;
}
+ @Ignore("todo: looks at server side AllowListAuthorizer")
@Test
public void shouldAuthorizeBytecodeRequest() {
final Cluster cluster =
TestClientFactory.build().auth(basic("stephen", "password")).create();
final GraphTraversalSource g =
AnonymousTraversalSource.traversal().with(
DriverRemoteConnection.using(cluster, "gmodern"));
-
try {
assertEquals(6, (long) g.V().count().next());
} finally {
@@ -147,7 +144,7 @@ public class GremlinServerAuthzIntegrateTest extends
AbstractGremlinServerIntegr
@Test
public void shouldAuthorizeBytecodeRequestWithLambda() {
final Cluster cluster = TestClientFactory.build().auth(basic("marko",
"rainbow-dash")).create();
- final GraphTraversalSource g =
AnonymousTraversalSource.traversal().withRemote(
+ final GraphTraversalSource g =
AnonymousTraversalSource.traversal().with(
DriverRemoteConnection.using(cluster, "gclassic"));
try {
@@ -161,7 +158,7 @@ public class GremlinServerAuthzIntegrateTest extends
AbstractGremlinServerIntegr
@Test
public void shouldFailBytecodeRequestWithLambda() throws Exception{
final Cluster cluster =
TestClientFactory.build().auth(basic("stephen", "password")).create();
- final GraphTraversalSource g =
AnonymousTraversalSource.traversal().withRemote(
+ final GraphTraversalSource g =
AnonymousTraversalSource.traversal().with(
DriverRemoteConnection.using(cluster, "gmodern"));
try {
@@ -184,10 +181,11 @@ public class GremlinServerAuthzIntegrateTest extends
AbstractGremlinServerIntegr
}
}
+ @Ignore("todo: looks at server side AllowListAuthorizer")
@Test
public void shouldKeepAuthorizingBytecodeRequests() {
final Cluster cluster =
TestClientFactory.build().auth(basic("stephen", "password")).create();
- final GraphTraversalSource g =
AnonymousTraversalSource.traversal().withRemote(
+ final GraphTraversalSource g =
AnonymousTraversalSource.traversal().with(
DriverRemoteConnection.using(cluster, "gmodern"));
try {
@@ -198,7 +196,7 @@ public class GremlinServerAuthzIntegrateTest extends
AbstractGremlinServerIntegr
} catch (Exception ex) {
final ResponseException re = (ResponseException) ex.getCause();
assertEquals(HttpResponseStatus.UNAUTHORIZED,
re.getResponseStatusCode());
- // assertEquals("Failed to authorize: User not authorized for
bytecode requests on [gmodern] using lambdas.", re.getMessage());
+ assertEquals("Failed to authorize: User not authorized for
bytecode requests on [gmodern] using lambdas.", re.getMessage());
}
assertEquals(6, (long) g.V().count().next());
} finally {
