Moved some common data handling logic to the common module

bungee/src/main/java/org/geysermc/floodgate/addon/data/BungeeDataAddon.java

@@ -79,7 +79,7 @@ public class BungeeDataAddon implements InjectorAddon {
 
         channel.pipeline().addBefore(
                 packetHandler, "floodgate_data_handler",
-                new BungeeProxyDataHandler(config, handshakeHandler, blocker, kickMessageAttribute)
+                new BungeeProxyDataHandler(handshakeHandler, config, kickMessageAttribute, blocker)
         );
     }
 

bungee/src/main/java/org/geysermc/floodgate/addon/data/BungeeProxyDataHandler.java

@@ -27,30 +27,22 @@ package org.geysermc.floodgate.addon.data;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import com.google.common.collect.Queues;
+import io.netty.channel.Channel;
-import io.netty.channel.ChannelHandlerContext;
-import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.util.AttributeKey;
 import java.lang.reflect.Field;
 import java.net.InetSocketAddress;
-import java.util.Queue;
-import java.util.concurrent.CompletableFuture;
-import lombok.RequiredArgsConstructor;
 import net.md_5.bungee.connection.InitialHandler;
 import net.md_5.bungee.netty.ChannelWrapper;
 import net.md_5.bungee.netty.HandlerBoss;
 import net.md_5.bungee.protocol.DefinedPacket;
 import net.md_5.bungee.protocol.PacketWrapper;
 import net.md_5.bungee.protocol.packet.Handshake;
-import org.geysermc.floodgate.api.handshake.HandshakeData;
 import org.geysermc.floodgate.config.ProxyFloodgateConfig;
 import org.geysermc.floodgate.player.FloodgateHandshakeHandler;
-import org.geysermc.floodgate.util.Constants;
 import org.geysermc.floodgate.util.ReflectionUtils;
 
 @SuppressWarnings("ConstantConditions")
