Skip to content

Commit 0533db5

Browse files
committed
fix(quic): close inbound connection when handler rejects it (#499)
The inbound QUIC server path invokes the application connection handler synchronously inside channelActive. When the handler rejects the connection by throwing (e.g. PeerAlreadyConnectedException for a duplicate/simultaneous peer), the handshake-waiter handler has already removed itself, so the exception reaches the Netty pipeline tail and logs a noisy 'exceptionCaught reached tail of pipeline' warning while leaking the rejected QuicChannel until idle timeout. Catch the exception in routeInboundHandshake and close the channel, mirroring the dial paths which already close on handler failure. Narrow the guard to catch(Exception) rather than Throwable so fatal Errors (OutOfMemoryError, LinkageError, ...) propagate instead of being downgraded to a routine connection close; logging stays at debug.
1 parent f51cee8 commit 0533db5

2 files changed

Lines changed: 72 additions & 2 deletions

File tree

libp2p/src/main/kotlin/io/libp2p/transport/quic/QuicTransport.kt

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,24 @@ class QuicTransport(
131131
// Route this validated inbound connection to the waiting hole-punch caller.
132132
pending.future.complete(holePunchChannel())
133133
} else {
134-
// Normal path: deliver to the connection handler.
135-
exposeConnection()
134+
// Normal path: deliver to the connection handler. The application handler may reject
135+
// the connection by throwing (e.g. PeerAlreadyConnectedException for a duplicate or
136+
// simultaneous connection to an already-connected peer). This runs inside the server
137+
// pipeline's channelActive, and the handshake-waiter handler has already removed itself
138+
// by this point, so an escaping exception would reach the Netty pipeline tail and log a
139+
// noisy "exceptionCaught reached tail of pipeline" warning while leaking the channel.
140+
// Close the rejected connection instead, mirroring the dial paths. Only Exception is
141+
// caught — fatal Errors (OutOfMemoryError, LinkageError, ...) must propagate rather than
142+
// be downgraded to a routine rejection. The library cannot tell a deliberate rejection
143+
// from an application bug (the handler contract has no typed rejection), so the log stays
144+
// at debug to avoid reintroducing the noise this guard removes; the application owns
145+
// logging of its own connection decisions.
146+
try {
147+
exposeConnection()
148+
} catch (e: Exception) {
149+
logger.debug("Inbound connection rejected by handler; closing", e)
150+
closeChannel()
151+
}
136152
}
137153
}
138154

libp2p/src/test/kotlin/io/libp2p/transport/quic/QuicInboundHandshakeRoutingTest.kt

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import io.libp2p.security.tls.TlsPeerIdentity
66
import io.mockk.mockk
77
import io.netty.handler.codec.quic.QuicChannel
88
import org.assertj.core.api.Assertions.assertThat
9+
import org.assertj.core.api.Assertions.assertThatCode
10+
import org.assertj.core.api.Assertions.assertThatThrownBy
911
import org.junit.jupiter.api.Test
1012
import java.util.concurrent.CompletableFuture
1113

@@ -107,4 +109,56 @@ class QuicInboundHandshakeRoutingTest {
107109
assertThat(exposed).isTrue()
108110
assertThat(closed).isFalse()
109111
}
112+
113+
@Test
114+
fun `closes the connection when the handler rejects a normal inbound connection`() {
115+
val transport = transport()
116+
val inbound = identity()
117+
118+
var closed = false
119+
120+
// The application connection handler may reject an inbound connection by throwing
121+
// (e.g. PeerAlreadyConnectedException for a duplicate/simultaneous connection). That
122+
// exception must NOT escape routeInboundHandshake: in the live server pipeline the
123+
// handshake-waiter handler has already removed itself by the time the connection is
124+
// exposed, so an escaping exception reaches the Netty pipeline tail and logs a noisy
125+
// "exceptionCaught reached tail of pipeline" warning. Instead the rejected connection
126+
// must be closed, mirroring the dial paths.
127+
assertThatCode {
128+
transport.routeInboundHandshake(
129+
remoteIdentity = inbound,
130+
pending = null,
131+
prepareConnection = { },
132+
closeChannel = { closed = true },
133+
holePunchChannel = { error("no hole punch is pending") },
134+
exposeConnection = { throw RuntimeException("Already connected to peer") }
135+
)
136+
}.doesNotThrowAnyException()
137+
138+
assertThat(closed).isTrue()
139+
}
140+
141+
@Test
142+
fun `does not swallow a fatal Error thrown by the handler`() {
143+
val transport = transport()
144+
val inbound = identity()
145+
146+
var closed = false
147+
148+
// A fatal Error (OutOfMemoryError, LinkageError, ...) is NOT a routine connection rejection
149+
// and must propagate rather than be silently downgraded to a closed connection. Only
150+
// Exception is caught by routeInboundHandshake.
151+
assertThatThrownBy {
152+
transport.routeInboundHandshake(
153+
remoteIdentity = inbound,
154+
pending = null,
155+
prepareConnection = { },
156+
closeChannel = { closed = true },
157+
holePunchChannel = { error("no hole punch is pending") },
158+
exposeConnection = { throw OutOfMemoryError("simulated fatal error") }
159+
)
160+
}.isInstanceOf(OutOfMemoryError::class.java)
161+
162+
assertThat(closed).isFalse()
163+
}
110164
}

0 commit comments

Comments
 (0)