Skip to content

Commit aac51f1

Browse files
lvcalvca
committed
fix: check idempotency in query api for all languages
1 parent 0735b32 commit aac51f1

File tree

10 files changed

+195
-16
lines changed

10 files changed

+195
-16
lines changed

‎engine/src/main/java/com/arcadedb/exception/QueryNotIdempotentException.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright © 2021-present Arcade Data Ltd ([email protected])
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-FileCopyrightText: 2021-present Arcade Data Ltd ([email protected])
17+
* SPDX-License-Identifier: Apache-2.0
18+
*/
19+
package com.arcadedb.exception;
20+
21+
public class QueryNotIdempotentException extends ArcadeDBException {
22+
public QueryNotIdempotentException(final String message) {
23+
super(message);
24+
}
25+
}

‎engine/src/main/java/com/arcadedb/query/opencypher/query/OpenCypherQueryEngine.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.arcadedb.database.DatabaseInternal;
2323
import com.arcadedb.exception.CommandExecutionException;
2424
import com.arcadedb.exception.CommandParsingException;
25+
import com.arcadedb.exception.QueryNotIdempotentException;
2526
import com.arcadedb.exception.SchemaException;
2627
import com.arcadedb.query.opencypher.ast.CypherAdminStatement;
2728
import com.arcadedb.query.opencypher.ast.CypherDDLStatement;
@@ -144,10 +145,10 @@ public ResultSet query(final String query, final ContextConfiguration configurat
144145
final CypherStatement statement = database.getCypherStatementCache().get(actualQuery);
145146

146147
if (!statement.isReadOnly())
147-
throw new CommandExecutionException("Query contains write operations. Use command() instead of query()");
148+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
148149

149150
return execute(actualQuery, statement, configuration, parameters, explain, profile);
150-
} catch (final CommandExecutionException | CommandParsingException | SecurityException e) {
151+
} catch (final QueryNotIdempotentException | CommandExecutionException | CommandParsingException | SecurityException e) {
151152
throw e;
152153
} catch (final Exception e) {
153154
throw new CommandExecutionException("Error executing Cypher query: " + query, e);

‎engine/src/main/java/com/arcadedb/query/sql/SQLQueryEngine.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.arcadedb.database.DatabaseInternal;
2323
import com.arcadedb.database.Identifiable;
2424
import com.arcadedb.exception.CommandExecutionException;
25+
import com.arcadedb.exception.QueryNotIdempotentException;
2526
import com.arcadedb.exception.CommandSQLParsingException;
2627
import com.arcadedb.function.FunctionDefinition;
2728
import com.arcadedb.query.QueryEngine;
@@ -84,7 +85,7 @@ public String getLanguage() {
8485
public ResultSet query(final String query, ContextConfiguration configuration, final Map<String, Object> parameters) {
8586
final Statement statement = parse(query, database);
8687
if (!statement.isIdempotent())
87-
throw new IllegalArgumentException("Query '" + query + "' is not idempotent");
88+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
8889

8990
statement.setLimit(new Limit(JJTLIMIT).setValue((int) database.getResultSetLimit()));
9091
return statement.execute(database, parameters);
@@ -94,7 +95,7 @@ public ResultSet query(final String query, ContextConfiguration configuration, f
9495
public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) {
9596
final Statement statement = parse(query, database);
9697
if (!statement.isIdempotent())
97-
throw new IllegalArgumentException("Query '" + query + "' is not idempotent");
98+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
9899

99100
statement.setLimit(new Limit(JJTLIMIT).setValue((int) database.getResultSetLimit()));
100101
return statement.execute(database, parameters);

‎engine/src/main/java/com/arcadedb/query/sql/SQLScriptQueryEngine.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.arcadedb.GlobalConfiguration;
2323
import com.arcadedb.database.DatabaseInternal;
2424
import com.arcadedb.exception.CommandExecutionException;
25+
import com.arcadedb.exception.QueryNotIdempotentException;
2526
import com.arcadedb.exception.CommandSQLParsingException;
2627
import com.arcadedb.query.QueryEngine;
2728
import com.arcadedb.query.sql.antlr.SQLAntlrParser;
@@ -62,11 +63,9 @@ public String getLanguage() {
6263
@Override
6364
public ResultSet query(final String query, ContextConfiguration configuration, final Map<String, Object> parameters) {
6465
final List<Statement> statements = parseScript(query, database);
65-
statements.stream().map((statement) -> {
66-
if (statement.isIdempotent())
67-
throw new IllegalArgumentException("Query '" + query + "' is not idempotent");
68-
return null;
69-
});
66+
for (final Statement statement : statements)
67+
if (!statement.isIdempotent())
68+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
7069

7170
final BasicCommandContext context = new BasicCommandContext();
7271
context.setDatabase(database.getWrappedDatabaseInstance());
@@ -77,11 +76,9 @@ public ResultSet query(final String query, ContextConfiguration configuration, f
7776
@Override
7877
public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) {
7978
final List<Statement> statements = parseScript(query, database);
80-
statements.stream().map((statement) -> {
81-
if (statement.isIdempotent())
82-
throw new IllegalArgumentException("Query '" + query + "' is not idempotent");
83-
return null;
84-
});
79+
for (final Statement statement : statements)
80+
if (!statement.isIdempotent())
81+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
8582

8683
final BasicCommandContext context = new BasicCommandContext();
8784
context.setDatabase(database.getWrappedDatabaseInstance());

‎graphql/src/main/java/com/arcadedb/graphql/query/GraphQLQueryEngine.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.arcadedb.ContextConfiguration;
2222
import com.arcadedb.exception.CommandParsingException;
23+
import com.arcadedb.exception.QueryNotIdempotentException;
2324
import com.arcadedb.graphql.parser.Definition;
2425
import com.arcadedb.graphql.parser.Document;
2526
import com.arcadedb.graphql.parser.GraphQLParser;
@@ -82,11 +83,15 @@ private static Set<OperationType> detectGraphQLOperationTypes(final String query
8283

8384
@Override
8485
public ResultSet query(final String query, ContextConfiguration configuration, final Map<String, Object> parameters) {
86+
if (!analyze(query).isIdempotent())
87+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
8588
return command(query, null, parameters);
8689
}
8790

8891
@Override
8992
public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) {
93+
if (!analyze(query).isIdempotent())
94+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
9095
return command(query, null, parameters);
9196
}
9297

‎gremlin/src/main/java/com/arcadedb/cypher/query/CypherQueryEngine.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.arcadedb.ContextConfiguration;
2222
import com.arcadedb.cypher.ArcadeCypher;
2323
import com.arcadedb.exception.CommandExecutionException;
24+
import com.arcadedb.exception.QueryNotIdempotentException;
2425
import com.arcadedb.gremlin.ArcadeGraph;
2526
import com.arcadedb.query.QueryEngine;
2627
import com.arcadedb.query.sql.executor.ResultInternal;
@@ -56,11 +57,15 @@ public AnalyzedQuery analyze(final String query) {
5657

5758
@Override
5859
public ResultSet query(final String query, final ContextConfiguration configuration, final Map<String, Object> parameters) {
60+
if (!analyze(query).isIdempotent())
61+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
5962
return command(query, configuration, parameters);
6063
}
6164

6265
@Override
6366
public ResultSet query(final String query, final ContextConfiguration configuration, final Object... parameters) {
67+
if (!analyze(query).isIdempotent())
68+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
6469
return command(query, null, parameters);
6570
}
6671

‎gremlin/src/main/java/com/arcadedb/gremlin/query/GremlinQueryEngine.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.arcadedb.ContextConfiguration;
2222
import com.arcadedb.exception.CommandParsingException;
23+
import com.arcadedb.exception.QueryNotIdempotentException;
2324
import com.arcadedb.gremlin.ArcadeGraph;
2425
import com.arcadedb.gremlin.ArcadeGremlin;
2526
import com.arcadedb.query.QueryEngine;
@@ -42,11 +43,15 @@ public String getLanguage() {
4243

4344
@Override
4445
public ResultSet query(final String query, ContextConfiguration configuration, final Map<String, Object> parameters) {
46+
if (!analyze(query).isIdempotent())
47+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
4548
return command(query, null, parameters);
4649
}
4750

4851
@Override
4952
public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) {
53+
if (!analyze(query).isIdempotent())
54+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
5055
return command(query, null, parameters);
5156
}
5257

‎mongodbw/src/main/java/com/arcadedb/mongo/query/MongoQueryEngine.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.arcadedb.ContextConfiguration;
2222
import com.arcadedb.exception.CommandParsingException;
23+
import com.arcadedb.exception.QueryNotIdempotentException;
2324
import com.arcadedb.log.LogManager;
2425
import com.arcadedb.mongo.MongoDBDatabaseWrapper;
2526
import com.arcadedb.query.OperationType;
@@ -90,6 +91,8 @@ private static Set<OperationType> detectMongoOperationTypes(final String query)
9091

9192
@Override
9293
public ResultSet query(final String query, ContextConfiguration configuration, final Map<String, Object> parameters) {
94+
if (!analyze(query).isIdempotent())
95+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
9396
try {
9497
return mongoDBWrapper.query(query);
9598
} catch (final Exception e) {
@@ -100,6 +103,8 @@ public ResultSet query(final String query, ContextConfiguration configuration, f
100103

101104
@Override
102105
public ResultSet query(final String query, ContextConfiguration configuration, final Object... parameters) {
106+
if (!analyze(query).isIdempotent())
107+
throw new QueryNotIdempotentException("Query '" + query + "' is not idempotent");
103108
return query(query, null, (Map) null);
104109
}
105110

‎server/src/main/java/com/arcadedb/server/http/handler/AbstractServerHttpHandler.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ public void handleRequest(final HttpServerExchange exchange) {
194194
.log(this, getUserSevereErrorLogLevel(), "Error on command execution (%s): %s", getClass().getSimpleName(),
195195
e.getMessage());
196196
sendErrorResponse(exchange, 404, "Record not found", e, null);
197+
} catch (final QueryNotIdempotentException e) {
198+
LogManager.instance()
199+
.log(this, getUserSevereErrorLogLevel(), "Error on command execution (%s): %s", getClass().getSimpleName(),
200+
e.getMessage());
201+
sendErrorResponse(exchange, 400, "Query is not idempotent", e, null);
197202
} catch (final IllegalArgumentException e) {
198203
LogManager.instance()
199204
.log(this, getUserSevereErrorLogLevel(), "Error on command execution (%s): %s", getClass().getSimpleName(),
@@ -204,8 +209,12 @@ public void handleRequest(final HttpServerExchange exchange) {
204209
if (e.getCause() != null)
205210
realException = e.getCause();
206211

207-
// If the root cause is a SecurityException, return 403 instead of 500
208-
if (realException instanceof SecurityException) {
212+
if (realException instanceof QueryNotIdempotentException) {
213+
LogManager.instance()
214+
.log(this, getUserSevereErrorLogLevel(), "Error on command execution (%s): %s", getClass().getSimpleName(),
215+
realException.getMessage());
216+
sendErrorResponse(exchange, 400, "Query is not idempotent", realException, null);
217+
} else if (realException instanceof SecurityException) {
209218
LogManager.instance().log(this, getUserSevereErrorLogLevel(), "Security error on command execution (%s): %s",
210219
SecurityException.class.getSimpleName(), realException.getMessage());
211220
sendErrorResponse(exchange, 403, "Security error", realException, null);
@@ -224,6 +233,11 @@ public void handleRequest(final HttpServerExchange exchange) {
224233
LogManager.instance().log(this, getUserSevereErrorLogLevel(), "Security error on transaction execution (%s): %s",
225234
SecurityException.class.getSimpleName(), realException.getMessage());
226235
sendErrorResponse(exchange, 403, "Security error", realException, null);
236+
} else if (realException instanceof QueryNotIdempotentException) {
237+
LogManager.instance()
238+
.log(this, getUserSevereErrorLogLevel(), "Error on command execution (%s): %s", getClass().getSimpleName(),
239+
realException.getMessage());
240+
sendErrorResponse(exchange, 400, "Query is not idempotent", realException, null);
227241
} else {
228242
LogManager.instance()
229243
.log(this, getUserSevereErrorLogLevel(), "Error on transaction execution (%s): %s", getClass().getSimpleName(),

‎server/src/test/java/com/arcadedb/server/http/QueryEndpointReadOnlyTest.java‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright © 2021-present Arcade Data Ltd ([email protected])
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-FileCopyrightText: 2021-present Arcade Data Ltd ([email protected])
17+
* SPDX-License-Identifier: Apache-2.0
18+
*/
19+
package com.arcadedb.server.http;
20+
21+
import com.arcadedb.serializer.json.JSONObject;
22+
import com.arcadedb.server.BaseGraphServerTest;
23+
import org.junit.jupiter.api.Test;
24+
25+
import java.net.HttpURLConnection;
26+
import java.net.URL;
27+
import java.util.Base64;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
31+
class QueryEndpointReadOnlyTest extends BaseGraphServerTest {
32+
33+
private static final String DATABASE_NAME = "graph";
34+
35+
@Test
36+
void queryEndpointRejectsSqlInsert() throws Exception {
37+
testEachServer((serverIndex) -> {
38+
executeCommand(serverIndex, "sql", "CREATE DOCUMENT TYPE ReadOnlyTest IF NOT EXISTS");
39+
40+
final HttpURLConnection connection = (HttpURLConnection) new URL(
41+
"http://127.0.0.1:248" + serverIndex + "/api/v1/query/" + DATABASE_NAME).openConnection();
42+
connection.setRequestMethod("POST");
43+
connection.setRequestProperty("Authorization",
44+
"Basic " + Base64.getEncoder().encodeToString(("root:" + DEFAULT_PASSWORD_FOR_TESTS).getBytes()));
45+
46+
final JSONObject payload = new JSONObject();
47+
payload.put("language", "sql");
48+
payload.put("command", "INSERT INTO ReadOnlyTest SET name='should_fail'");
49+
50+
formatPayload(connection, payload);
51+
connection.connect();
52+
53+
try {
54+
assertThat(connection.getResponseCode()).isEqualTo(400);
55+
final String error = readError(connection);
56+
assertThat(error).contains("idempotent");
57+
} finally {
58+
connection.disconnect();
59+
}
60+
});
61+
}
62+
63+
@Test
64+
void queryEndpointRejectsSqlScriptInsert() throws Exception {
65+
testEachServer((serverIndex) -> {
66+
executeCommand(serverIndex, "sql", "CREATE DOCUMENT TYPE ReadOnlyTest2 IF NOT EXISTS");
67+
68+
final HttpURLConnection connection = (HttpURLConnection) new URL(
69+
"http://127.0.0.1:248" + serverIndex + "/api/v1/query/" + DATABASE_NAME).openConnection();
70+
connection.setRequestMethod("POST");
71+
connection.setRequestProperty("Authorization",
72+
"Basic " + Base64.getEncoder().encodeToString(("root:" + DEFAULT_PASSWORD_FOR_TESTS).getBytes()));
73+
74+
final JSONObject payload = new JSONObject();
75+
payload.put("language", "sqlscript");
76+
payload.put("command", "INSERT INTO ReadOnlyTest2 SET name='should_fail';");
77+
78+
formatPayload(connection, payload);
79+
connection.connect();
80+
81+
try {
82+
assertThat(connection.getResponseCode()).isEqualTo(400);
83+
final String error = readError(connection);
84+
assertThat(error).contains("idempotent");
85+
} finally {
86+
connection.disconnect();
87+
}
88+
});
89+
}
90+
91+
@Test
92+
void queryEndpointAllowsSqlSelect() throws Exception {
93+
testEachServer((serverIndex) -> {
94+
executeCommand(serverIndex, "sql", "CREATE DOCUMENT TYPE ReadOnlyTest3 IF NOT EXISTS");
95+
executeCommand(serverIndex, "sql", "INSERT INTO ReadOnlyTest3 SET name='readable'");
96+
97+
final HttpURLConnection connection = (HttpURLConnection) new URL(
98+
"http://127.0.0.1:248" + serverIndex + "/api/v1/query/" + DATABASE_NAME).openConnection();
99+
connection.setRequestMethod("POST");
100+
connection.setRequestProperty("Authorization",
101+
"Basic " + Base64.getEncoder().encodeToString(("root:" + DEFAULT_PASSWORD_FOR_TESTS).getBytes()));
102+
103+
final JSONObject payload = new JSONObject();
104+
payload.put("language", "sql");
105+
payload.put("command", "SELECT FROM ReadOnlyTest3");
106+
107+
formatPayload(connection, payload);
108+
connection.connect();
109+
110+
try {
111+
final String response = readResponse(connection);
112+
assertThat(connection.getResponseCode()).isEqualTo(200);
113+
114+
final JSONObject responseJson = new JSONObject(response);
115+
assertThat(responseJson.has("result")).isTrue();
116+
} finally {
117+
connection.disconnect();
118+
}
119+
});
120+
}
121+
}

0 commit comments

Comments
 (0)