bitflicker64 commented on code in PR #824: URL: https://github.com/apache/hugegraph-toolchain/pull/824#discussion_r4301465404
########## hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/util/ReadOnlyCypher.java: ########## @@ -0,0 +1,570 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import org.apache.hugegraph.exception.ExternalException; + +/** + * A bounded, deliberately restricted Cypher grammar for Text2GQL execution. + * This is not a complete Cypher parser. Only read clauses, literal expressions, + * parameters and explicitly listed functions are accepted; unsupported syntax + * must fail closed instead of being forwarded to the graph server. + */ +public final class ReadOnlyCypher { + + public static final int MAX_PATH_HOPS = 10; + + private static final int MAX_LENGTH = 10000; + private static final int MAX_TOKENS = 4096; + private static final int MAX_DEPTH = 32; + private static final Set<String> KEYWORDS = Set.of( + "MATCH", "OPTIONAL", "WHERE", "WITH", "RETURN", "ORDER", "BY", "SKIP", "LIMIT", + "AS", "DISTINCT", "ASC", "ASCENDING", "DESC", "DESCENDING", "AND", "OR", "XOR", + "NOT", "IN", "IS", "NULL", "TRUE", "FALSE", "STARTS", "ENDS", "CONTAINS", + "CREATE", "INSERT", "MERGE", "SET", "REMOVE", "DELETE", "DETACH", "DROP", "CALL", + "LOAD", "CSV", "UNION", "ALL", "FOREACH", "USE", "SHOW", "TERMINATE", "ALTER", + "GRANT", "DENY", "REVOKE", "START", "STOP", "TRANSACTION", "TRANSACTIONS", + "DATABASE", "DATABASES", "EXPLAIN", "PROFILE", "FINISH", "YIELD", "UNWIND", + "CASE", "WHEN", "THEN", "ELSE", "END"); + private static final Set<String> FUNCTIONS = Set.of( + "id", "elementid", "label", "labels", "type", "properties", "keys", "length", "size", + "head", "last", "tail", "reverse", "coalesce", "exists", "count", "sum", "avg", "min", + "max", "collect", "stdev", "stdevp", "percentilecont", "percentiledisc", "abs", "ceil", + "ceiling", "floor", "round", "sign", "sqrt", "exp", "log", "log10", "rand", "left", + "right", "trim", "ltrim", "rtrim", "toupper", "tolower", "replace", "substring", "split", + "tostring", "tointeger", "tofloat", "toboolean", "range", "nodes", "relationships", + "startnode", "endnode", "timestamp"); + + private ReadOnlyCypher() { + } + + public static Set<String> supportedFunctions() { + return FUNCTIONS; + } + + public static void validate(String query) { + if (query == null || query.length() > MAX_LENGTH || query.isBlank()) { + throw invalid(); + } + new Parser(tokenize(query)).query(); + } + + private static ExternalException invalid() { + return new ExternalException("text2gql.cypher.read-only"); + } + + private static List<Token> tokenize(String query) { + List<Token> tokens = new ArrayList<>(); + int position = 0; + while (position < query.length()) { + char current = query.charAt(position); + if (current == ' ' || current == '\t' || current == '\r' || current == '\n') { + position++; + continue; + } + int start = position; + Kind kind; + if (current == '\'' || current == '"' || current == '`') { + position = quotedEnd(query, position, current); + kind = current == '`' ? Kind.QUOTED_IDENTIFIER : Kind.STRING; + } else if (current == '$') { Review Comment: Important: `ReadOnlyCypher` accepts `$name` parameters, but the Text2GQL path never sends bindings, so a generated parameterized query runs and silently returns nothing on HugeGraph 1.7.0. `CypherController` passes only `query.getContent()`, and the client `CypherAPI.post` sends only the string. With `MATCH (n:person) WHERE n.name = $name RETURN n LIMIT 100`, `normalize` accepts it and the UI runs it. translation 1.0.4 (used by Server 1.7.0) turns the missing `$name` into the `' cypher.null'` constant, so the result is empty with no error. I reproduced this locally with translation 1.0.4 on TinkerGraph: zero rows, while the same query with 'Alice' inlined returns the vertex. Servers with apache/hugegraph#3289 fail instead with "Missing Cypher parameter". The prompt already forbids parameters. Please reject `PARAMETER` tokens here, update the class comment, and move the `$props`, `$offset`, `$limit` and `$name` cases in `ReadOnlyCypherTest` to the rejected list. ########## hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/ReadOnlyGremlinTest.java: ########## @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.unit; + +import org.apache.hugegraph.exception.ExternalException; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.util.ReadOnlyGremlin; +import org.junit.Test; + +public class ReadOnlyGremlinTest { + + @Test + public void acceptsReadTraversalsAndNestedPredicates() { + String[] queries = { + "g.V().limit(10)", + " g.V().count(); \n", + "g.E().hasLabel('knows').inV().values('name')", + "g.V().has('age', P.gte(18).and(P.lt(65))).count()", + "g.V().has('name', TextP.startingWith('A')).elementMap()", + "g.V().has('name', within(['Alice', 'Bob'])).valueMap(true)", + "g.V().has('age', between(-2.5, 3e2)).order().by('age', Order.desc)", + "g.V().project('name', 'friends').by('name').by(__.out('knows').count())", + "g.V().repeat(__.out('knows')).times(2).path().by('name')", + "g.V().groupCount().by('name').order(Scope.local).by(Column.values, desc)", + "g.V().order().by(label).by(id)", + "g.V().group().by(key).by(value)", Review Comment: Minor: This change adds bare `id`, `label`, `key` and `value` as constants, but no rejected-case test shows that they cannot be used as receivers. The parser joins one `.segment` to a bare name before the `CONSTANTS` lookup. That is why `by(id.class)` is rejected today. I checked this with a local probe. The rejected list covers `T.id.class` only, so a later change to that lookup could accept `by(label.getClass())` or `by(id.class.forName('x'))` and no test would fail. The Cypher rejected list also has only upper-case mutating clauses. Please add `g.V().by(label.class)`, `g.V().by(id.getClass())` and `g.V().order().by(key.class)` to `rejectsWritesScriptsAndDynamicGroovy`. In `ReadOnlyCypherTest`, add lower-case and mixed-case writes such as `match (n) detach delete n return n` and `Match (n) Set n.x = 1 Return n`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
