Skip to content

Commit df30f7c

Browse files
authored
Fix wrong exception propagation on connection errors and channel binding failure (#1676)
See #1672, #1674, #1675 Inflight commands should not fail with ClosedConnectionException but with the actual error, e.g. a PgException reporting SQLSTATE 57P01 when pg_terminate_backend kills a connection mid-flight. When channel binding is required but SSL is not available, the PostgreSQL protocol recommends immediately closing the connection. The ChannelBindingException is now correctly reported to the caller rather than being masked by a subsequent ClosedConnectionException. Some portions of this content were created with the assistance of Claude Code. Fixup Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
1 parent 2d579e7 commit df30f7c

6 files changed

Lines changed: 55 additions & 32 deletions

File tree

vertx-mssql-client/src/test/java/io/vertx/tests/mssqlclient/MSSQLQueriesTest.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040
import java.util.stream.Collectors;
4141
import java.util.stream.Stream;
4242

43-
import static org.hamcrest.CoreMatchers.instanceOf;
44-
import static org.hamcrest.CoreMatchers.is;
43+
import static org.hamcrest.CoreMatchers.*;
4544
import static org.hamcrest.MatcherAssert.assertThat;
4645

4746
@RunWith(VertxUnitRunner.class)
@@ -206,4 +205,22 @@ public void testQuerySequences(TestContext ctx) {
206205
}));
207206
}));
208207
}
208+
209+
@Test
210+
public void testUnsupportedXmlTypeErrorPropagation(TestContext ctx) {
211+
connection
212+
.query("CREATE TABLE #TempXmlTest (id INT, xmlData XML)")
213+
.execute()
214+
.onComplete(ctx.asyncAssertSuccess(create -> {
215+
connection
216+
.query("SELECT * FROM #TempXmlTest")
217+
.execute()
218+
.onComplete(ctx.asyncAssertFailure(t -> {
219+
ctx.verify(v -> {
220+
assertThat(t, instanceOf(UnsupportedOperationException.class));
221+
assertThat(t.getMessage(), containsString("XML"));
222+
});
223+
}));
224+
}));
225+
}
209226
}

vertx-pg-client/src/main/java/io/vertx/pgclient/impl/codec/InitPgCommandMessage.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@
1616
*/
1717
package io.vertx.pgclient.impl.codec;
1818

19-
import java.nio.charset.Charset;
20-
import java.nio.charset.StandardCharsets;
21-
2219
import io.netty.buffer.ByteBuf;
2320
import io.vertx.core.VertxException;
2421
import io.vertx.pgclient.impl.PgDatabaseMetadata;
2522
import io.vertx.pgclient.impl.PgSocketConnection;
2623
import io.vertx.pgclient.impl.auth.scram.ScramAuthentication;
2724
import io.vertx.pgclient.impl.auth.scram.ScramSession;
28-
import io.vertx.sqlclient.spi.connection.Connection;
2925
import io.vertx.sqlclient.codec.CommandResponse;
26+
import io.vertx.sqlclient.spi.connection.Connection;
3027
import io.vertx.sqlclient.spi.protocol.InitCommand;
3128

