From 0eb00ca20ea42da6f47f5eef533d704594e661d8 Mon Sep 17 00:00:00 2001 From: andrii0lomakin Date: Thu, 6 Nov 2025 11:49:02 +0100 Subject: [PATCH] Fix JavaTranslator's `has` handling for null arguments - Updated `has()` special case logic to address potential issues when first or third arguments are `null`. Example : `g.E().has("knows","weight",null)` this query can lead to NPE on server execution as JavaTranslator can choose GraphTravers#has(String, Sring, P) method instead of GraphTravers#has(String, Sring, Object) --- .../gremlin/jsr223/JavaTranslator.java | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java index fa56a2cd4fd..614960fe16f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java @@ -22,10 +22,7 @@ import org.apache.commons.configuration2.BaseConfiguration; import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.MapConfiguration; -import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; -import org.apache.tinkerpop.gremlin.process.traversal.Translator; -import org.apache.tinkerpop.gremlin.process.traversal.Traversal; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.*; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.lambda.CardinalityValueTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet; @@ -60,7 +57,7 @@ public final class JavaTranslator anonymousTraversal; private static final Map, Map>> GLOBAL_METHOD_CACHE = new ConcurrentHashMap<>(); - private final Map, Map> localMethodCache = new ConcurrentHashMap<>(); + private final Map, Map> localMethodCache = new ConcurrentHashMap<>(); private final Method anonymousTraversalStart; private JavaTranslator(final S traversalSource) { @@ -109,7 +106,7 @@ public String toString() { return StringFactory.translatorString(this); } - //// + /// / private Object translateObject(final Object object) { if (object instanceof Bytecode.Binding) @@ -287,12 +284,12 @@ private Object invokeMethod(final Object delegate, final Class returnType, fi // do something far more drastic that doesn't involve reflection. if (i < argumentsCopy.length && (null == argumentsCopy[i] || (argumentsCopy[i] != null && ( - parameters[i].getType().isAssignableFrom(argumentsCopy[i].getClass()) || - (parameters[i].getType().isPrimitive() && - (Number.class.isAssignableFrom(argumentsCopy[i].getClass()) || - argumentsCopy[i].getClass().equals(Boolean.class) || - argumentsCopy[i].getClass().equals(Byte.class) || - argumentsCopy[i].getClass().equals(Character.class))))))) { + parameters[i].getType().isAssignableFrom(argumentsCopy[i].getClass()) || + (parameters[i].getType().isPrimitive() && + (Number.class.isAssignableFrom(argumentsCopy[i].getClass()) || + argumentsCopy[i].getClass().equals(Boolean.class) || + argumentsCopy[i].getClass().equals(Byte.class) || + argumentsCopy[i].getClass().equals(Character.class))))))) { newArguments[i] = argumentsCopy[i]; } else { found = false; @@ -301,20 +298,30 @@ private Object invokeMethod(final Object delegate, final Class returnType, fi } } - // special case has() where the first arg is null - sometimes this can end up with the T being - // null and in 3.5.x that generates an exception which raises badly in the translator. it is - // safer to force this to the String form by letting this "found" version pass. In java this - // form of GraphTraversal can't be produced because of validations for has(T, ...) but in - // other language it might be allowed which means that has((T) null, ...) from something like + // special cases of has() where the first arg is null or third arg is null - sometimes this can end up with the T being + // null or in the second case calling has that accepts predicate instead of string as parameter in the last argument. + // That generates an exception which raises badly in the translator. it is + // safer to force this to the String form by letting this "found" version pass. In Java + // form of GraphTraversal generated in the first case can't be produced because of validations + // for has(T, ...) but in other language it might be allowed which means that has((T) null, ...) from something like // python will end up as has((String) null, ...) which filters rather than generates an // exception. calling the T version even in a non-JVM language seems odd and more likely the // caller was shooting for the String one, but ugh who knows. this issue showcases again how // badly bytecode should either change to use gremlin-language and go away or bytecode should // get a safer way to be translated to a traversal with more explicit calls that don't rely // on reflection. - if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0 && null == newArguments[0] && - method.getParameterTypes()[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) { - found = false; + var parametersTypes = method.getParameterTypes(); + if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0) { + //first case has((T)null, ...) instead of has((String)null, ...) + if (null == newArguments[0] && + parametersTypes[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) { + found = false; + } else if (newArguments.length == 3 && newArguments[2] == null && parametersTypes[0] == String.class && + parametersTypes[1] == String.class && + parametersTypes[2] == P.class) { + //the second case has(String, String, (P)(null)) instead of has(String,String, (Object)null) + found = false; + } } if (found) {