This is an automated email from the ASF dual-hosted git repository.
florianhockmann pushed a commit to branch TINKERPOP-3063
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit e0e52b7300443fd9ce51007750585170e68d82fe
Author: Florian Hockmann <f...@florian-hockmann.de>
AuthorDate: Wed Mar 13 15:16:02 2024 +0100
TINKERPOP-3063 Fix authentication for concurrency
Executing multiple queries in parallel could lead to authentication
failures if `MaxInProcessPerConnection` is set to a value higher than
`1` as the second request could then be sent to the server while the
server was still waiting for the authentication challenge response from
the driver for the first query.
This made it necessary to set `MaxInProcessPerConnection=1` as a
workaround for such scenarios which of course means that connection
pooling is less efficient.
Simply sending a validation request on each connection right after
establishing the connection and therefore before it can be used to
submit queries from the user should fix this issue.
The downside of this is of course that connection establishment takes
slightly longer. I think this is an acceptable trade-off even for
scenarios where authentication is not used since this also validates the
connection irrespective of authentication and also because this delay
only occurs once right at the start of an application.
---
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.6.x.asciidoc | 16 +++++
.../src/Gremlin.Net/Driver/Connection.cs | 2 +-
.../src/Gremlin.Net/Driver/ConnectionPool.cs | 17 +++++
.../Driver/GremlinClientAuthenticationTests.cs | 73 +++++++++++++---------
5 files changed, 79 insertions(+), 30 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 27fde68afd..f1622b1bad 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -24,6 +24,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
=== TinkerPop 3.6.7 (NOT OFFICIALLY RELEASED YET)
* Fixed a bug in Gremlin.Net for .NET 8 that led to exceptions:
`InvalidOperationException: Enumeration has not started. Call MoveNext.`
+* Fixed a bug in the Gremlin.Net driver that could lead to failed
authentication if multiple requests are executed in parallel.
* Fixed message requestId serialization in `gremlin-python`.
* Improved performance of `PathRetractionStrategy` for traversals that carry
many children, but don't hold many labels to propogate.
* Fixed bug in bytecode translation of `g.tx().commit()` and
`g.tx().rollback()` in all languages.
diff --git a/docs/src/upgrade/release-3.6.x.asciidoc
b/docs/src/upgrade/release-3.6.x.asciidoc
index da190bdc89..ac268528d1 100644
--- a/docs/src/upgrade/release-3.6.x.asciidoc
+++ b/docs/src/upgrade/release-3.6.x.asciidoc
@@ -43,6 +43,22 @@ Gremlin.Net driver expected.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-3029[TINKERPOP-3029]
+==== Gremlin.Net: Fixed a bug related to authentication for multiple requests
in parallel
+
+Executing multiple queries in parallel could lead to authentication failures
if `MaxInProcessPerConnection` is set to a
+value higher than `1` as the second request could then be sent to the server
while the server was still waiting for
+the authentication challenge response from the driver for the first query.
+
+This is now fixed by simply executing a validation request on each connection
right after establishing the connection.
+That validation request will already handle authentication if necessary so
user requests can afterwards directly be
+executed in parallel without needing to authenticate again.
+
+This is strictly speaking a breaking change since exceptions caused by
authentication failures (no/wrong credentials
+configured) now occur directly when the `GremlinClient` is instantiated.
+In previous versions, the exceptions would only occur when the first request
is executed.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-3063[TINKERPOP-3063]
+
=== Upgrading for Providers
diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs
b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs
index cabe587fcd..6e4da37b7c 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs
@@ -140,7 +140,7 @@ namespace Gremlin.Net.Driver
}
catch (Exception e)
{
- if (receivedMsg!.RequestId != null)
+ if (receivedMsg.RequestId != null)
{
if(_callbackByRequestId.TryRemove(receivedMsg.RequestId.Value, out var
responseHandler))
{
diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs
b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs
index e4f4a72ae2..82ecace0a7 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs
@@ -27,6 +27,7 @@ using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Gremlin.Net.Driver.Exceptions;
+using Gremlin.Net.Driver.Messages;
using Gremlin.Net.Process;
using Microsoft.Extensions.Logging;
using Polly;
@@ -163,6 +164,7 @@ namespace Gremlin.Net.Driver
try
{
await
newConnection.ConnectAsync(_cts.Token).ConfigureAwait(false);
+ await
ValidateConnectionAsync(newConnection).ConfigureAwait(false);
}
catch (Exception)
{
@@ -173,6 +175,21 @@ namespace Gremlin.Net.Driver
return newConnection;
}
+ /// <summary>
+ /// Validates the connection by executing a simple validation request.
+ /// </summary>
+ /// <remarks>This also authenticates the user if necessary and
therefore ensures that user requests can directly
+ /// be executed.</remarks>
+ private async Task ValidateConnectionAsync(IConnection connection)
+ {
+ await connection.SubmitAsync<string>(ValidationRequestMessage,
_cts.Token).ConfigureAwait(false);
+ }
+
+ private static readonly string ValidationRequest = "''";
+
+ private static readonly RequestMessage ValidationRequestMessage =
RequestMessage.Build(Tokens.OpsEval)
+ .AddArgument(Tokens.ArgsGremlin, ValidationRequest).Create();
+
private IConnection GetConnectionFromPool()
{
var connections = _connections.Snapshot;
diff --git
a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientAuthenticationTests.cs
b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientAuthenticationTests.cs
index e56e282248..e9a00d5579 100644
---
a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientAuthenticationTests.cs
+++
b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientAuthenticationTests.cs
@@ -22,13 +22,13 @@
#endregion
using System;
+using System.Collections.Generic;
using System.Net.WebSockets;
using System.Threading.Tasks;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
using Gremlin.Net.Driver;
using Gremlin.Net.Driver.Exceptions;
-using Gremlin.Net.IntegrationTest.Util;
using Xunit;
namespace Gremlin.Net.IntegrationTest.Driver
@@ -37,7 +37,6 @@ namespace Gremlin.Net.IntegrationTest.Driver
{
private static readonly string TestHost =
ConfigProvider.Configuration["TestServerIpAddress"];
private static readonly int TestPort =
Convert.ToInt32(ConfigProvider.Configuration["TestSecureServerPort"]);
- private readonly RequestMessageProvider _requestMessageProvider = new
RequestMessageProvider();
public static bool
IgnoreCertificateValidationLiveDangerouslyWheeeeeeee(
object sender,
@@ -49,48 +48,41 @@ namespace Gremlin.Net.IntegrationTest.Driver
}
[Fact]
- public async Task ShouldThrowForMissingCredentials()
+ public void ShouldThrowForMissingCredentials()
{
- ClientWebSocketOptions optionsSet = null;
var webSocketConfiguration =
new Action<ClientWebSocketOptions>(options =>
{
options.RemoteCertificateValidationCallback +=
IgnoreCertificateValidationLiveDangerouslyWheeeeeeee;
- optionsSet = options;
});
var gremlinServer = new GremlinServer(TestHost, TestPort,
enableSsl: true);
- using (var gremlinClient = new GremlinClient(gremlinServer,
webSocketConfiguration: webSocketConfiguration))
- {
- var exception = await
Assert.ThrowsAsync<InvalidOperationException>(
- async () => await
gremlinClient.SubmitWithSingleResultAsync<string>(_requestMessageProvider
- .GetDummyMessage()));
+ var aggregateException = Assert.Throws<AggregateException>(() =>
+ new GremlinClient(gremlinServer, webSocketConfiguration:
webSocketConfiguration));
- Assert.Contains("authentication", exception.Message);
- Assert.Contains("credentials", exception.Message);
- }
+ var innerException = aggregateException.InnerException;
+ Assert.Equal(typeof(InvalidOperationException),
innerException!.GetType());
+ Assert.Contains("authentication", innerException.Message);
+ Assert.Contains("credentials", innerException.Message);
}
[Theory]
[InlineData("unknownUser", "passwordDoesntMatter")]
[InlineData("stephen", "wrongPassword")]
- public async Task ShouldThrowForWrongCredentials(string username,
string password)
+ public void ShouldThrowForWrongCredentials(string username, string
password)
{
- ClientWebSocketOptions optionsSet = null;
var webSocketConfiguration =
new Action<ClientWebSocketOptions>(options =>
{
options.RemoteCertificateValidationCallback +=
IgnoreCertificateValidationLiveDangerouslyWheeeeeeee;
- optionsSet = options;
});
var gremlinServer = new GremlinServer(TestHost, TestPort,
username: username, password: password, enableSsl: true);
- using (var gremlinClient = new GremlinClient(gremlinServer,
webSocketConfiguration: webSocketConfiguration))
- {
- var exception = await Assert.ThrowsAsync<ResponseException>(
- async () => await
gremlinClient.SubmitWithSingleResultAsync<string>(_requestMessageProvider
- .GetDummyMessage()));
+ var aggregateException = Assert.Throws<AggregateException>(() =>
+ new GremlinClient(gremlinServer, webSocketConfiguration:
webSocketConfiguration));
- Assert.Contains("Unauthorized", exception.Message);
- }
+ var innerException = aggregateException.InnerException;
+ Assert.Equal(typeof(ResponseException), innerException!.GetType());
+
+ Assert.Contains("Unauthorized", innerException.Message);
}
[Theory]
@@ -98,22 +90,45 @@ namespace Gremlin.Net.IntegrationTest.Driver
public async Task
ScriptShouldBeEvaluatedAndResultReturnedForCorrectCredentials(string requestMsg,
string expectedResponse)
{
- ClientWebSocketOptions optionsSet = null;
var webSocketConfiguration =
new Action<ClientWebSocketOptions>(options =>
{
options.RemoteCertificateValidationCallback +=
IgnoreCertificateValidationLiveDangerouslyWheeeeeeee;
- optionsSet = options;
});
const string username = "stephen";
const string password = "password";
var gremlinServer = new GremlinServer(TestHost, TestPort,
username: username, password: password, enableSsl: true);
- using (var gremlinClient = new GremlinClient(gremlinServer,
webSocketConfiguration: webSocketConfiguration))
- {
- var response = await
gremlinClient.SubmitWithSingleResultAsync<string>(requestMsg);
+ using var gremlinClient = new GremlinClient(gremlinServer,
webSocketConfiguration: webSocketConfiguration);
+
+ var response = await
gremlinClient.SubmitWithSingleResultAsync<string>(requestMsg);
+
+ Assert.Equal(expectedResponse, response);
+ }
- Assert.Equal(expectedResponse, response);
+ [Fact]
+ public async Task
ExecutingRequestsInParallelOverSameConnectionShouldWorkWithAuthentication()
+ {
+ var webSocketConfiguration =
+ new Action<ClientWebSocketOptions>(options =>
+ {
+ options.RemoteCertificateValidationCallback +=
IgnoreCertificateValidationLiveDangerouslyWheeeeeeee;
+ });
+ const string username = "stephen";
+ const string password = "password";
+ const int nrOfParallelRequests = 10;
+ var gremlinServer = new GremlinServer(TestHost, TestPort,
username: username, password: password, enableSsl: true);
+ var connectionPoolSettings = new ConnectionPoolSettings
+ { PoolSize = 1, MaxInProcessPerConnection =
nrOfParallelRequests };
+
+ using var gremlinClient = new GremlinClient(gremlinServer,
webSocketConfiguration: webSocketConfiguration,
+ connectionPoolSettings: connectionPoolSettings);
+ var tasks = new List<Task>(nrOfParallelRequests);
+ for (var i = 0; i < nrOfParallelRequests; i++)
+ {
+
tasks.Add(gremlinClient.SubmitWithSingleResultAsync<string>("''"));
}
+
+ await Task.WhenAll(tasks);
}
}
}
\ No newline at end of file