-@RequiredArgsConstructor
+public class BungeeProxyDataHandler extends CommonDataHandler {
-public class BungeeProxyDataHandler extends ChannelInboundHandlerAdapter {
     private static final Field HANDLER;
     private static final Field CHANNEL_WRAPPER;
 
@@ -63,100 +55,45 @@ public class BungeeProxyDataHandler extends ChannelInboundHandlerAdapter {
         checkNotNull(CHANNEL_WRAPPER, "ChannelWrapper field cannot be null");
     }
 
-    private final ProxyFloodgateConfig config;
+    public BungeeProxyDataHandler(
-    private final FloodgateHandshakeHandler handler;
+            FloodgateHandshakeHandler handshakeHandler,
-    private final PacketBlocker blocker;
+            ProxyFloodgateConfig config,
-    private final AttributeKey<String> kickMessageAttribute;
+            AttributeKey<String> kickMessageAttribute,
-
+            PacketBlocker blocker) {
-    private final Queue<Object> packetQueue = Queues.newConcurrentLinkedQueue();
+        super(handshakeHandler, config, kickMessageAttribute, blocker);
+    }
 
     @Override
-    public void channelRead(ChannelHandlerContext ctx, Object msg) {
+    protected void setNewIp(Channel channel, InetSocketAddress newIp) {
-        // prevent other packets from being handled while we handle the handshake packet
+        HandlerBoss handlerBoss = ctx.pipeline().get(HandlerBoss.class);
-        if (!packetQueue.isEmpty()) {
+        // InitialHandler extends PacketHandler and implements PendingConnection
-            packetQueue.add(msg);
+        InitialHandler connection = ReflectionUtils.getCastedValue(handlerBoss, HANDLER);
-            return;
-        }
 
+        ChannelWrapper channelWrapper = ReflectionUtils.getCastedValue(connection, CHANNEL_WRAPPER);
+        channelWrapper.setRemoteAddress(newIp);
+    }
+
+    @Override
+    protected void setHostname(Object wrapperWithHandshake, String hostname) {
+        PacketWrapper wrapper = (PacketWrapper) wrapperWithHandshake;
+        Handshake handshake = (Handshake) wrapper.packet;
+        handshake.setHost(hostname);
+    }
+
+    @Override
+    public boolean channelRead(Object msg) {
         if (msg instanceof PacketWrapper) {
             DefinedPacket packet = ((PacketWrapper) msg).packet;
 
             // we're only interested in the Handshake packet
             if (packet instanceof Handshake) {
-                blocker.enable();
+                handle(msg, ((Handshake) packet).getHost());
-                packetQueue.add(msg);
 
-                handleHandshake(ctx, (Handshake) packet).thenRun(() -> {
+                // otherwise, it'll get read twice. once by the packet queue and once by this method
-                    Object queuedPacket;
+                return false;
-                    while ((queuedPacket = packetQueue.poll()) != null) {
-                        ctx.fireChannelRead(queuedPacket);
-                    }
-                    ctx.pipeline().remove(this);
-                    blocker.disable();
-                });
-                return;
             }
         }
 
-        ctx.fireChannelRead(msg);
+        return true;
-    }
-
-    private CompletableFuture<Void> handleHandshake(ChannelHandlerContext ctx, Handshake packet) {
-        String data = packet.getHost();
-
-        return handler.handle(ctx.channel(), data).thenAccept(result -> {
-            HandshakeData handshakeData = result.getHandshakeData();
-
-            // we'll change the IP address from the proxy to the real IP of the client very early on
-            // so that almost every plugin will use the real IP of the client
-            InetSocketAddress newIp = result.getNewIp(ctx.channel());
-            if (newIp != null) {
-                HandlerBoss handlerBoss = ctx.pipeline().get(HandlerBoss.class);
-                // InitialHandler extends PacketHandler and implements PendingConnection
-                InitialHandler connection = ReflectionUtils.getCastedValue(handlerBoss, HANDLER);
-
-                ChannelWrapper channelWrapper =
-                        ReflectionUtils.getCastedValue(connection, CHANNEL_WRAPPER);
-
-                channelWrapper.setRemoteAddress(newIp);
-            }
-
-            packet.setHost(handshakeData.getHostname());
-
-            if (handshakeData.getDisconnectReason() != null) {
-                ctx.channel().attr(kickMessageAttribute).set(handshakeData.getDisconnectReason());
-                return;
-            }
-
-            switch (result.getResultType()) {
-                case EXCEPTION:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(Constants.INTERNAL_ERROR_MESSAGE);
-                    break;
-                case DECRYPT_ERROR:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(config.getDisconnect().getInvalidKey());
-                    break;
-                case INVALID_DATA_LENGTH:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(config.getDisconnect().getInvalidArgumentsLength());
-                    break;
-                default:
-                    break;
-            }
-        }).handle((v, error) -> {
-            if (error != null) {
-                error.printStackTrace();
-            }
-            return v;
-        });
-    }
-
-    @Override
-    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-        super.exceptionCaught(ctx, cause);
-        if (config.isDebug()) {
-            cause.printStackTrace();
-        }
     }
 }

common/src/main/java/org/geysermc/floodgate/addon/data/CommonDataHandler.java

@@ -0,0 +1,179 @@
+/*
+ * Copyright (c) 2019-2021 GeyserMC. http://geysermc.org
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author GeyserMC
+ * @link https://github.com/GeyserMC/Floodgate
+ */
+
+package org.geysermc.floodgate.addon.data;
+
+import com.google.common.collect.Queues;
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.util.AttributeKey;
+import it.unimi.dsi.fastutil.Pair;
+import java.net.InetSocketAddress;
+import java.util.Queue;
+import lombok.RequiredArgsConstructor;
+import org.geysermc.floodgate.api.handshake.HandshakeData;
+import org.geysermc.floodgate.config.FloodgateConfig;
+import org.geysermc.floodgate.player.FloodgateHandshakeHandler;
+import org.geysermc.floodgate.player.FloodgateHandshakeHandler.HandshakeResult;
+import org.geysermc.floodgate.util.Constants;
+
+@RequiredArgsConstructor
+public abstract class CommonDataHandler extends ChannelInboundHandlerAdapter {
+    protected final FloodgateHandshakeHandler handshakeHandler;
+    protected final FloodgateConfig config;
+    protected final AttributeKey<String> kickMessageAttribute;
+    protected final PacketBlocker blocker;
+
+    protected final Queue<Object> packetQueue = Queues.newConcurrentLinkedQueue();
+    protected ChannelHandlerContext ctx;
+
+    protected abstract void setNewIp(Channel channel, InetSocketAddress newIp);
+
+    protected abstract void setHostname(Object handshakePacket, String hostname);
+
+    protected abstract boolean channelRead(Object packet) throws Exception;
+
+    protected boolean shouldRemoveHandler(HandshakeResult result) {
+        return true;
+    }
+
+    protected boolean shouldCallFireRead(Object queuedPacket) {
+        return true;
+    }
+
+    protected void handle(Object handshakePacket, String hostname) {
+        Pair<String, String> separated = handshakeHandler.separateHostname(hostname);
+
+        if (separated.left() == null) {
+            // not a Floodgate player
+            removeSelf();
+            return;
+        }
+
+        blocker.enable();
+        packetQueue.add(handshakePacket);
+
+        Channel channel = ctx.channel();
+
+        handshakeHandler
+                .handle(channel, separated.left(), separated.right())
+                .thenApply(result -> {
+                    HandshakeData handshakeData = result.getHandshakeData();
+
+                    // we'll change the IP address to the real IP of the client very early on
+                    // so that almost every plugin will use the real IP of the client
+                    InetSocketAddress newIp = result.getNewIp(channel);
+                    if (newIp != null) {
+                        setNewIp(channel, newIp);
+                    }
+
+                    setHostname(handshakePacket, handshakeData.getHostname());
+
+                    if (handshakeData.shouldDisconnect()) {
+                        setKickMessage(handshakeData.getDisconnectReason());
+                        return shouldRemoveHandler(result);
+                    }
+
+                    switch (result.getResultType()) {
+                        case EXCEPTION:
+                            setKickMessage(Constants.INTERNAL_ERROR_MESSAGE);
+                            break;
+                        case DECRYPT_ERROR:
+                            setKickMessage(config.getDisconnect().getInvalidKey());
+                            break;
+                        case INVALID_DATA_LENGTH:
+                            setKickMessage(config.getDisconnect().getInvalidArgumentsLength());
+                            break;
+                        default:
+                            break;
+                    }
+                    return shouldRemoveHandler(result);
+                }).handle((shouldRemove, error) -> {
+                    if (error != null) {
+                        error.printStackTrace();
+                    }
+                    disablePacketQueue(shouldRemove);
+                    return shouldRemove;
+                });
+    }
+
+    protected void disablePacketQueue(boolean removeSelf) {
+        Object queuedPacket;
+        while ((queuedPacket = packetQueue.poll()) != null) {
+            if (shouldCallFireRead(queuedPacket)) {
+                ctx.fireChannelRead(queuedPacket);
+            }
+        }
+        if (removeSelf) {
+            removeSelf();
+        }
+        blocker.disable();
+    }
+
+    protected void removeSelf() {
+        ctx.pipeline().remove(this);
+    }
+
+    protected final void setKickMessage(String message) {
+        ctx.channel().attr(kickMessageAttribute).set(message);
+    }
+
+    protected final String getKickMessage() {
+        return ctx.channel().attr(kickMessageAttribute).get();
+    }
+
+    @Override
+    public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+        super.handlerAdded(ctx);
+        this.ctx = ctx;
+    }
+
+    @Override
+    public void channelRead(ChannelHandlerContext ctx, Object packet) {
+        // prevent other packets from being handled while we handle the handshake packet
+        if (!packetQueue.isEmpty()) {
+            packetQueue.add(packet);
+            return;
+        }
+
+        try {
+            if (channelRead(packet)) {
+                ctx.fireChannelRead(packet);
+            }
+        } catch (Exception exception) {
+            exception.printStackTrace();
+            ctx.close();
+        }
+    }
+
+    @Override
+    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+        super.exceptionCaught(ctx, cause);
+        if (config.isDebug()) {
+            cause.printStackTrace();
+        }
+    }
+}

