Skip to content

Commit 50ead96

Browse files
authored
netty: Preserve early server handshake failure cause in logs
Early server-side negotiation failures may terminate a transport before NettyServerHandler is fully active in the pipeline. In those cases, the original handshake failure can be missing from transport termination logging because termination may rely on connectionError(), which can be null on this early path. This change adds a server-side NOOP write in NettyServerTransport.start() (analogous to the existing client-side NOOP write path). If that write fails, its cause is passed to notifyTerminated(), preserving and logging the original transport termination reason for debugging. To support this, NettyServerHandler now accepts NOOP_MESSAGE writes by writing an empty buffer, and tests are added to verify: - transport failure logging for plaintext-client to TLS-server failure - server NOOP write handling in NettyServerHandler Fixes #8495
1 parent eae16b2 commit 50ead96

File tree

4 files changed

+76
-0
lines changed

4 files changed

+76
-0
lines changed

netty/src/main/java/io/grpc/netty/NettyServerHandler.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@ class NettyServerHandler extends AbstractNettyHandler {
124124
private static final boolean DISABLE_CONNECTION_HEADER_CHECK = Boolean.parseBoolean(
125125
System.getProperty("io.grpc.netty.disableConnectionHeaderCheck", "false"));
126126

127+
/**
128+
* A message that simply passes through the channel without any real processing. It is useful to
129+
* check if buffers have been drained and test the health of the channel in a single operation.
130+
*/
131+
static final Object NOOP_MESSAGE = new Object();
132+
127133
private final Http2Connection.PropertyKey streamKey;
128134
private final ServerTransportListener transportListener;
129135
private final int maxMessageSize;
@@ -709,6 +715,8 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
709715
gracefulClose(ctx, (GracefulServerCloseCommand) msg, promise);
710716
} else if (msg instanceof ForcefulCloseCommand) {
711717
forcefulClose(ctx, (ForcefulCloseCommand) msg, promise);
718+
} else if (msg == NOOP_MESSAGE) {
719+
ctx.write(Unpooled.EMPTY_BUFFER, promise);
712720
} else {
713721
AssertionError e =
714722
new AssertionError("Write called for unexpected type: " + msg.getClass().getName());

netty/src/main/java/io/grpc/netty/NettyServerTransport.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,18 @@ public void operationComplete(ChannelFuture future) throws Exception {
160160
channel.closeFuture().addListener(terminationNotifier);
161161

162162
channel.pipeline().addLast(bufferingHandler);
163+
164+
channel.writeAndFlush(NettyServerHandler.NOOP_MESSAGE).addListener(new ChannelFutureListener() {
165+
@Override
166+
public void operationComplete(ChannelFuture future) throws Exception {
167+
if (!future.isSuccess()) {
168+
// grpcHandler (NettyServerHandler) may not be in the pipeline yet on early negotiation
169+
// failure, so connectionError() can remain null. Notify termination here with the write
170+
// failure cause to preserve/log the original transport termination reason.
171+
notifyTerminated(future.cause());
172+
}
173+
}
174+
});
163175
}
164176

165177
@Override

netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,25 @@ public void maxRstCountSent_exceedsLimit_fails() throws Exception {
13471347
assertFalse(channel().isOpen());
13481348
}
13491349

1350+
@Test
1351+
public void write_noopMessage_writesEmptyBuffer() throws Exception {
1352+
manualSetUp();
1353+
1354+
ChannelPromise promise = newPromise();
1355+
handler().write(ctx(), NettyServerHandler.NOOP_MESSAGE, promise);
1356+
channel().flush();
1357+
1358+
assertTrue(promise.isSuccess());
1359+
1360+
Object outbound = channel().readOutbound();
1361+
assertTrue(outbound instanceof ByteBuf);
1362+
ByteBuf buf = (ByteBuf) outbound;
1363+
assertEquals(0, buf.readableBytes());
1364+
buf.release();
1365+
1366+
assertNull(channel().readOutbound());
1367+
}
1368+
13501369
private void madeYouReset(int burstSize) throws Exception {
13511370
when(streamTracerFactory.newServerStreamTracer(anyString(), any(Metadata.class)))
13521371
.thenAnswer((args) -> new TestServerStreamTracer());

netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
import java.util.concurrent.TimeUnit;
134134
import java.util.concurrent.atomic.AtomicReference;
135135
import java.util.logging.Filter;
136+
import java.util.logging.Handler;
136137
import java.util.logging.Level;
137138
import java.util.logging.LogRecord;
138139
import java.util.logging.Logger;
@@ -472,6 +473,42 @@ public void from_tls_clientAuthRequire_noClientCert() throws Exception {
472473
}
473474
}
474475

476+
@Test
477+
public void from_plaintextClient_toTlsServer_logsTransportFailureCause() throws Exception {
478+
final AtomicReference<LogRecord> transportFailureLog = new AtomicReference<>();
479+
Handler handler = new Handler() {
480+
@Override
481+
public void publish(LogRecord record) {
482+
if ("Transport failed".equals(record.getMessage()) && record.getThrown() != null) {
483+
transportFailureLog.compareAndSet(null, record);
484+
}
485+
}
486+
487+
@Override
488+
public void flush() {}
489+
490+
@Override
491+
public void close() throws SecurityException {}
492+
};
493+
Logger logger =
494+
Logger.getLogger(String.format("%s.connections", NettyServerTransport.class.getName()));
495+
Level oldLevel = logger.getLevel();
496+
try {
497+
logger.addHandler(handler);
498+
logger.setLevel(Level.ALL);
499+
500+
ServerCredentials serverCreds = TlsServerCredentials.create(server1Cert, server1Key);
501+
ChannelCredentials channelCreds = InsecureChannelCredentials.create();
502+
Status status = expectFailedHandshake(channelCreds, serverCreds);
503+
504+
assertEquals(Status.Code.UNAVAILABLE, status.getCode());
505+
assertThat(transportFailureLog.get()).isNotNull();
506+
} finally {
507+
logger.removeHandler(handler);
508+
logger.setLevel(oldLevel);
509+
}
510+
}
511+
475512
@Test
476513
public void from_tls_clientAuthRequire_clientCert() throws Exception {
477514
ServerCredentials serverCreds = TlsServerCredentials.newBuilder()

0 commit comments

Comments
 (0)