This is an automated email from the ASF dual-hosted git repository.
kenhuuu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master by this push:
new b2b2bcb0d1 Prevent transaction leak on failed begin CTR
b2b2bcb0d1 is described below
commit b2b2bcb0d1e5bbc525fb8cb74b6652d688f409fa
Author: Ken Hu <[email protected]>
AuthorDate: Thu Jul 16 11:34:17 2026 -0700
Prevent transaction leak on failed begin CTR
There's two related issues here which both cause transaction leaks.
The first is that the ordering of checking for errors matters and
transactions needs to be last or else a transaction could get
opened and leaked since the response will actually be an error.
The second is the transaction might not properly begin itself so
it needs to be cleaned up.
Assisted-by: Codex:gpt-5.5
---
.../server/handler/HttpGremlinEndpointHandler.java | 72 ++++++++++++----------
.../GremlinServerHttpTransactionIntegrateTest.java | 19 ++++++
2 files changed, 57 insertions(+), 34 deletions(-)
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
index 14f4a4584a..13531f856d 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
@@ -213,6 +213,39 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
requestMessage.getGremlin());
}
+ // Validate the request before any transaction lifecycle side
effects.
+ final Map<String, Object> args = requestMessage.getFields();
+ final String language = args.containsKey(Tokens.ARGS_LANGUAGE)
? (String) args.get(Tokens.ARGS_LANGUAGE) : "gremlin-lang";
+ if
(gremlinExecutor.getScriptEngineManager().getEngineByName(language) == null) {
+ throw new
ProcessingException(GremlinError.scriptEngineNotAvailable(language));
+ }
+
+ // Guard against bad parameters while trying to parse
string-based parameters into a Map<String, Object>
+ if
(requestMessage.optionalField(Tokens.ARGS_PARAMETERS).isPresent()) {
+ Map<String, Object> parameters = null;
+ final String parametersString = (String)
requestMessage.getFields().get(Tokens.ARGS_PARAMETERS);
+ try {
+ parameters =
GremlinQueryParser.parseParameters(parametersString);
+ } catch (GremlinParserException e) {
+ throw new
ProcessingException(GremlinError.incorrectParameterFormat(parametersString, e));
+ }
+
+ if ("gremlin-groovy".equals(language)) {
+ final Set<String> badParameters =
IteratorUtils.set(IteratorUtils.<String>filter(
+ parameters.keySet().iterator(),
+ INVALID_PARAMETERS_KEYS::contains));
+ if (!badParameters.isEmpty()) {
+ throw new
ProcessingException(GremlinError.parameter(badParameters));
+ }
+ }
+
+ if (parameters.size() > settings.maxParameters) {
+ throw new
ProcessingException(GremlinError.parameter(parameters.size(),
settings.maxParameters));
+ }
+
+ requestCtx.setParameters(parameters);
+ }
+
// These guards prevent any obvious failures from returning
200 OK early by detecting them here and
// throwing before any other processing starts so the user
gets a better error code.
final String txId = requestCtx.getTransactionId();
@@ -246,39 +279,6 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
throw new
ProcessingException(GremlinError.transactionalControlRequiresTransaction());
}
- // validate script engine availability before committing the
200 response
- final Map<String, Object> args = requestMessage.getFields();
- final String language = args.containsKey(Tokens.ARGS_LANGUAGE)
? (String) args.get(Tokens.ARGS_LANGUAGE) : "gremlin-lang";
- if
(gremlinExecutor.getScriptEngineManager().getEngineByName(language) == null) {
- throw new
ProcessingException(GremlinError.scriptEngineNotAvailable(language));
- }
-
- // Guard against bad parameters while trying to parse
string-based parameters into a Map<String, Object>
- if
(requestMessage.optionalField(Tokens.ARGS_PARAMETERS).isPresent()) {
- Map<String, Object> parameters = null;
- final String parametersString = (String)
requestMessage.getFields().get(Tokens.ARGS_PARAMETERS);
- try {
- parameters =
GremlinQueryParser.parseParameters(parametersString);
- } catch (GremlinParserException e) {
- throw new
ProcessingException(GremlinError.incorrectParameterFormat(parametersString, e));
- }
-
- if ("gremlin-groovy".equals(language)) {
- final Set<String> badParameters =
IteratorUtils.set(IteratorUtils.<String>filter(
- parameters.keySet().iterator(),
- INVALID_PARAMETERS_KEYS::contains));
- if (!badParameters.isEmpty()) {
- throw new
ProcessingException(GremlinError.parameter(badParameters));
- }
- }
-
- if (parameters.size() > settings.maxParameters) {
- throw new
ProcessingException(GremlinError.parameter(parameters.size(),
settings.maxParameters));
- }
-
- requestCtx.setParameters(parameters);
- }
-
// Send back the 200 OK response header here since the
response is always chunk transfer encoded. Any
// failures that follow this will show up in the response body
instead.
coordinator.writeHeader(createResponseHeaders(ctx, serializer,
requestCtx).toArray(CharSequence[]::new));
@@ -522,7 +522,8 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
private void doBegin(final Context ctx) throws Exception {
final String traversalSourceName =
ctx.getRequestMessage().getField(Tokens.ARGS_G);
- final UnmanagedTransaction txCtx;
+ UnmanagedTransaction txCtx = null;
+ boolean closeTransactionOnFailure = true;
try {
txCtx = transactionManager.create(traversalSourceName);
ctx.setTransactionId(txCtx.getTransactionId());
@@ -531,6 +532,7 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
graph.tx().begin();
return null;
}), ctx).get(5000, TimeUnit.MILLISECONDS); // Not an option for
now, but 5s should be plenty.
+ closeTransactionOnFailure = false;
} catch (IllegalStateException ise) {
throw new
ProcessingException(GremlinError.maxTransactionsExceeded(ise.getMessage()));
} catch (IllegalArgumentException iae) {
@@ -539,6 +541,8 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
throw new
ProcessingException(GremlinError.transactionNotSupported(uoe));
} catch (ExecutionException | TimeoutException e) {
throw new
ProcessingException(GremlinError.transactionUnableToStart(e.getMessage()));
+ } finally {
+ if (closeTransactionOnFailure && txCtx != null) txCtx.close(false);
}
}
diff --git
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpTransactionIntegrateTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpTransactionIntegrateTest.java
index c1def1e6ec..d507810478 100644
---
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpTransactionIntegrateTest.java
+++
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpTransactionIntegrateTest.java
@@ -353,6 +353,25 @@ public class GremlinServerHttpTransactionIntegrateTest
extends AbstractGremlinSe
}
}
+ @Test
+ public void shouldNotCreateTransactionWhenBeginRequestValidationFails()
throws Exception {
+ final TransactionManager txManager =
server.getServerGremlinExecutor().getTransactionManager();
+
+ try (final CloseableHttpResponse r = postJson(client,
+ "{\"gremlin\":\"g.tx().begin()\",\"g\":\"" + GTX +
"\",\"language\":\"not-an-engine\"}")) {
+ assertEquals(400, r.getStatusLine().getStatusCode());
+ assertTrue(extractStatusMessage(r).contains("Script engine
[not-an-engine] is not available"));
+ }
+ assertEquals(0, txManager.getActiveTransactionCount());
+
+ try (final CloseableHttpResponse r = postJson(client,
+ "{\"gremlin\":\"g.tx().begin()\",\"g\":\"" + GTX +
"\",\"parameters\":\"[x:]\"}")) {
+ assertEquals(400, r.getStatusLine().getStatusCode());
+ assertTrue(extractStatusMessage(r).contains("could not be
converted into a Map"));
+ }
+ assertEquals(0, txManager.getActiveTransactionCount());
+ }
+
@Test
public void shouldTimeoutIdleTransactionWithNoOperations() throws
Exception {
final String txId = beginTx(client, GTX);