29+
import java.nio.charset.Charset;
30+
import java.nio.charset.StandardCharsets;
31+
3232
class InitPgCommandMessage extends PgCommandMessage<Connection, InitCommand> {
3333

3434
private PgEncoder encoder;
@@ -66,9 +66,17 @@ void handleAuthenticationSasl(ByteBuf in) {
6666
}
6767
PgSocketConnection pgSocketConn = (PgSocketConnection) cmd.connection().unwrap();
6868
scramSession = scramAuth.session(cmd.username(), cmd.password().toCharArray(), pgSocketConn.channelBinding());
69-
encoder.writeScramClientInitialMessage(
69+
try {
70+
encoder.writeScramClientInitialMessage(
7071
scramSession.createInitialSaslMessage(in, encoder.channelHandlerContext()));
71-
encoder.flush();
72+
encoder.flush();
73+
} catch (RuntimeException e) {
74+
decoder.fireCommandResponse(CommandResponse.failure(e));
75+
// If the frontend does not support the authentication method requested by the server,
76+
// then it should immediately close the connection.
77+
// See https://www.postgresql.org/docs/current/protocol-flow.html
78+
encoder.close();
79+
}
7280
}
7381

7482
@Override

vertx-pg-client/src/test/java/io/vertx/tests/pgclient/PgConnectionTest.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import io.vertx.ext.unit.Async;
2121
import io.vertx.ext.unit.TestContext;
2222
import io.vertx.pgclient.PgConnection;
23-
import io.vertx.sqlclient.ClosedConnectionException;
23+
import io.vertx.pgclient.PgException;
2424
import io.vertx.sqlclient.Row;
2525
import io.vertx.sqlclient.RowSet;
2626
import io.vertx.sqlclient.Tuple;
@@ -148,7 +148,12 @@ public void testInflightCommandsFailWhenConnectionClosed(TestContext ctx) {
148148
.query("SELECT pg_sleep(20)")
149149
.execute()
150150
.onComplete(ctx.asyncAssertFailure(t -> {
151-
ctx.assertTrue(t instanceof ClosedConnectionException);
151+
if (t instanceof PgException) {
152+
PgException pge = (PgException) t;
153+
ctx.assertEquals("57P01", pge.getSqlState()); // ADMIN SHUTDOWN
154+
} else {
155+
ctx.fail(t);
156+
}
152157
}));
153158
connector.accept(ctx.asyncAssertSuccess(conn2 -> {
154159
conn2

vertx-pg-client/src/test/java/io/vertx/tests/pgclient/TLSTest.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package io.vertx.tests.pgclient;
1313

14+
import com.ongres.scram.client.ChannelBindingException;
1415
import io.vertx.core.Vertx;
1516
import io.vertx.core.VertxOptions;
1617
import io.vertx.core.buffer.Buffer;
@@ -20,12 +21,7 @@
2021
import io.vertx.ext.unit.Async;
2122
import io.vertx.ext.unit.TestContext;
2223
import io.vertx.ext.unit.junit.VertxUnitRunner;
23-
import io.vertx.pgclient.ChannelBinding;
24-
import io.vertx.pgclient.PgConnectOptions;
25-
import io.vertx.pgclient.PgConnection;
26-
import io.vertx.pgclient.SslMode;
27-
import io.vertx.pgclient.SslNegotiation;
28-
import io.vertx.sqlclient.ClosedConnectionException;
24+
import io.vertx.pgclient.*;
2925
import io.vertx.sqlclient.Tuple;
3026
import io.vertx.tests.pgclient.junit.ContainerPgRule;
3127
import org.junit.*;
@@ -281,14 +277,12 @@ public void testChannelBindingRequireWithSsl(TestContext ctx) {
281277
@Test
282278
public void testChannelBindingRequireWithoutSsl(TestContext ctx) {
283279
Async async = ctx.async();
284-
PgConnectOptions options = new PgConnectOptions(ruleOptionalSll.options())
285-
.setSslMode(SslMode.DISABLE)
280+
PgConnectOptions options = new PgConnectOptions(ruleSllOff.options())
286281
.setChannelBinding(ChannelBinding.REQUIRE)
287282
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
288283

289284
PgConnection.connect(vertx, options).onComplete(ctx.asyncAssertFailure(err -> {
290-
// ctx.assertEquals("Channel bindins is required", err.getMessage());
291-
ctx.assertTrue(err instanceof ClosedConnectionException); // TODO: handle ChannelBindingException
285+
ctx.assertTrue(err instanceof ChannelBindingException);
292286
async.complete();
293287
}));
294288
}

vertx-pg-client/src/test/java/module-info.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
requires io.netty.buffer;
44
requires io.netty.transport;
5+
requires com.ongres.scram.client;
56
requires io.vertx.core;
67
requires io.vertx.sql.client;
78
requires io.vertx.sql.client.pg;

vertx-sql-client-codec/src/main/java/io/vertx/sqlclient/codec/SocketConnectionBase.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,25 @@
2121
import io.netty.channel.ChannelFutureListener;
2222
import io.netty.channel.ChannelHandlerContext;
2323
import io.netty.handler.codec.DecoderException;
24-
import io.vertx.core.*;
24+
import io.vertx.core.Completable;
25+
import io.vertx.core.Vertx;
26+
import io.vertx.core.VertxException;
27+
import io.vertx.core.internal.ContextInternal;
2528
import io.vertx.core.internal.logging.Logger;
2629
import io.vertx.core.internal.logging.LoggerFactory;
30+
import io.vertx.core.internal.net.NetSocketInternal;
2731
import io.vertx.core.net.SocketAddress;
2832
import io.vertx.core.spi.metrics.ClientMetrics;
2933
import io.vertx.core.tracing.TracingPolicy;
30-
import io.vertx.core.internal.ContextInternal;
31-
import io.vertx.core.internal.net.NetSocketInternal;
3234
import io.vertx.sqlclient.ClosedConnectionException;
3335
import io.vertx.sqlclient.Row;
3436
import io.vertx.sqlclient.SqlConnectOptions;
3537
import io.vertx.sqlclient.codec.impl.PreparedStatementCache;
36-
import io.vertx.sqlclient.spi.connection.Connection;
3738
import io.vertx.sqlclient.internal.PreparedStatement;
3839
import io.vertx.sqlclient.spi.DatabaseMetadata;
40+
import io.vertx.sqlclient.spi.connection.Connection;
3941
import io.vertx.sqlclient.spi.connection.ConnectionContext;
40-
import io.vertx.sqlclient.spi.protocol.CloseConnectionCommand;
41-
import io.vertx.sqlclient.spi.protocol.CloseStatementCommand;
42-
import io.vertx.sqlclient.spi.protocol.CommandBase;
43-
import io.vertx.sqlclient.spi.protocol.CompositeCommand;
44-
import io.vertx.sqlclient.spi.protocol.ExtendedQueryCommand;
45-
import io.vertx.sqlclient.spi.protocol.PrepareStatementCommand;
42+
import io.vertx.sqlclient.spi.protocol.*;
4643

4744
import java.util.ArrayDeque;
4845
import java.util.List;
@@ -437,11 +434,12 @@ private void handleClose(Throwable t) {
437434
if (t != null) {
438435
reportException(t);
439436
}
437+
Throwable inflightCause = t != null ? t : ClosedConnectionException.INSTANCE;
440438
CommandMessage<?, ?> msg;
441439
while ((msg = inflights.poll()) != null) {
442-
fail(msg, ClosedConnectionException.INSTANCE);
440+
fail(msg, inflightCause);
443441
}
444-
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : new VertxException(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
442+
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
445443
CommandBase<?> cmd;
446444
while ((cmd = pending.poll()) != null) {
447445
CommandBase c = cmd;

0 commit comments

Comments
 (0)