From 2c5e10459efe411677a16ee7fe3a3c69473df354 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 21 Nov 2023 14:42:25 +0100 Subject: [PATCH] Remove QuicConnectEvent (#618) Motivation: How we used QuicConnectEvent to notify about connection migration was not correct at all. Let's remove it for now as we will introduce something better once https://github.com/cloudflare/quiche/pull/1660 was merged into quiche. Modifications: Remove QuicConnectEvent Result: Get rid of incorrect implementation to prepare for adding a proper one --- .../codec/quic/QuicConnectionEvent.java | 58 ------------------- .../codec/quic/QuicheQuicChannel.java | 16 ----- .../codec/quic/QuicheQuicServerCodec.java | 1 - .../codec/quic/QuicChannelConnectTest.java | 12 ---- .../quic/QuicChannelValidationHandler.java | 10 ---- 5 files changed, 97 deletions(-) delete mode 100644 codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicConnectionEvent.java diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicConnectionEvent.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicConnectionEvent.java deleted file mode 100644 index 85c041b39..000000000 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicConnectionEvent.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2021 The Netty Project - * - * The Netty Project licenses this file to you under the Apache License, - * version 2.0 (the "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at: - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ -package io.netty.incubator.codec.quic; - -import java.net.SocketAddress; - -/** - * {@link QuicEvent} which is fired when an QUIC connection creation or migration was detected. - */ -public final class QuicConnectionEvent implements QuicEvent { - - private final SocketAddress oldAddress; - private final SocketAddress newAddress; - - QuicConnectionEvent(SocketAddress oldAddress, SocketAddress newAddress) { - this.oldAddress = oldAddress; - this.newAddress = newAddress; - } - - /** - * The old {@link SocketAddress} of the connection or {@code null} if the connection was just created. - * - * @return the old {@link SocketAddress} of the connection. - */ - public SocketAddress oldAddress() { - return oldAddress; - } - - /** - * The new {@link SocketAddress} of the connection. - * - * @return the new {@link SocketAddress} of the connection. - */ - public SocketAddress newAddress() { - return newAddress; - } - - @Override - public String toString() { - return "QuicConnectionEvent{" + - "oldAddress=" + oldAddress + - ", newAddress=" + newAddress + - '}'; - } -} diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java index 2f16586b4..8d9d16fc4 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java @@ -1077,8 +1077,6 @@ private boolean connectionSendSegments(SegmentedDatagramPacketAllocator segmente remote = QuicheSendInfo.getToAddress(sendInfo); local = QuicheSendInfo.getFromAddress(sendInfo); - fireConnectionEventIfNeeded(oldRemote, remote); - if (size > 0) { // We have something in the out list already, we need to send this now and so we set the // segmentSize. @@ -1136,16 +1134,6 @@ private boolean connectionSendSegments(SegmentedDatagramPacketAllocator segmente } } - private void fireConnectionEventIfNeeded(SocketAddress oldRemote, SocketAddress newRemote) { - // Check if the remote address changed and only in this case fire an event. - // This is required as even tho we check the SendInfo / RecvInfo without this extra check we might - // notify two times (one time in the recv and one time in the send code-path). - if (!oldRemote.equals(newRemote)) { - pipeline().fireUserEventTriggered( - new QuicConnectionEvent(oldRemote, newRemote)); - } - } - private static int segmentSize(List bufferList) { assert !bufferList.isEmpty(); int size = bufferList.size(); @@ -1189,11 +1177,8 @@ private boolean connectionSendSimple() { } if (connection.isSendInfoChanged()) { // Change the cached address - InetSocketAddress oldRemote = remote; remote = QuicheSendInfo.getToAddress(sendInfo); local = QuicheSendInfo.getFromAddress(sendInfo); - - fireConnectionEventIfNeeded(oldRemote, remote); } out.writerIndex(writerIndex + written); boolean stop = writePacket(new DatagramPacket(out, remote), maxDatagramSize, len); @@ -1416,7 +1401,6 @@ void connectionRecv(InetSocketAddress sender, InetSocketAddress recipient, ByteB if (connection.isRecvInfoChanged()) { // Update the cached address remote = sender; - fireConnectionEventIfNeeded(oldRemote, sender); } local = recipient; diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java index b0d3d58f7..3099b87b4 100644 --- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java +++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java @@ -246,7 +246,6 @@ private QuicheQuicChannel handleServer(ChannelHandlerContext ctx, InetSocketAddr putChannel(channel); ctx.channel().eventLoop().register(channel); - channel.pipeline().fireUserEventTriggered(new QuicConnectionEvent(null, sender)); return channel; } } diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java index 1bcf8dc50..179b11da0 100644 --- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java +++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java @@ -520,7 +520,6 @@ public void testConnectWithoutTokenValidation(Executor executor) throws Throwabl @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS) public void testConnectAndGetAddressesAfterClose(Executor executor) throws Throwable { AtomicReference acceptedRef = new AtomicReference<>(); - AtomicReference serverEventRef = new AtomicReference<>(); Channel server = QuicTestUtils.newServer(executor, new ChannelInboundHandlerAdapter() { @Override @@ -528,14 +527,6 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception { acceptedRef.set((QuicChannel) ctx.channel()); super.channelActive(ctx); } - - @Override - public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { - if (evt instanceof QuicConnectionEvent) { - serverEventRef.set((QuicConnectionEvent) evt); - } - super.userEventTriggered(ctx, evt); - } }, new ChannelInboundHandlerAdapter()); InetSocketAddress address = (InetSocketAddress) server.localAddress(); @@ -555,9 +546,6 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc assertNotNull(quicChannel.localAddress()); assertNotNull(quicChannel.remoteAddress()); - assertNull(serverEventRef.get().oldAddress()); - assertEquals(channel.localAddress(), serverEventRef.get().newAddress()); - QuicChannel accepted; while ((accepted = acceptedRef.get()) == null) { Thread.sleep(50); diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelValidationHandler.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelValidationHandler.java index f29e42012..7cdb7e6f5 100644 --- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelValidationHandler.java +++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicChannelValidationHandler.java @@ -18,20 +18,10 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; -import static org.junit.jupiter.api.Assertions.fail; - class QuicChannelValidationHandler extends ChannelInboundHandlerAdapter { private volatile Throwable cause; - @Override - public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { - if (evt instanceof QuicConnectionEvent && ((QuicConnectionEvent) evt).oldAddress() != null) { - fail("QuicConnectionEvent indication migration should never happen atm"); - } - super.userEventTriggered(ctx, evt); - } - @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { this.cause = cause;