github-advanced-security[bot] commented on code in PR #20594:
URL: https://github.com/apache/druid/pull/20594#discussion_r4276429833
##########
sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java:
##########
@@ -1107,6 +1110,73 @@
}
}
+ @Test
+ public void testCreateStatementFailsAuthenticationAndFreesConnectionSlot()
throws Exception
+ {
+ final AvaticaServerConfig config = new AvaticaServerConfig();
+ config.maxConnections = 1;
+ config.maxStatementsPerConnection = STATEMENT_LIMIT;
+
+ final ScheduledExecutorService exec =
Execs.scheduledSingleThreaded("testCreateStatementAuthFailure");
+ final List<Authenticator> authenticators = ImmutableList.of(
+ new AllowAllAuthenticator()
+ {
+ @Override
+ public AuthenticationResult authenticateJDBCContext(Map<String,
Object> context)
+ {
+ if (!"druid".equals(context.get("password"))) {
+ return null;
+ }
+ return new AuthenticationResult((String) context.get("user"),
AuthConfig.ALLOW_ALL_NAME, null, null);
+ }
+ }
+ );
+
+ final DruidMeta druidMeta = new DruidMeta(
+ makeStatementFactory(),
+ DefaultQueryConfig.NIL,
+ config,
+ new ErrorHandler(new ServerConfig()),
+ exec,
+ authenticators,
+ new ResultFetcherFactory(config.fetchTimeoutMs)
+ );
+
+ try (final ServerWrapper authServer = new ServerWrapper(druidMeta)) {
+ try (final Connection badConnection =
authServer.getConnection("regularUser", "wrong-password")) {
+ // openConnection() succeeds even with bad credentials; authentication
is deferred.
+ final AvaticaClientRuntimeException ex = Assertions.assertThrows(
+ AvaticaClientRuntimeException.class,
+ badConnection::createStatement
+ );
+ Assertions.assertTrue(ex.getMessage().contains("Unauthorized"));
+ }
+
+ // The failed connection must not have consumed the only available slot:
a correctly
+ // authenticated connection should succeed immediately afterward, with
no need to wait
+ // for the idle timeout or for another connection attempt to trigger
closeIfEmpty() eviction.
+ try (final Connection goodConnection =
authServer.getConnection("regularUser", "druid")) {
+ // codeql[java/database-resource-leak] - held open deliberately to
occupy the only slot
+ final Statement statement = goodConnection.createStatement();
+ final ResultSet resultSet = statement.executeQuery("SELECT COUNT(*) AS
cnt FROM druid.foo");
Review Comment:
## CodeQL / Potential database resource leak
This ResultSet is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/12088)
##########
sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java:
##########
@@ -1107,6 +1110,73 @@
}
}
+ @Test
+ public void testCreateStatementFailsAuthenticationAndFreesConnectionSlot()
throws Exception
+ {
+ final AvaticaServerConfig config = new AvaticaServerConfig();
+ config.maxConnections = 1;
+ config.maxStatementsPerConnection = STATEMENT_LIMIT;
+
+ final ScheduledExecutorService exec =
Execs.scheduledSingleThreaded("testCreateStatementAuthFailure");
+ final List<Authenticator> authenticators = ImmutableList.of(
+ new AllowAllAuthenticator()
+ {
+ @Override
+ public AuthenticationResult authenticateJDBCContext(Map<String,
Object> context)
+ {
+ if (!"druid".equals(context.get("password"))) {
+ return null;
+ }
+ return new AuthenticationResult((String) context.get("user"),
AuthConfig.ALLOW_ALL_NAME, null, null);
+ }
+ }
+ );
+
+ final DruidMeta druidMeta = new DruidMeta(
+ makeStatementFactory(),
+ DefaultQueryConfig.NIL,
+ config,
+ new ErrorHandler(new ServerConfig()),
+ exec,
+ authenticators,
+ new ResultFetcherFactory(config.fetchTimeoutMs)
+ );
+
+ try (final ServerWrapper authServer = new ServerWrapper(druidMeta)) {
+ try (final Connection badConnection =
authServer.getConnection("regularUser", "wrong-password")) {
+ // openConnection() succeeds even with bad credentials; authentication
is deferred.
+ final AvaticaClientRuntimeException ex = Assertions.assertThrows(
+ AvaticaClientRuntimeException.class,
+ badConnection::createStatement
+ );
+ Assertions.assertTrue(ex.getMessage().contains("Unauthorized"));
+ }
+
+ // The failed connection must not have consumed the only available slot:
a correctly
+ // authenticated connection should succeed immediately afterward, with
no need to wait
+ // for the idle timeout or for another connection attempt to trigger
closeIfEmpty() eviction.
+ try (final Connection goodConnection =
authServer.getConnection("regularUser", "druid")) {
+ // codeql[java/database-resource-leak] - held open deliberately to
occupy the only slot
+ final Statement statement = goodConnection.createStatement();
Review Comment:
## CodeQL / Potential database resource leak
This Statement is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/12087)
--
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]