common/src/main/java/org/geysermc/floodgate/addon/data/PacketBlocker.java

@@ -63,6 +63,10 @@ public class PacketBlocker extends ChannelInboundHandlerAdapter {
         ctx.pipeline().remove(this);
     }
 
+    public boolean enabled() {
+        return blockPackets;
+    }
+
     @Override
     public void channelRead(ChannelHandlerContext ctx, Object msg) {
         if (blockPackets || !packetQueue.isEmpty()) {

common/src/main/java/org/geysermc/floodgate/player/FloodgateHandshakeHandler.java

@@ -30,8 +30,6 @@ import static org.geysermc.floodgate.player.FloodgateHandshakeHandler.ResultType
 import static org.geysermc.floodgate.util.BedrockData.EXPECTED_LENGTH;
 
 import com.google.common.base.Charsets;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
 import io.netty.channel.Channel;
 import io.netty.util.AttributeKey;
 import it.unimi.dsi.fastutil.Pair;
@@ -40,12 +38,10 @@ import java.net.InetSocketAddress;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
-import java.util.concurrent.TimeUnit;
 import lombok.AccessLevel;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NonNull;
-import lombok.RequiredArgsConstructor;
 import org.geysermc.floodgate.addon.data.HandshakeDataImpl;
 import org.geysermc.floodgate.addon.data.HandshakeHandlersImpl;
 import org.geysermc.floodgate.api.SimpleFloodgateApi;
@@ -56,15 +52,11 @@ import org.geysermc.floodgate.api.player.PropertyKey;
 import org.geysermc.floodgate.config.FloodgateConfigHolder;
 import org.geysermc.floodgate.crypto.FloodgateCipher;
 import org.geysermc.floodgate.skin.SkinUploadManager;
-import org.geysermc.floodgate.time.TimeSyncer;
 import org.geysermc.floodgate.util.BedrockData;
-import org.geysermc.floodgate.util.Constants;
 import org.geysermc.floodgate.util.InvalidFormatException;
 import org.geysermc.floodgate.util.LinkedPlayer;
-import org.geysermc.floodgate.util.TimeSyncerHolder;
 import org.geysermc.floodgate.util.Utils;
 
-@RequiredArgsConstructor
 public final class FloodgateHandshakeHandler {
     private final HandshakeHandlersImpl handshakeHandlers;
     private final SimpleFloodgateApi api;
@@ -74,36 +66,79 @@ public final class FloodgateHandshakeHandler {
     private final AttributeKey<FloodgatePlayer> playerAttribute;
     private final FloodgateLogger logger;
 
-    public CompletableFuture<HandshakeResult> handle(
+    public FloodgateHandshakeHandler(
-            @NonNull Channel channel,
+            HandshakeHandlersImpl handshakeHandlers,
-            @NonNull String originalHostname) {
+            SimpleFloodgateApi api,
+            FloodgateCipher cipher,
+            FloodgateConfigHolder configHolder,
+            SkinUploadManager skinUploadManager,
+            AttributeKey<FloodgatePlayer> playerAttribute,
+            FloodgateLogger logger) {
 
-        String[] split = originalHostname.split("\0");
+        this.handshakeHandlers = handshakeHandlers;
-        String data = null;
+        this.api = api;
+        this.cipher = cipher;
+        this.configHolder = configHolder;
+        this.skinUploadManager = skinUploadManager;
+        this.playerAttribute = playerAttribute;
+        this.logger = logger;
+    }
+
+    /**
+     * Separates the Floodgate data from the hostname
+     *
+     * @param hostname the string to look in
+     * @return The first string is the Floodgate data (or null if there isn't) and the second string
+     * is the hostname without the Floodgate.
+     */
+    public Pair<String, String> separateHostname(@NonNull String hostname) {
+        String[] hostnameItems = hostname.split("\0");
+        String floodgateData = null;
 
         StringBuilder hostnameBuilder = new StringBuilder();
-        for (String value : split) {
+        for (String value : hostnameItems) {
-            if (data == null && FloodgateCipher.hasHeader(value)) {
+            if (floodgateData == null && FloodgateCipher.hasHeader(value)) {
-                data = value;
+                floodgateData = value;
                 continue;
             }
             hostnameBuilder.append(value).append('\0');
         }
         // hostname now doesn't have Floodgate data anymore if it had
-        String hostname = hostnameBuilder.toString();
+        return Pair.of(floodgateData, hostnameBuilder.toString());
+    }
 
-        if (data == null) {
+    public CompletableFuture<HandshakeResult> handle(
-            return CompletableFuture.completedFuture(
+            @NonNull Channel channel,
-                    callHandlerAndReturnResult(NOT_FLOODGATE_DATA, channel, null, hostname)
+            @NonNull String floodgateDataString,
-            );
+            @NonNull String hostname) {
-        }
 
-        byte[] floodgateData = data.getBytes(Charsets.UTF_8);
+        byte[] floodgateData = floodgateDataString.getBytes(Charsets.UTF_8);
 
         return CompletableFuture.supplyAsync(() -> {
+
+            String decrypted;
+            try {
+                // the actual decryption of the data
+                decrypted = cipher.decryptToString(floodgateData);
+            } catch (InvalidFormatException e) {
+                // when the Floodgate format couldn't be found
+                throw callHandlerAndReturnResult(
+                        NOT_FLOODGATE_DATA,
+                        channel, null, hostname
+                );
+            } catch (Exception e) {
+                // all the other exceptions are caused by invalid/tempered Floodgate data
+                if (configHolder.get().isDebug()) {
+                    e.printStackTrace();
+                }
+
+                throw callHandlerAndReturnResult(
+                        ResultType.DECRYPT_ERROR,
+                        channel, null, hostname
+                );
+            }
+
             try {
-                // actual decryption
-                String decrypted = cipher.decryptToString(floodgateData);
                 BedrockData bedrockData = BedrockData.fromString(decrypted);
 
                 if (bedrockData.getDataLength() != EXPECTED_LENGTH) {
@@ -122,22 +157,6 @@ public final class FloodgateHandshakeHandler {
                 // let's check if there is a link
                 return bedrockData;
 
-            } catch (InvalidFormatException formatException) {
-                // only header exceptions should return 'not floodgate data',
-                // all the other format exceptions are because of invalid/tempered Floodgate data
-                if (formatException.isHeader()) {
-                    throw callHandlerAndReturnResult(
-                            NOT_FLOODGATE_DATA,
-                            channel, null, hostname
-                    );
-                }
-
-                formatException.printStackTrace();
-
-                throw callHandlerAndReturnResult(
-                        ResultType.DECRYPT_ERROR,
-                        channel, null, hostname
-                );
             } catch (Exception exception) {
                 if (exception instanceof HandshakeResult) {
                     throw (HandshakeResult) exception;
@@ -174,7 +193,12 @@ public final class FloodgateHandshakeHandler {
         });
     }
 
-    private HandshakeResult handlePart2(Channel channel, String hostname, BedrockData bedrockData, LinkedPlayer linkedPlayer) {
+    private HandshakeResult handlePart2(
+            Channel channel,
+            String hostname,
+            BedrockData bedrockData,
+            LinkedPlayer linkedPlayer) {
+
         try {
             HandshakeData handshakeData = new HandshakeDataImpl(
                     channel, true, bedrockData.clone(), configHolder.get(),
@@ -252,7 +276,8 @@ public final class FloodgateHandshakeHandler {
                 .thenApply(link -> new ObjectObjectImmutablePair<>(data, link))
                 .handle((result, error) -> {
                     if (error != null) {
-                        logger.error("The player linking implementation returned an error", error.getCause());
+                        logger.error("The player linking implementation returned an error",
+                                error.getCause());
                         return new ObjectObjectImmutablePair<>(data, null);
                     }
                     return result;

spigot/src/main/java/org/geysermc/floodgate/addon/data/SpigotDataAddon.java

@@ -42,26 +42,24 @@ public final class SpigotDataAddon implements InjectorAddon {
     @Inject private SimpleFloodgateApi api;
     @Inject private FloodgateLogger logger;
 
-    @Inject
-    @Named("packetDecoder")
-    private String packetDecoder;
-
     @Inject
     @Named("packetHandler")
     private String packetHandlerName;
 
+    @Inject
+    @Named("kickMessageAttribute")
+    private AttributeKey<String> kickMessageAttribute;
+
     @Inject
     @Named("playerAttribute")
     private AttributeKey<FloodgatePlayer> playerAttribute;
 
     @Override
     public void onInject(Channel channel, boolean toServer) {
-        PacketBlocker blocker = new PacketBlocker();
+        // we have to add the packet blocker in the data handler, otherwise ProtocolSupport breaks
-        channel.pipeline().addBefore(packetDecoder, "floodgate_packet_blocker", blocker);
-
         channel.pipeline().addBefore(
                 packetHandlerName, "floodgate_data_handler",
-                new SpigotDataHandler(config, handshakeHandler, blocker, logger)
+                new SpigotDataHandler(handshakeHandler, config, kickMessageAttribute)
         );
     }
 

spigot/src/main/java/org/geysermc/floodgate/addon/data/SpigotDataHandler.java

@@ -28,131 +28,109 @@ package org.geysermc.floodgate.addon.data;
 import static org.geysermc.floodgate.util.ReflectionUtils.getCastedValue;
 import static org.geysermc.floodgate.util.ReflectionUtils.setValue;
 
-import com.google.common.collect.Queues;
 import com.mojang.authlib.GameProfile;
-import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.Channel;
-import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.util.AttributeKey;
 import java.net.InetSocketAddress;
-import java.util.Queue;
-import lombok.RequiredArgsConstructor;
-import org.geysermc.floodgate.api.handshake.HandshakeData;
-import org.geysermc.floodgate.api.logger.FloodgateLogger;
 import org.geysermc.floodgate.api.player.FloodgatePlayer;
 import org.geysermc.floodgate.config.FloodgateConfig;
 import org.geysermc.floodgate.player.FloodgateHandshakeHandler;
-import org.geysermc.floodgate.util.BedrockData;
+import org.geysermc.floodgate.player.FloodgateHandshakeHandler.HandshakeResult;
 import org.geysermc.floodgate.util.ClassNames;
-import org.geysermc.floodgate.util.Constants;
 import org.geysermc.floodgate.util.ProxyUtils;
 
-@RequiredArgsConstructor
+public final class SpigotDataHandler extends CommonDataHandler {
-public final class SpigotDataHandler extends ChannelInboundHandlerAdapter {
-    private final FloodgateConfig config;
-    private final FloodgateHandshakeHandler handshakeHandler;
-    private final PacketBlocker blocker;
-    private final FloodgateLogger logger;
-
-    private final Queue<Object> packetQueue = Queues.newConcurrentLinkedQueue();
-
     private Object networkManager;
     private FloodgatePlayer player;
 
-    @Override
+    public SpigotDataHandler(
-    public void channelRead(ChannelHandlerContext ctx, Object packet) throws Exception {
+            FloodgateHandshakeHandler handshakeHandler,
-        // prevent other packets from being handled while we handle the handshake packet
+            FloodgateConfig config,
-        if (!packetQueue.isEmpty()) {
+            AttributeKey<String> kickMessageAttribute) {
-            packetQueue.add(packet);
+        super(handshakeHandler, config, kickMessageAttribute, new PacketBlocker());
-            return;
-        }
-
-        if (ClassNames.HANDSHAKE_PACKET.isInstance(packet)) {
-            blocker.enable();
-            packetQueue.add(packet);
-
-            networkManager = ctx.channel().pipeline().get("packet_handler");
-            String handshakeValue = getCastedValue(packet, ClassNames.HANDSHAKE_HOST);
-
-            handshakeHandler.handle(ctx.channel(), handshakeValue).thenApply(result -> {
-                HandshakeData handshakeData = result.getHandshakeData();
-
-                setValue(packet, ClassNames.HANDSHAKE_HOST, handshakeData.getHostname());
-
-                InetSocketAddress newIp = result.getNewIp(ctx.channel());
-                if (newIp != null) {
-                    setValue(networkManager, ClassNames.SOCKET_ADDRESS, newIp);
-                    //todo the socket address will be overridden when bungeeData is true
-                }
-
-                if (handshakeData.getDisconnectReason() != null) {
-                    ctx.close(); //todo disconnect with message
-                    return true;
-                }
-
-                //todo use kickMessageAttribute and let this be common logic
-
-                switch (result.getResultType()) {
-                    case SUCCESS:
-                        break;
-                    case EXCEPTION:
-                        logger.info(Constants.INTERNAL_ERROR_MESSAGE);
-                        ctx.close();
-                        return true;
-                    case DECRYPT_ERROR:
-                        logger.info(config.getDisconnect().getInvalidKey());
-                        ctx.close();
-                        return true;
-                    case INVALID_DATA_LENGTH:
-                        int dataLength = result.getBedrockData().getDataLength();
-                        logger.info(
-                                config.getDisconnect().getInvalidArgumentsLength(),
-                                BedrockData.EXPECTED_LENGTH, dataLength
-                        );
-                        ctx.close();
-                        return true;
-                    default: // only continue when SUCCESS
-                        return true;
-                }
-
-                player = result.getFloodgatePlayer();
-                boolean bungeeData = ProxyUtils.isProxyData();
-
-                if (!bungeeData) {
-                    // Use a spoofedUUID for initUUID (just like Bungeecord)
-                    setValue(networkManager, "spoofedUUID", player.getCorrectUniqueId());
-                }
-                return bungeeData || player == null;
-            }).thenAccept(shouldRemove -> {
-                Object queuedPacket;
-                while ((queuedPacket = packetQueue.poll()) != null) {
-                    try {
-                        if (checkLogin(ctx, packet)) {
-                            break;
-                        }
-                    } catch (Exception ignored) {}
-                    ctx.fireChannelRead(queuedPacket);
-                }
-
-                if (shouldRemove) {
-                    ctx.pipeline().remove(SpigotDataHandler.this);
-                }
-                blocker.disable();
-            });
-            return;
-        }
-
-        if (!checkLogin(ctx, packet)) {
-            ctx.fireChannelRead(packet);
-        }
     }
 
-    private boolean checkLogin(ChannelHandlerContext ctx, Object packet) throws Exception {
+    @Override
+    protected void setNewIp(Channel channel, InetSocketAddress newIp) {
+        //todo the socket address will be overridden when bungeeData is true
+        setValue(networkManager, ClassNames.SOCKET_ADDRESS, newIp);
+    }
+
+    @Override
+    protected void setHostname(Object handshakePacket, String hostname) {
+        setValue(handshakePacket, ClassNames.HANDSHAKE_HOST, hostname);
+    }
+
+    @Override
+    protected boolean shouldRemoveHandler(HandshakeResult result) {
+        player = result.getFloodgatePlayer();
+
+        if (getKickMessage() != null) {
+            // we also have to keep this handler if we want to kick then with a disconnect message
+            return false;
+        } else if (player == null) {
+            // player is not a Floodgate player
+            return true;
+        }
+
+        // the server will do all the work if BungeeCord mode is enabled,
+        // otherwise we have to help the server.
+        boolean bungeeData = ProxyUtils.isProxyData();
+
+        if (!bungeeData) {
+            // Use a spoofedUUID for initUUID (just like Bungeecord)
+            setValue(networkManager, "spoofedUUID", player.getCorrectUniqueId());
+        }
+
+        return bungeeData;
+    }
+
+    @Override
+    protected boolean shouldCallFireRead(Object queuedPacket) {
+        // we have to ignore the 'login start' packet if BungeeCord mode is disabled,
+        // otherwise the server might ask the user to login
+        try {
+            if (checkAndHandleLogin(queuedPacket)) {
+                return false;
+            }
+        } catch (Exception exception) {
+            exception.printStackTrace();
+        }
+        return true;
+    }
+
+    @Override
+    public boolean channelRead(Object packet) throws Exception {
+        if (ClassNames.HANDSHAKE_PACKET.isInstance(packet)) {
+            // ProtocolSupport would break if we added this during the creation of this handler
+            ctx.pipeline().addAfter("splitter", "floodgate_packet_blocker", blocker);
+
+            networkManager = ctx.channel().pipeline().get("packet_handler");
+
+            handle(packet, getCastedValue(packet, ClassNames.HANDSHAKE_HOST));
+            // otherwise, it'll get read twice. once by the packet queue and once by this method
+            return false;
+        }
+
+        return !checkAndHandleLogin(packet);
+    }
+
+    private boolean checkAndHandleLogin(Object packet) throws Exception {
         if (ClassNames.LOGIN_START_PACKET.isInstance(packet)) {
-            // we have to fake the offline player (login) cycle
+            Object packetListener = ClassNames.PACKET_LISTENER.get(networkManager);
-            Object loginListener = ClassNames.PACKET_LISTENER.get(networkManager);
+
+            String kickMessage = getKickMessage();
+            if (kickMessage != null) {
+                disconnect(packetListener, kickMessage);
+                return true;
+            }
 
             // check if the server is actually in the Login state
-            if (!ClassNames.LOGIN_LISTENER.isInstance(loginListener)) {
+            if (!ClassNames.LOGIN_LISTENER.isInstance(packetListener)) {
                 // player is not in the login state, abort
+
+                // I would've liked to close the channel for security reasons, but our big friend
+                // ProtocolSupport, who likes to break things, doesn't work otherwise
                 ctx.pipeline().remove(this);
                 return true;
             }
@@ -161,8 +139,9 @@ public final class SpigotDataHandler extends ChannelInboundHandlerAdapter {
             GameProfile gameProfile = new GameProfile(
                     player.getCorrectUniqueId(), player.getCorrectUsername()
             );
-            setValue(loginListener, ClassNames.LOGIN_PROFILE, gameProfile);
+            setValue(packetListener, ClassNames.LOGIN_PROFILE, gameProfile);
 
+            // we have to fake the offline player (login) cycle
             // just like on Spigot:
 
             // LoginListener#initUUID
@@ -170,10 +149,10 @@ public final class SpigotDataHandler extends ChannelInboundHandlerAdapter {
 
             // and the tick of LoginListener will do the rest
 
-            ClassNames.INIT_UUID.invoke(loginListener);
+            ClassNames.INIT_UUID.invoke(packetListener);
 
             Object loginHandler =
-                    ClassNames.LOGIN_HANDLER_CONSTRUCTOR.newInstance(loginListener);
+                    ClassNames.LOGIN_HANDLER_CONSTRUCTOR.newInstance(packetListener);
             ClassNames.FIRE_LOGIN_EVENTS.invoke(loginHandler);
 
             ctx.pipeline().remove(this);
@@ -182,11 +161,16 @@ public final class SpigotDataHandler extends ChannelInboundHandlerAdapter {
         return false;
     }
 
-    @Override
+    private void disconnect(Object packetListener, String kickMessage) throws Exception {
-    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+        // both versions close the channel for us
-        super.exceptionCaught(ctx, cause);
+        if (ClassNames.LOGIN_LISTENER.isInstance(packetListener)) {
-        if (config.isDebug()) {
+            ClassNames.LOGIN_DISCONNECT.invoke(packetListener, kickMessage);
-            cause.printStackTrace();
+        } else {
+            // ProtocolSupport for example has their own PacketLoginInListener implementation
+            ClassNames.NETWORK_EXCEPTION_CAUGHT.invoke(
+                    networkManager,
+                    ctx, new IllegalStateException(kickMessage)
+            );
         }
     }
 }

spigot/src/main/java/org/geysermc/floodgate/util/ClassNames.java

@@ -33,6 +33,7 @@ import static org.geysermc.floodgate.util.ReflectionUtils.makeAccessible;
 
 import com.google.common.base.Preconditions;
 import com.mojang.authlib.GameProfile;
+import io.netty.channel.ChannelHandlerContext;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
@@ -65,6 +66,8 @@ public class ClassNames {
     public static final Method IS_WHITELISTED;
     public static final Method ADD_WHITELIST_ENTRY;
     public static final Method REMOVE_WHITELIST_ENTRY;
+    public static final Method LOGIN_DISCONNECT;
+    public static final Method NETWORK_EXCEPTION_CAUGHT;
     public static final Method INIT_UUID;
     public static final Method FIRE_LOGIN_EVENTS;
 
@@ -166,6 +169,15 @@ public class ClassNames {
         LOGIN_PROFILE = getFieldOfType(LOGIN_LISTENER, GameProfile.class);
         checkNotNull(LOGIN_PROFILE, "Profile from LoginListener");
 
+        LOGIN_DISCONNECT = getMethod(LOGIN_LISTENER, "disconnect", String.class);
+        checkNotNull(LOGIN_DISCONNECT, "LoginListener's disconnect method");
+
+        NETWORK_EXCEPTION_CAUGHT = getMethod(
+                networkManager,
+                "exceptionCaught",
+                ChannelHandlerContext.class, Throwable.class
+        );
+
         // there are multiple no-arg void methods
         INIT_UUID = getMethod(LOGIN_LISTENER, "initUUID");
         checkNotNull(INIT_UUID, "initUUID from LoginListener");

velocity/src/main/java/org/geysermc/floodgate/addon/data/VelocityProxyDataHandler.java

@@ -31,24 +31,18 @@ import static org.geysermc.floodgate.util.ReflectionUtils.getField;
 import static org.geysermc.floodgate.util.ReflectionUtils.getPrefixedClass;
 import static org.geysermc.floodgate.util.ReflectionUtils.setValue;
 
-import com.google.common.collect.Queues;
+import io.netty.channel.Channel;
-import io.netty.channel.ChannelHandlerContext;
-import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.util.AttributeKey;
 import java.lang.reflect.Field;
 import java.net.InetSocketAddress;
-import java.util.Queue;
-import java.util.concurrent.CompletableFuture;
-import lombok.RequiredArgsConstructor;
-import org.geysermc.floodgate.api.handshake.HandshakeData;
 import org.geysermc.floodgate.api.logger.FloodgateLogger;
 import org.geysermc.floodgate.api.player.FloodgatePlayer;
-import org.geysermc.floodgate.config.ProxyFloodgateConfig;
+import org.geysermc.floodgate.config.FloodgateConfig;
 import org.geysermc.floodgate.player.FloodgateHandshakeHandler;
-import org.geysermc.floodgate.util.Constants;
+import org.geysermc.floodgate.player.FloodgateHandshakeHandler.HandshakeResult;
+import org.geysermc.floodgate.player.FloodgateHandshakeHandler.ResultType;
 
-@RequiredArgsConstructor
+public final class VelocityProxyDataHandler extends CommonDataHandler {
-public final class VelocityProxyDataHandler extends ChannelInboundHandlerAdapter {
     private static final Field HANDSHAKE;
     private static final Class<?> HANDSHAKE_PACKET;
     private static final Field HANDSHAKE_SERVER_ADDRESS;
@@ -71,97 +65,47 @@ public final class VelocityProxyDataHandler extends ChannelInboundHandlerAdapter
         REMOTE_ADDRESS = getField(minecraftConnection, "remoteAddress");
     }
 
-    private final ProxyFloodgateConfig config;
-    private final FloodgateHandshakeHandler handshakeHandler;
-    private final PacketBlocker blocker;
-    private final AttributeKey<String> kickMessageAttribute;
     private final FloodgateLogger logger;
 
-    private final Queue<Object> packetQueue = Queues.newConcurrentLinkedQueue();
+    public VelocityProxyDataHandler(
-
+            FloodgateConfig config,
-    @Override
+            FloodgateHandshakeHandler handshakeHandler,
-    public void channelRead(ChannelHandlerContext ctx, Object msg) {
+            PacketBlocker blocker,
-        // prevent other packets from being handled while we handle the handshake packet
+            AttributeKey<String> kickMessageAttribute,
-        if (!packetQueue.isEmpty()) {
+            FloodgateLogger logger) {
-            packetQueue.add(msg);
+        super(handshakeHandler, config, kickMessageAttribute, blocker);
-            return;
+        this.logger = logger;
-        }
-
-        // we're only interested in the Handshake packet.
-        // it should be the first packet but you never know
-        if (HANDSHAKE_PACKET.isInstance(msg)) {
-            blocker.enable();
-            packetQueue.add(msg);
-
-            handleClientToProxy(ctx, msg).thenRun(() -> {
-                Object packet;
-                while ((packet = packetQueue.poll()) != null) {
-                    ctx.fireChannelRead(packet);
-                }
-                ctx.pipeline().remove(this);
-                blocker.disable();
-            });
-            return;
-        }
-
-        ctx.fireChannelRead(msg);
     }
 
-    private CompletableFuture<Void> handleClientToProxy(ChannelHandlerContext ctx, Object packet) {
+    @Override
-        String address = getCastedValue(packet, HANDSHAKE_SERVER_ADDRESS);
+    protected void setNewIp(Channel channel, InetSocketAddress newIp) {
+        setValue(channel.pipeline().get("handler"), REMOTE_ADDRESS, newIp);
+    }
 
-        return handshakeHandler.handle(ctx.channel(), address).thenAccept(result -> {
+    @Override
-            HandshakeData handshakeData = result.getHandshakeData();
+    protected void setHostname(Object handshakePacket, String hostname) {
-
+        setValue(handshakePacket, HANDSHAKE_SERVER_ADDRESS, hostname);
-            InetSocketAddress newIp = result.getNewIp(ctx.channel());
+    }
-            if (newIp != null) {
-                Object connection = ctx.pipeline().get("handler");
-                setValue(connection, REMOTE_ADDRESS, newIp);
-            }
-
-            setValue(packet, HANDSHAKE_SERVER_ADDRESS, handshakeData.getHostname());
-
-            if (handshakeData.getDisconnectReason() != null) {
-                ctx.channel().attr(kickMessageAttribute).set(handshakeData.getDisconnectReason());
-                return;
-            }
-
-            switch (result.getResultType()) {
-                case SUCCESS:
-                    break;
-                case EXCEPTION:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(Constants.INTERNAL_ERROR_MESSAGE);
-                    return;
-                case DECRYPT_ERROR:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(config.getDisconnect().getInvalidKey());
-                    return;
-                case INVALID_DATA_LENGTH:
-                    ctx.channel().attr(kickMessageAttribute)
-                            .set(config.getDisconnect().getInvalidArgumentsLength());
-                    return;
-                default: // only continue when SUCCESS
-                    return;
-            }
 
+    @Override
+    protected boolean shouldRemoveHandler(HandshakeResult result) {
+        if (result.getResultType() == ResultType.SUCCESS) {
             FloodgatePlayer player = result.getFloodgatePlayer();
-
             logger.info("Floodgate player who is logged in as {} {} joined",
                     player.getCorrectUsername(), player.getCorrectUniqueId());
-        }).handle((v, error) -> {
+        }
-            if (error != null) {
+        return true;
-                error.printStackTrace();
-            }
-            return v;
-        });
     }
 
     @Override
-    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+    public boolean channelRead(Object packet) {
-        super.exceptionCaught(ctx, cause);
+        // we're only interested in the Handshake packet.
-        if (config.isDebug()) {
+        // it should be the first packet but you never know
-            cause.printStackTrace();
+        if (HANDSHAKE_PACKET.isInstance(packet)) {
+            handle(packet, getCastedValue(packet, HANDSHAKE_SERVER_ADDRESS));
+            // otherwise, it'll get read twice. once by the packet queue and once by this method
+            return false;
         }
+        return true;
     }
 }