From c520e04ac890c0b17972b03f5b0e92a40b7f7976 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 2 Sep 2018 20:50:24 -0400 Subject: [PATCH 1/3] [BREAKING] PluginMessageEvent instead --- .../event/connection/PluginMessageEvent.java | 94 +++++++++++++++++++ .../api/proxy/messages/ChannelRegistrar.java | 8 +- .../api/proxy/messages/ChannelSide.java | 15 --- .../api/proxy/messages/MessageHandler.java | 28 ------ .../backend/BackendPlaySessionHandler.java | 16 ++-- .../client/ClientPlaySessionHandler.java | 17 ++-- .../messages/VelocityChannelRegistrar.java | 29 +----- 7 files changed, 120 insertions(+), 87 deletions(-) create mode 100644 api/src/main/java/com/velocitypowered/api/event/connection/PluginMessageEvent.java delete mode 100644 api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelSide.java delete mode 100644 api/src/main/java/com/velocitypowered/api/proxy/messages/MessageHandler.java diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PluginMessageEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PluginMessageEvent.java new file mode 100644 index 000000000..4caf3280c --- /dev/null +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PluginMessageEvent.java @@ -0,0 +1,94 @@ +package com.velocitypowered.api.event.connection; + +import com.google.common.base.Preconditions; +import com.google.common.io.ByteArrayDataInput; +import com.google.common.io.ByteStreams; +import com.velocitypowered.api.event.ResultedEvent; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; +import com.velocitypowered.api.proxy.messages.ChannelMessageSink; +import com.velocitypowered.api.proxy.messages.ChannelMessageSource; +import org.checkerframework.checker.nullness.qual.NonNull; + +import java.util.Arrays; + +/** + * This event is fired when a plugin message is sent to the proxy, either from a client ({@link com.velocitypowered.api.proxy.Player}) + * or a server ({@link com.velocitypowered.api.proxy.ServerConnection}). + */ +public class PluginMessageEvent implements ResultedEvent { + private final ChannelMessageSource source; + private final ChannelMessageSink target; + private final ChannelIdentifier identifier; + private final byte[] data; + private ForwardResult result; + + public PluginMessageEvent(ChannelMessageSource source, ChannelMessageSink target, ChannelIdentifier identifier, byte[] data) { + this.source = Preconditions.checkNotNull(source, "source"); + this.target = Preconditions.checkNotNull(target, "target"); + this.identifier = Preconditions.checkNotNull(identifier, "identifier"); + this.data = Preconditions.checkNotNull(data, "data"); + this.result = ForwardResult.forward(); + } + + @Override + public ForwardResult getResult() { + return result; + } + + @Override + public void setResult(@NonNull ForwardResult result) { + this.result = Preconditions.checkNotNull(result, "result"); + } + + public ChannelMessageSource getSource() { + return source; + } + + public ChannelMessageSink getTarget() { + return target; + } + + public ChannelIdentifier getIdentifier() { + return identifier; + } + + public byte[] getData() { + return Arrays.copyOf(data, data.length); + } + + public ByteArrayDataInput dataAsDataStream() { + return ByteStreams.newDataInput(data); + } + + /** + * A result determining whether or not to forward this message on. + */ + public static class ForwardResult implements ResultedEvent.Result { + private static final ForwardResult ALLOWED = new ForwardResult(true); + private static final ForwardResult DENIED = new ForwardResult(false); + + private final boolean allowed; + + private ForwardResult(boolean b) { + this.allowed = b; + } + + @Override + public boolean isAllowed() { + return allowed; + } + + @Override + public String toString() { + return allowed ? "forward to sink" : "handled message at proxy"; + } + + public static ForwardResult forward() { + return ALLOWED; + } + + public static ForwardResult handled() { + return DENIED; + } + } +} diff --git a/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelRegistrar.java b/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelRegistrar.java index 2d77988b5..84db8799d 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelRegistrar.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelRegistrar.java @@ -1,16 +1,14 @@ package com.velocitypowered.api.proxy.messages; /** - * Represents an interface to register and unregister {@link MessageHandler} instances for handling plugin messages from - * the client or the server. + * Represents an interface to register and unregister {@link ChannelIdentifier}s for the proxy to listen on. */ public interface ChannelRegistrar { /** - * Registers the specified message handler to listen for plugin messages on the specified channels. - * @param handler the handler to register + * Registers the specified message identifiers to listen on for the * @param identifiers the channel identifiers to register */ - void register(MessageHandler handler, ChannelIdentifier... identifiers); + void register(ChannelIdentifier... identifiers); /** * Unregisters the handler for the specified channel. diff --git a/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelSide.java b/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelSide.java deleted file mode 100644 index 12256f432..000000000 --- a/api/src/main/java/com/velocitypowered/api/proxy/messages/ChannelSide.java +++ /dev/null @@ -1,15 +0,0 @@ -package com.velocitypowered.api.proxy.messages; - -/** - * Represents from "which side" of the proxy the plugin message came from. - */ -public enum ChannelSide { - /** - * The plugin message came from a server that a client was connected to. - */ - FROM_SERVER, - /** - * The plugin message came from the client. - */ - FROM_CLIENT -} diff --git a/api/src/main/java/com/velocitypowered/api/proxy/messages/MessageHandler.java b/api/src/main/java/com/velocitypowered/api/proxy/messages/MessageHandler.java deleted file mode 100644 index 70a7e5fa7..000000000 --- a/api/src/main/java/com/velocitypowered/api/proxy/messages/MessageHandler.java +++ /dev/null @@ -1,28 +0,0 @@ -package com.velocitypowered.api.proxy.messages; - -/** - * Represents a handler for handling plugin messages. - */ -public interface MessageHandler { - /** - * Handles an incoming plugin message. - * @param source the source of the plugin message - * @param side from where the plugin message originated - * @param identifier the channel on which the message was sent - * @param data the data inside the plugin message - * @return a {@link ForwardStatus} indicating whether or not to forward this plugin message on - */ - ForwardStatus handle(ChannelMessageSource source, ChannelSide side, ChannelIdentifier identifier, byte[] data); - - enum ForwardStatus { - /** - * Forwards this plugin message on to the client or server, depending on the {@link ChannelSide} it originated - * from. - */ - FORWARD, - /** - * Discard the plugin message and do not forward it on. - */ - HANDLED - } -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 60ed678c0..8fbee6e6d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -1,8 +1,7 @@ package com.velocitypowered.proxy.connection.backend; +import com.velocitypowered.api.event.connection.PluginMessageEvent; import com.velocitypowered.api.event.player.ServerConnectedEvent; -import com.velocitypowered.api.proxy.messages.ChannelSide; -import com.velocitypowered.api.proxy.messages.MessageHandler; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; @@ -68,11 +67,14 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { return; } - MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(connection, - ChannelSide.FROM_SERVER, pm); - if (status == MessageHandler.ForwardStatus.FORWARD) { - connection.getPlayer().getConnection().write(pm); - } + PluginMessageEvent event = new PluginMessageEvent(connection, connection.getPlayer(), server.getChannelRegistrar().getFromId(pm.getChannel()), + pm.getData()); + server.getEventManager().fire(event) + .thenAcceptAsync(pme -> { + if (pme.getResult().isAllowed()) { + connection.getPlayer().getConnection().write(pm); + } + }, connection.getMinecraftConnection().getChannel().eventLoop()); } else { // Just forward the packet on. We don't have anything to handle at this time. connection.getPlayer().getConnection().write(packet); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index 9c36766ef..aa296599d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -1,8 +1,7 @@ package com.velocitypowered.proxy.connection.client; import com.velocitypowered.api.event.connection.DisconnectEvent; -import com.velocitypowered.api.proxy.messages.ChannelSide; -import com.velocitypowered.api.proxy.messages.MessageHandler; +import com.velocitypowered.api.event.connection.PluginMessageEvent; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; @@ -235,12 +234,14 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return; } - MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, - ChannelSide.FROM_CLIENT, packet); - if (status == MessageHandler.ForwardStatus.FORWARD) { - // We're going to forward on the original packet. - player.getConnectedServer().getMinecraftConnection().write(packet); - } + PluginMessageEvent event = new PluginMessageEvent(player, player.getConnectedServer(), + server.getChannelRegistrar().getFromId(packet.getChannel()), packet.getData()); + server.getEventManager().fire(event) + .thenAcceptAsync(pme -> { + if (pme.getResult().isAllowed()) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } + }, player.getConnectedServer().getMinecraftConnection().getChannel().eventLoop()); } public Set getClientPluginMsgChannels() { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/messages/VelocityChannelRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/messages/VelocityChannelRegistrar.java index 1fb47cce1..1536716cc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/messages/VelocityChannelRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/messages/VelocityChannelRegistrar.java @@ -3,9 +3,6 @@ package com.velocitypowered.proxy.messages; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.velocitypowered.api.proxy.messages.*; -import com.velocitypowered.proxy.protocol.packet.PluginMessage; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import java.util.Collection; import java.util.Map; @@ -13,39 +10,20 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; public class VelocityChannelRegistrar implements ChannelRegistrar { - private static final Logger logger = LogManager.getLogger(VelocityChannelRegistrar.class); - private final Map handlers = new ConcurrentHashMap<>(); private final Map identifierMap = new ConcurrentHashMap<>(); @Override - public void register(MessageHandler handler, ChannelIdentifier... identifiers) { + public void register(ChannelIdentifier... identifiers) { for (ChannelIdentifier identifier : identifiers) { Preconditions.checkArgument(identifier instanceof LegacyChannelIdentifier || identifier instanceof MinecraftChannelIdentifier, "identifier is unknown"); } for (ChannelIdentifier identifier : identifiers) { - handlers.put(identifier.getId(), handler); identifierMap.put(identifier.getId(), identifier); } } - public MessageHandler.ForwardStatus handlePluginMessage(ChannelMessageSource source, ChannelSide side, PluginMessage message) { - MessageHandler handler = handlers.get(message.getChannel()); - ChannelIdentifier identifier = identifierMap.get(message.getChannel()); - if (handler == null || identifier == null) { - return MessageHandler.ForwardStatus.FORWARD; - } - - try { - return handler.handle(source, side, identifier, message.getData()); - } catch (Exception e) { - logger.info("Unable to handle plugin message on channel {} for {}", message.getChannel(), source); - // In case of doubt, do not forward the message on. - return MessageHandler.ForwardStatus.HANDLED; - } - } - @Override public void unregister(ChannelIdentifier... identifiers) { for (ChannelIdentifier identifier : identifiers) { @@ -54,7 +32,6 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { } for (ChannelIdentifier identifier : identifiers) { - handlers.remove(identifier.getId()); identifierMap.remove(identifier.getId()); } } @@ -73,4 +50,8 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { public boolean registered(String id) { return identifierMap.containsKey(id); } + + public ChannelIdentifier getFromId(String id) { + return identifierMap.get(id); + } } From 8b94fe6ed2343bc3eaf06b01ff487e3edd9a552a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 16 Sep 2018 02:41:35 -0400 Subject: [PATCH 2/3] Readd missing check --- .../proxy/connection/backend/BackendPlaySessionHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 0f82b12eb..e0dc386af 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -92,7 +92,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getPlayer().getConnection().write(pm); } }, connection.getMinecraftConnection().getChannel().eventLoop()); - } else { + } else if (connection.hasCompletedJoin()) { // Just forward the packet on. We don't have anything to handle at this time. connection.getPlayer().getConnection().write(packet); } From 2a842bffbe13d0db88d14c5ea184eefb7615a1a5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 16 Sep 2018 15:32:51 -0400 Subject: [PATCH 3/3] Add missing null check --- .../backend/BackendPlaySessionHandler.java | 21 ++++++++++++------- .../client/ClientPlaySessionHandler.java | 21 ++++++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index e0dc386af..2785a38b9 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.connection.backend; import com.velocitypowered.api.event.connection.PluginMessageEvent; import com.velocitypowered.api.event.player.ServerConnectedEvent; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; @@ -84,14 +85,18 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { return; } - PluginMessageEvent event = new PluginMessageEvent(connection, connection.getPlayer(), server.getChannelRegistrar().getFromId(pm.getChannel()), - pm.getData()); - server.getEventManager().fire(event) - .thenAcceptAsync(pme -> { - if (pme.getResult().isAllowed()) { - connection.getPlayer().getConnection().write(pm); - } - }, connection.getMinecraftConnection().getChannel().eventLoop()); + ChannelIdentifier id = server.getChannelRegistrar().getFromId(pm.getChannel()); + if (id == null) { + connection.getPlayer().getConnection().write(pm); + } else { + PluginMessageEvent event = new PluginMessageEvent(connection, connection.getPlayer(), id, pm.getData()); + server.getEventManager().fire(event) + .thenAcceptAsync(pme -> { + if (pme.getResult().isAllowed()) { + connection.getPlayer().getConnection().write(pm); + } + }, connection.getMinecraftConnection().getChannel().eventLoop()); + } } else if (connection.hasCompletedJoin()) { // Just forward the packet on. We don't have anything to handle at this time. connection.getPlayer().getConnection().write(packet); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index b5a88ee55..836481fba 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.connection.client; import com.velocitypowered.api.event.connection.DisconnectEvent; import com.velocitypowered.api.event.connection.PluginMessageEvent; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; @@ -298,14 +299,18 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { loginPluginMessages.add(packet); } } else { - PluginMessageEvent event = new PluginMessageEvent(player, player.getConnectedServer(), - server.getChannelRegistrar().getFromId(packet.getChannel()), packet.getData()); - server.getEventManager().fire(event) - .thenAcceptAsync(pme -> { - if (pme.getResult().isAllowed()) { - player.getConnectedServer().getMinecraftConnection().write(packet); - } - }, player.getConnectedServer().getMinecraftConnection().getChannel().eventLoop()); + ChannelIdentifier id = server.getChannelRegistrar().getFromId(packet.getChannel()); + if (id == null) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } else { + PluginMessageEvent event = new PluginMessageEvent(player, player.getConnectedServer(), id, packet.getData()); + server.getEventManager().fire(event) + .thenAcceptAsync(pme -> { + if (pme.getResult().isAllowed()) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } + }, player.getConnectedServer().getMinecraftConnection().getChannel().eventLoop()); + } } }