Skip to content

Commit c0a134b

Browse files
authored
IGNITE-28110 Fix buffer leak in ClientInboundMessageHandler on invalid message (#7733)
1 parent d6d7b42 commit c0a134b

2 files changed

Lines changed: 76 additions & 16 deletions

File tree

modules/client-handler/src/integrationTest/java/org/apache/ignite/client/handler/ItClientHandlerTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,60 @@ public void testServerReturnsAllItsFeatures() throws IOException {
580580
}
581581
}
582582

583+
@Test
584+
void testInvalidHandshakeStateDropsConnection() throws Exception {
585+
try (var sock = new Socket("127.0.0.1", serverPort)) {
586+
OutputStream out = sock.getOutputStream();
587+
588+
// Magic: IGNI
589+
out.write(MAGIC);
590+
591+
// Send first handshake.
592+
try (var packer1 = MessagePack.newDefaultBufferPacker()) {
593+
packer1.packInt(0);
594+
packer1.packInt(0);
595+
packer1.packInt(0);
596+
packer1.packInt(7); // Size.
597+
598+
packer1.packInt(3); // Major
599+
packer1.packInt(0); // Minor
600+
packer1.packInt(0); // Patch
601+
602+
packer1.packInt(2); // Client type: general purpose.
603+
604+
packer1.packBinaryHeader(0); // Features.
605+
packer1.packInt(0); // Extensions.
606+
607+
out.write(packer1.toByteArray());
608+
}
609+
610+
// Second message before handshake completes.
611+
// This should trigger "Unexpected message received before handshake completion"
612+
try (var packer2 = MessagePack.newDefaultBufferPacker()) {
613+
packer2.packInt(0);
614+
packer2.packInt(0);
615+
packer2.packInt(0);
616+
packer2.packInt(7); // Size.
617+
618+
packer2.packInt(3); // Major
619+
packer2.packInt(0); // Minor
620+
packer2.packInt(0); // Patch
621+
622+
packer2.packInt(2); // Client type: general purpose.
623+
624+
packer2.packBinaryHeader(0); // Features.
625+
packer2.packInt(0); // Extensions.
626+
627+
out.write(packer2.toByteArray());
628+
}
629+
630+
out.flush();
631+
632+
// Server drops the connection due to invalid message.
633+
assertThrows(IOException.class, () -> writeAndFlushLoop(sock));
634+
}
635+
}
636+
583637
private static void writeAndFlushLoop(Socket socket) throws Exception {
584638
var stop = System.currentTimeMillis() + 5000;
585639
var out = socket.getOutputStream();

modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -421,28 +421,34 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
421421

422422
// Each inbound handler in a pipeline has to release the received messages.
423423
var unpacker = new ClientMessageUnpacker(byteBuf);
424-
metrics.bytesReceivedAdd(byteBuf.readableBytes() + ClientMessageCommon.HEADER_SIZE);
425424

426-
switch (state) {
427-
case STATE_BEFORE_HANDSHAKE:
428-
state = STATE_HANDSHAKE_REQUESTED;
429-
metrics.bytesReceivedAdd(ClientMessageCommon.MAGIC_BYTES.length);
430-
handshake(ctx, unpacker);
425+
try {
426+
metrics.bytesReceivedAdd(byteBuf.readableBytes() + ClientMessageCommon.HEADER_SIZE);
431427

432-
break;
428+
switch (state) {
429+
case STATE_BEFORE_HANDSHAKE:
430+
state = STATE_HANDSHAKE_REQUESTED;
431+
metrics.bytesReceivedAdd(ClientMessageCommon.MAGIC_BYTES.length);
432+
handshake(ctx, unpacker);
433433

434-
case STATE_HANDSHAKE_REQUESTED:
435-
// Handshake is in progress, any messages are not allowed.
436-
throw new IgniteException(PROTOCOL_ERR, "Unexpected message received before handshake completion");
434+
break;
437435

438-
case STATE_HANDSHAKE_RESPONSE_SENT:
439-
assert clientContext != null : "Client context != null";
440-
processOperation(ctx, unpacker);
436+
case STATE_HANDSHAKE_REQUESTED:
437+
// Handshake is in progress, any messages are not allowed.
438+
throw new IgniteException(PROTOCOL_ERR, "Unexpected message received before handshake completion");
441439

442-
break;
440+
case STATE_HANDSHAKE_RESPONSE_SENT:
441+
assert clientContext != null : "Client context != null";
442+
processOperation(ctx, unpacker);
443443

444-
default:
445-
throw new IllegalStateException("Unexpected state: " + state);
444+
break;
445+
446+
default:
447+
throw new IllegalStateException("Unexpected state: " + state);
448+
}
449+
} catch (Throwable t) {
450+
unpacker.close();
451+
throw t;
446452
}
447453
}
448454

0 commit comments

Comments
 (0)