diff --git a/src/main/java/com/comphenix/protocol/async/AsyncListenerHandler.java b/src/main/java/com/comphenix/protocol/async/AsyncListenerHandler.java index e5b357ea7..2b1ae637b 100644 --- a/src/main/java/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/src/main/java/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -607,8 +607,6 @@ private void processPacket(int workerID, PacketEvent packet, String methodName) } catch (OutOfMemoryError e) { throw e; - } catch (ThreadDeath e) { - throw e; } catch (Throwable e) { // Minecraft doesn't want your Exception. filterManager.getErrorReporter().reportMinimal(listener.getPlugin(), methodName, e); diff --git a/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java b/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java index 0d7299ab9..ad945e94a 100644 --- a/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java +++ b/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java @@ -30,8 +30,12 @@ import java.util.logging.Level; import java.util.logging.Logger; +import org.apache.commons.lang.builder.ToStringBuilder; +import org.apache.commons.lang.builder.ToStringStyle; +import org.bukkit.Bukkit; +import org.bukkit.plugin.Plugin; + import com.comphenix.protocol.ProtocolConfig; -import com.comphenix.protocol.ProtocolLib; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolLogger; import com.comphenix.protocol.collections.ExpireHashMap; @@ -41,11 +45,6 @@ import com.google.common.base.Preconditions; import com.google.common.primitives.Primitives; -import org.apache.commons.lang.builder.ToStringBuilder; -import org.apache.commons.lang.builder.ToStringStyle; -import org.bukkit.Bukkit; -import org.bukkit.plugin.Plugin; - /** * Internal class used to handle exceptions. * @@ -479,7 +478,7 @@ public static String getStringDescription(Object value) { } catch (LinkageError ex) { // Apache is probably missing apacheCommonsMissing = true; - } catch (ThreadDeath | OutOfMemoryError e) { + } catch (OutOfMemoryError e) { throw e; } catch (Throwable ex) { // Don't use the error logger to log errors in error logging (that could lead to infinite loops) diff --git a/src/main/java/com/comphenix/protocol/events/ListenerOptions.java b/src/main/java/com/comphenix/protocol/events/ListenerOptions.java index c948a9fd3..e8cc83335 100644 --- a/src/main/java/com/comphenix/protocol/events/ListenerOptions.java +++ b/src/main/java/com/comphenix/protocol/events/ListenerOptions.java @@ -13,6 +13,7 @@ public enum ListenerOptions { * Disable the automatic game phase detection that will normally force {@link GamePhase#LOGIN} when a packet ID is * known to be transmitted during login. */ + @Deprecated DISABLE_GAMEPHASE_DETECTION, /** diff --git a/src/main/java/com/comphenix/protocol/events/ListeningWhitelist.java b/src/main/java/com/comphenix/protocol/events/ListeningWhitelist.java index b88e7b149..fd9f0ba01 100644 --- a/src/main/java/com/comphenix/protocol/events/ListeningWhitelist.java +++ b/src/main/java/com/comphenix/protocol/events/ListeningWhitelist.java @@ -17,7 +17,12 @@ package com.comphenix.protocol.events; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; import com.comphenix.protocol.PacketType; import com.comphenix.protocol.injector.GamePhase; @@ -90,20 +95,6 @@ public static Builder newBuilder(ListeningWhitelist template) { return new Builder(template); } - /** - * Construct a copy of a given enum. - * - * @param options - the options to copy, or NULL to indicate the empty set. - * @return A copy of the enum set. - */ - private static > EnumSet safeEnumSet(Collection options, Class enumClass) { - if (options != null && !options.isEmpty()) { - return EnumSet.copyOf(options); - } else { - return EnumSet.noneOf(enumClass); - } - } - /** * Construct a copy of a given set. * @@ -311,6 +302,7 @@ public Builder types(Collection types) { * @param gamePhase - the gamephase. * @return This builder, for chaining. */ + @Deprecated public Builder gamePhase(GamePhase gamePhase) { this.gamePhase = gamePhase; return this; @@ -321,6 +313,7 @@ public Builder gamePhase(GamePhase gamePhase) { * * @return This builder, for chaining. */ + @Deprecated public Builder gamePhaseBoth() { return gamePhase(GamePhase.BOTH); } diff --git a/src/main/java/com/comphenix/protocol/injector/GamePhase.java b/src/main/java/com/comphenix/protocol/injector/GamePhase.java index 7de5e9910..95330fdd4 100644 --- a/src/main/java/com/comphenix/protocol/injector/GamePhase.java +++ b/src/main/java/com/comphenix/protocol/injector/GamePhase.java @@ -22,6 +22,7 @@ * * @author Kristian */ +@Deprecated public enum GamePhase { /** * Only listen for packets sent or received before a player has logged in. diff --git a/src/main/java/com/comphenix/protocol/injector/NetworkProcessor.java b/src/main/java/com/comphenix/protocol/injector/NetworkProcessor.java index 01701f117..2d5fe49c7 100644 --- a/src/main/java/com/comphenix/protocol/injector/NetworkProcessor.java +++ b/src/main/java/com/comphenix/protocol/injector/NetworkProcessor.java @@ -1,5 +1,7 @@ package com.comphenix.protocol.injector; +import java.util.Deque; + import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolManager; import com.comphenix.protocol.error.ErrorReporter; @@ -7,7 +9,6 @@ import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.events.PacketPostListener; import com.comphenix.protocol.events.ScheduledPacket; -import java.util.Deque; /** * Represents a processor for network markers. @@ -43,7 +44,7 @@ public void invokePostEvent(PacketEvent event, NetworkMarker marker) { for (PacketPostListener listener : marker.getPostListeners()) { try { listener.onPostEvent(event); - } catch (OutOfMemoryError | ThreadDeath e) { + } catch (OutOfMemoryError e) { throw e; } catch (Throwable e) { this.reporter.reportMinimal(listener.getPlugin(), "SentListener.run()", e); diff --git a/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java b/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java index 6a32b4e38..8895181b2 100644 --- a/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java +++ b/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java @@ -172,8 +172,6 @@ public PacketConstructor withPacket(PacketType type, Object[] values) { result = unwrapper.unwrapItem(values[i]); } catch (OutOfMemoryError e) { throw e; - } catch (ThreadDeath e) { - throw e; } catch (Throwable e) { lastException = e; } diff --git a/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index c8827bfdf..3273c7260 100644 --- a/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -46,14 +46,10 @@ import com.comphenix.protocol.injector.netty.WirePacket; import com.comphenix.protocol.injector.netty.manager.NetworkManagerInjector; import com.comphenix.protocol.injector.packet.PacketRegistry; -import com.comphenix.protocol.injector.player.PlayerInjectionHandler; -import com.comphenix.protocol.injector.player.PlayerInjectionHandler.ConflictStrategy; import com.comphenix.protocol.utility.MinecraftReflection; import com.comphenix.protocol.utility.MinecraftVersion; import com.google.common.collect.ImmutableSet; -import io.netty.channel.Channel; - public class PacketFilterManager implements ListenerInvoker, InternalManager { // plugin verifier reports @@ -80,7 +76,6 @@ public class PacketFilterManager implements ListenerInvoker, InternalManager { private final Set registeredListeners; // injectors - private final PlayerInjectionHandler playerInjectionHandler; private final NetworkManagerInjector networkManagerInjector; // status of this manager @@ -111,10 +106,8 @@ public PacketFilterManager(PacketFilterBuilder builder) { // injectors this.networkManagerInjector = new NetworkManagerInjector( builder.getLibrary(), - builder.getServer(), this, builder.getReporter()); - this.playerInjectionHandler = this.networkManagerInjector.getPlayerInjectionHandler(); // ensure that all packet types are loaded and synced PacketRegistry.getClientPacketTypes(); @@ -122,7 +115,7 @@ public PacketFilterManager(PacketFilterBuilder builder) { // hook into all connected players for (Player player : this.server.getOnlinePlayers()) { - this.playerInjectionHandler.injectPlayer(player, ConflictStrategy.BAIL_OUT); + this.networkManagerInjector.getInjector(player).inject(); } } @@ -180,7 +173,7 @@ public void sendServerPacket(Player receiver, PacketContainer packet, NetworkMar } // process outbound - this.playerInjectionHandler.sendServerPacket(receiver, packet, marker, filters); + this.networkManagerInjector.getInjector(receiver).sendServerPacket(packet.getHandle(), marker, filters); } } @@ -192,13 +185,7 @@ public void sendWirePacket(Player receiver, int id, byte[] bytes) { @Override public void sendWirePacket(Player receiver, WirePacket packet) { if (!this.closed) { - // special case - we just throw the wire packet down the pipeline without processing it - Channel outboundChannel = this.playerInjectionHandler.getChannel(receiver); - if (outboundChannel == null) { - throw new IllegalArgumentException("Unable to obtain connection of player " + receiver); - } - - outboundChannel.writeAndFlush(packet); + this.networkManagerInjector.getInjector(receiver).sendWirePacket(packet); } } @@ -240,13 +227,13 @@ public void receiveClientPacket(Player sender, PacketContainer packet, NetworkMa } // post to the player inject, reset our cancel state change - this.playerInjectionHandler.receiveClientPacket(sender, nmsPacket); + this.networkManagerInjector.getInjector(sender).receiveClientPacket(nmsPacket); } } @Override public int getProtocolVersion(Player player) { - return this.playerInjectionHandler.getProtocolVersion(player); + return this.networkManagerInjector.getInjector(player).getProtocolVersion(); } @Override @@ -447,12 +434,12 @@ public void registerEvents(PluginManager manager, Plugin plugin) { @EventHandler(priority = EventPriority.LOWEST) public void handleLogin(PlayerLoginEvent event) { - PacketFilterManager.this.playerInjectionHandler.updatePlayer(event.getPlayer()); + networkManagerInjector.getInjector(event.getPlayer()).inject(); } @EventHandler(priority = EventPriority.LOWEST) public void handleJoin(PlayerJoinEvent event) { - PacketFilterManager.this.playerInjectionHandler.updatePlayer(event.getPlayer()); + networkManagerInjector.getInjector(event.getPlayer()).inject(); } @EventHandler(priority = EventPriority.MONITOR) diff --git a/src/main/java/com/comphenix/protocol/injector/netty/Injector.java b/src/main/java/com/comphenix/protocol/injector/netty/Injector.java index 9614ccd82..87a586185 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/Injector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/Injector.java @@ -1,9 +1,12 @@ package com.comphenix.protocol.injector.netty; +import java.net.SocketAddress; + +import org.bukkit.entity.Player; + import com.comphenix.protocol.PacketType; import com.comphenix.protocol.PacketType.Protocol; import com.comphenix.protocol.events.NetworkMarker; -import org.bukkit.entity.Player; /** * Represents an injected client connection. @@ -23,12 +26,8 @@ public interface Injector { * Inject the current channel. *

* Note that only active channels can be injected. - * - * @return TRUE if we injected the channel, false if we could not inject or it was already injected. */ - boolean inject(); - - void uninject(); + void inject(); /** * Close the current injector. @@ -46,16 +45,7 @@ public interface Injector { void receiveClientPacket(Object packet); - /** - * Retrieve the current protocol state. - * - * @return The current protocol. - * @deprecated use {@link #getCurrentProtocol(PacketType.Sender)} instead. - */ - @Deprecated - default Protocol getCurrentProtocol() { - return this.getCurrentProtocol(PacketType.Sender.SERVER); - } + void sendWirePacket(WirePacket packet); /** * Retrieve the current protocol state. Note that since 1.20.2 the client and server direction can be in different @@ -66,21 +56,7 @@ default Protocol getCurrentProtocol() { */ Protocol getCurrentProtocol(PacketType.Sender sender); - /** - * Retrieve the network marker associated with a given packet. - * - * @param packet - the packet. - * @return The network marker. - */ - NetworkMarker getMarker(Object packet); - - /** - * Associate a given network marker with a specific packet. - * - * @param packet - the NMS packet. - * @param marker - the associated marker. - */ - void saveMarker(Object packet, NetworkMarker marker); + SocketAddress getAddress(); /** * Retrieve the current player or temporary player associated with the injector. @@ -98,6 +74,8 @@ default Protocol getCurrentProtocol() { void disconnect(String message); + boolean isConnected(); + /** * Determine if the channel has already been injected. * diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/EmptyInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/EmptyInjector.java index 7b60d6f79..a75440a96 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/EmptyInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/EmptyInjector.java @@ -1,10 +1,14 @@ package com.comphenix.protocol.injector.netty.channel; +import java.net.SocketAddress; + +import org.bukkit.entity.Player; + import com.comphenix.protocol.PacketType; import com.comphenix.protocol.PacketType.Protocol; import com.comphenix.protocol.events.NetworkMarker; import com.comphenix.protocol.injector.netty.Injector; -import org.bukkit.entity.Player; +import com.comphenix.protocol.injector.netty.WirePacket; final class EmptyInjector implements Injector { @@ -17,17 +21,20 @@ public EmptyInjector(Player player) { } @Override - public int getProtocolVersion() { - return Integer.MIN_VALUE; + public SocketAddress getAddress() { + if (this.player != null) { + return this.player.getAddress(); + } + return null; } @Override - public boolean inject() { - return false; + public int getProtocolVersion() { + return Integer.MIN_VALUE; } @Override - public void uninject() { + public void inject() { } @Override @@ -43,17 +50,12 @@ public void receiveClientPacket(Object packet) { } @Override - public Protocol getCurrentProtocol(PacketType.Sender sender) { - return Protocol.HANDSHAKING; - } - - @Override - public NetworkMarker getMarker(Object packet) { - return null; + public void sendWirePacket(WirePacket packet) { } @Override - public void saveMarker(Object packet, NetworkMarker marker) { + public Protocol getCurrentProtocol(PacketType.Sender sender) { + return Protocol.HANDSHAKING; } @Override @@ -70,6 +72,11 @@ public void setPlayer(Player player) { public void disconnect(String message) { } + @Override + public boolean isConnected() { + return false; + } + @Override public boolean isInjected() { return false; diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/InboundPacketInterceptor.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/InboundPacketInterceptor.java index 96887c461..1bcf87d65 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/InboundPacketInterceptor.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/InboundPacketInterceptor.java @@ -2,7 +2,8 @@ import com.comphenix.protocol.PacketType; import com.comphenix.protocol.ProtocolLogger; -import com.comphenix.protocol.injector.netty.ChannelListener; +import com.comphenix.protocol.PacketType.Protocol; +import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.injector.packet.PacketRegistry; import com.comphenix.protocol.utility.MinecraftReflection; @@ -12,39 +13,38 @@ final class InboundPacketInterceptor extends ChannelInboundHandlerAdapter { private final NettyChannelInjector injector; - private final ChannelListener channelListener; - public InboundPacketInterceptor(NettyChannelInjector injector, ChannelListener listener) { + public InboundPacketInterceptor(NettyChannelInjector injector) { this.injector = injector; - this.channelListener = listener; } @Override public void channelRead(ChannelHandlerContext ctx, Object msg) { if (MinecraftReflection.isPacketClass(msg)) { - // process the login if the packet is one before posting the packet to any - // handler to provide "real" data the method invocation will do nothing if the - // packet is not a login packet - this.injector.tryProcessLogin(msg); - + // try get packet type PacketType.Protocol protocol = this.injector.getInboundProtocol(); - PacketType packetType = PacketRegistry.getPacketType(protocol, msg.getClass()); + if (protocol == Protocol.UNKNOWN) { + ProtocolLogger.debug("skipping unknown inbound protocol for {0}", msg.getClass()); + ctx.fireChannelRead(msg); + return; + } + PacketType packetType = PacketRegistry.getPacketType(protocol, msg.getClass()); if (packetType == null) { ProtocolLogger.debug("skipping unknown inbound packet type for {0}", msg.getClass()); ctx.fireChannelRead(msg); return; } - // check if there are any listeners bound for the packet - if not just post the + // check if there are any listeners bound for the packet - if not just send the // packet down the pipeline - if (!this.channelListener.hasInboundListener(packetType)) { + if (!this.injector.hasInboundListener(packetType)) { ctx.fireChannelRead(msg); return; } - // call all inbound listeners - this.injector.processInboundPacket(ctx, msg, packetType); + // don't invoke next handler and transfer packet to injector for further processing + this.injector.processInbound(ctx, new PacketContainer(packetType, msg)); } else { // just pass the message down the pipeline ctx.fireChannelRead(msg); diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/InjectionFactory.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/InjectionFactory.java index 1bd0f14d0..68ae5a8cd 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/InjectionFactory.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/InjectionFactory.java @@ -14,21 +14,25 @@ */ package com.comphenix.protocol.injector.netty.channel; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentMap; + +import javax.annotation.Nonnull; + +import org.bukkit.entity.Player; +import org.bukkit.plugin.Plugin; + import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.injector.netty.ChannelListener; import com.comphenix.protocol.injector.netty.Injector; -import com.comphenix.protocol.injector.temporary.MinimalInjector; import com.comphenix.protocol.injector.temporary.TemporaryPlayerFactory; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.utility.MinecraftFields; import com.comphenix.protocol.utility.MinecraftReflection; import com.google.common.collect.MapMaker; + import io.netty.channel.Channel; -import java.util.concurrent.ConcurrentMap; -import javax.annotation.Nonnull; -import org.bukkit.Server; -import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; +import io.netty.channel.ChannelHandler; /** * Represents an injector factory. @@ -39,23 +43,22 @@ */ public class InjectionFactory { - // This should work as long as the injectors are, uh, injected private final ConcurrentMap nameLookup = new MapMaker().weakValues().makeMap(); private final ConcurrentMap playerLookup = new MapMaker().weakKeys().weakValues().makeMap(); // bukkit stuff private final Plugin plugin; - private final Server server; // protocol lib stuff + private final ChannelListener channelListener; private final ErrorReporter errorReporter; // state of the factory private boolean closed; - public InjectionFactory(Plugin plugin, Server server, ErrorReporter errorReporter) { + public InjectionFactory(Plugin plugin, ChannelListener channelListener, ErrorReporter errorReporter) { this.plugin = plugin; - this.server = server; + this.channelListener = channelListener; this.errorReporter = errorReporter; } @@ -72,11 +75,10 @@ public Plugin getPlugin() { * Construct or retrieve a channel injector from an existing Bukkit player. * * @param player - the existing Bukkit player. - * @param listener - the listener. * @return A new injector, an existing injector associated with this player, or a closed injector. */ @Nonnull - public Injector fromPlayer(Player player, ChannelListener listener) { + public Injector fromPlayer(Player player) { if (this.closed) { return new EmptyInjector(player); } @@ -114,10 +116,9 @@ public Injector fromPlayer(Player player, ChannelListener listener) { // construct a new injector as it seems like we have none yet injector = new NettyChannelInjector( player, - this.server, networkManager, channel, - listener, + this.channelListener, this, this.errorReporter); this.cacheInjector(player, injector); @@ -155,31 +156,27 @@ public Injector fromName(String name, Player player) { * Construct a new channel injector for the given channel. * * @param channel - the channel. - * @param listener - the listener. - * @param playerFactory - a temporary player creator. * @return The channel injector, or a closed injector. */ @Nonnull - public Injector fromChannel(Channel channel, ChannelListener listener, TemporaryPlayerFactory playerFactory) { + public Injector fromChannel(Channel channel) { if (this.closed) { return EmptyInjector.WITHOUT_PLAYER; } - Object netManager = this.findNetworkManager(channel); - Player temporaryPlayer = playerFactory.createTemporaryPlayer(this.server); + Object networkManager = this.findNetworkManager(channel); + Player temporaryPlayer = TemporaryPlayerFactory.createTemporaryPlayer(); NettyChannelInjector injector = new NettyChannelInjector( temporaryPlayer, - this.server, - netManager, + networkManager, channel, - listener, + this.channelListener, this, this.errorReporter); - MinimalInjector minimalInjector = new NettyChannelMinimalInjector(injector); // Initialize temporary player - TemporaryPlayerFactory.setInjectorInPlayer(temporaryPlayer, minimalInjector); + TemporaryPlayerFactory.setInjectorForPlayer(temporaryPlayer, injector); return injector; } @@ -235,9 +232,9 @@ public Injector cacheInjector(String name, Injector injector) { * @return The associated injector, or NULL if this is a Bukkit player. */ private NettyChannelInjector getTemporaryInjector(Player player) { - MinimalInjector injector = TemporaryPlayerFactory.getInjectorFromPlayer(player); - if (injector instanceof NettyChannelMinimalInjector) { - return ((NettyChannelMinimalInjector) injector).getInjector(); + Injector injector = TemporaryPlayerFactory.getInjectorFromPlayer(player); + if (injector instanceof NettyChannelInjector) { + return (NettyChannelInjector) injector; } return null; @@ -250,11 +247,12 @@ private NettyChannelInjector getTemporaryInjector(Player player) { * @return The network manager. */ private Object findNetworkManager(Channel channel) { - // Find the network manager - Object networkManager = NettyChannelInjector.findChannelHandler(channel, - MinecraftReflection.getNetworkManagerClass()); - if (networkManager != null) { - return networkManager; + Class networkManagerClass = MinecraftReflection.getNetworkManagerClass(); + + for (Entry entry : channel.pipeline()) { + if (networkManagerClass.isAssignableFrom(entry.getValue().getClass())) { + return entry.getValue(); + } } throw new IllegalArgumentException("Unable to find NetworkManager in " + channel); diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java index b9e1da77c..d0e0d8392 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java @@ -2,14 +2,19 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.net.SocketAddress; import java.util.Map; -import java.util.Map.Entry; -import java.util.NoSuchElementException; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicBoolean; + +import javax.annotation.Nullable; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; import com.comphenix.protocol.PacketType; import com.comphenix.protocol.PacketType.Protocol; @@ -24,10 +29,12 @@ import com.comphenix.protocol.injector.NetworkProcessor; import com.comphenix.protocol.injector.netty.ChannelListener; import com.comphenix.protocol.injector.netty.Injector; +import com.comphenix.protocol.injector.netty.WirePacket; import com.comphenix.protocol.injector.packet.PacketRegistry; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.accessors.Accessors; import com.comphenix.protocol.reflect.accessors.FieldAccessor; +import com.comphenix.protocol.reflect.accessors.MethodAccessor; import com.comphenix.protocol.reflect.fuzzy.FuzzyFieldContract; import com.comphenix.protocol.utility.ByteBuddyGenerated; import com.comphenix.protocol.utility.MinecraftFields; @@ -35,62 +42,45 @@ import com.comphenix.protocol.utility.MinecraftProtocolVersion; import com.comphenix.protocol.utility.MinecraftReflection; import com.comphenix.protocol.utility.MinecraftVersion; +import com.comphenix.protocol.wrappers.WrappedChatComponent; import com.comphenix.protocol.wrappers.WrappedGameProfile; import io.netty.channel.Channel; -import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; import io.netty.channel.EventLoop; import io.netty.util.AttributeKey; -import org.bukkit.Server; -import org.bukkit.entity.Player; public class NettyChannelInjector implements Injector { - // an accessor used when we're unable to retrieve the actual packet field in an outbound packet send - private static final FieldAccessor NO_OP_ACCESSOR = new FieldAccessor() { - @Override - public Object get(Object instance) { - return null; - } - - @Override - public void set(Object instance, Object value) { - } - - @Override - public Field getField() { - return null; - } - }; - - private static final String INTERCEPTOR_NAME = "protocol_lib_inbound_interceptor"; - private static final String PROTOCOL_READER_NAME = "protocol_lib_protocol_reader"; + private static final String INBOUND_INTERCEPTOR_NAME = "protocol_lib_inbound_interceptor"; + private static final String INBOUND_PROTOCOL_GETTER_NAME = "protocol_lib_inbound_protocol_getter"; private static final String WIRE_PACKET_ENCODER_NAME = "protocol_lib_wire_packet_encoder"; // all registered channel handlers to easier make sure we unregister them all from the pipeline - private static final String[] PROTOCOL_LIB_HANDLERS = new String[]{ - WIRE_PACKET_ENCODER_NAME, INTERCEPTOR_NAME, PROTOCOL_READER_NAME + private static final String[] NETTY_HANDLER_NAMES = new String[]{ + WIRE_PACKET_ENCODER_NAME, INBOUND_INTERCEPTOR_NAME, INBOUND_PROTOCOL_GETTER_NAME }; private static final ReportType REPORT_CANNOT_SEND_PACKET = new ReportType("Unable to send packet %s to %s"); + private static final ReportType REPORT_CANNOT_SEND_WRITE_PACKET = new ReportType("Unable to send wire packet %s to %s"); + private static final ReportType REPORT_CANNOT_READ_PACKET = new ReportType("Unable to read packet %s for %s"); + private static final ReportType REPORT_CANNOT_DISCONNECT = new ReportType("Unable to disconnect %s for %s"); private static final WirePacketEncoder WIRE_PACKET_ENCODER = new WirePacketEncoder(); private static final Map, FieldAccessor> PACKET_ACCESSORS = new ConcurrentHashMap<>(16, 0.9f); - private static final Class LOGIN_PACKET_START_CLASS = PacketType.Login.Client.START.getPacketClass(); - private static final Class PACKET_PROTOCOL_CLASS = PacketType.Handshake.Client.SET_PROTOCOL.getPacketClass(); - + // use random attribute name because they need to be unique and would throw on reload private static final AttributeKey PROTOCOL_VERSION = AttributeKey.valueOf(getRandomKey()); private static final AttributeKey INJECTOR = AttributeKey.valueOf(getRandomKey()); - // lazy initialized fields, if we don't need them we don't bother about them - private static FieldAccessor LOGIN_PROFILE_ACCESSOR; - private static FieldAccessor PROTOCOL_VERSION_ACCESSOR; + static NettyChannelInjector findInjector(Channel channel) { + return channel.attr(INJECTOR).get(); + } - // bukkit stuff - private final Server server; + private static String getRandomKey() { + return "ProtocolLib-" + ThreadLocalRandom.current().nextLong(); + } // protocol lib stuff we need private final ErrorReporter errorReporter; @@ -98,7 +88,7 @@ public Field getField() { // references private final Object networkManager; - private final Channel wrappedChannel; + private final Channel channel; private final ChannelListener channelListener; private final InjectionFactory injectionFactory; @@ -110,47 +100,44 @@ public Field getField() { protected final ThreadLocal processedPackets = ThreadLocal.withInitial(() -> Boolean.FALSE); // status of this injector - private volatile boolean closed = false; + private final AtomicBoolean closed = new AtomicBoolean(false); private volatile boolean injected = false; // information about the player belonging to this injector private String playerName; - private Player resolvedPlayer; + private Player player; // lazy initialized fields, if we don't need them we don't bother about them - private Object playerConnection; - - private InboundProtocolReader inboundProtocolReader; + private volatile Object playerConnection; + private volatile InboundProtocolReader inboundProtocolReader; public NettyChannelInjector( Player player, - Server server, - Object netManager, + Object networkManager, Channel channel, ChannelListener listener, InjectionFactory injector, ErrorReporter errorReporter ) { // bukkit stuff - this.server = server; - this.resolvedPlayer = player; + this.player = player; // protocol lib stuff this.errorReporter = errorReporter; this.networkProcessor = new NetworkProcessor(errorReporter); // references - this.networkManager = netManager; - this.wrappedChannel = channel; + this.networkManager = networkManager; + this.channel = channel; this.channelListener = listener; this.injectionFactory = injector; // register us into the channel - this.wrappedChannel.attr(INJECTOR).set(this); + this.channel.attr(INJECTOR).set(this); // read the channel field from the network manager given to this method // we re-read this field every time as plugins/spigot forks might give us different network manager types - Field channelField = FuzzyReflection.fromObject(netManager, true).getField(FuzzyFieldContract.newBuilder() + Field channelField = FuzzyReflection.fromObject(networkManager, true).getField(FuzzyFieldContract.newBuilder() .typeExact(Channel.class) .banModifier(Modifier.STATIC) .build()); @@ -159,135 +146,110 @@ public NettyChannelInjector( // hook here into the close future to be 100% sure that this injector gets closed when the channel we wrap gets closed // normally we listen to the disconnect event, but there is a very small period of time, between the login and actual // join that is not covered by the disconnect event and may lead to unexpected injector states... - this.wrappedChannel.closeFuture().addListener(future -> this.close()); + this.channel.closeFuture().addListener(future -> this.close()); } - static NettyChannelInjector findInjector(Channel channel) { - return channel.attr(INJECTOR).get(); - } - - static Object findChannelHandler(Channel channel, Class type) { - for (Entry entry : channel.pipeline()) { - if (type.isAssignableFrom(entry.getValue().getClass())) { - return entry.getValue(); - } - } - return null; - } - - private static String getRandomKey() { - return "ProtocolLib-" + ThreadLocalRandom.current().nextLong(); - } - - private static boolean hasProtocolLibHandler(Channel channel) { - for (String handler : PROTOCOL_LIB_HANDLERS) { - if (channel.pipeline().get(handler) != null) { - return true; - } - } - return false; + @Override + public SocketAddress getAddress() { + return this.channel.remoteAddress(); } @Override public int getProtocolVersion() { - Integer protocolVersion = this.wrappedChannel.attr(PROTOCOL_VERSION).get(); + Integer protocolVersion = this.channel.attr(PROTOCOL_VERSION).get(); return protocolVersion == null ? MinecraftProtocolVersion.getCurrentVersion() : protocolVersion; } @Override - public boolean inject() { - // we only do this on the channel event loop to prevent blocking the main server thread - // and to be sure that the netty pipeline view we get is up-to-date - if (this.wrappedChannel.eventLoop().inEventLoop()) { - // ensure that we should actually inject into the channel - if (this.closed || this.wrappedChannel instanceof ByteBuddyGenerated || !this.wrappedChannel.isActive()) { - return false; - } + public void inject() { + // ensure we are in the channel's event loop to be "thread-safe" within netty + // and our own code + if (!this.channel.eventLoop().inEventLoop()) { + channel.eventLoop().execute(this::inject); + return; + } - // check here if we need to rewrite the channel field and do so - // minecraft overrides the channel field when the channel actually becomes active, so we need to ensure that our - // proxied channel is always on that field - therefore this rewrite is event before we check if we're already - // injected into the channel - this.rewriteChannelField(); + // don't need to inject if injector or channel got closed + if (this.closed.get() || !this.channel.isActive()) { + return; + } - // check if we already injected into the channel - if (hasProtocolLibHandler(this.wrappedChannel)) { - return false; - } + // wrap channel inside the NetworkMananger to proxy write calls to our + // processOutbound method, this way we try to minimize the amount of thread + // jumps from event loop to minecraft main thread in order to process packets in + // main thread + Object networkManangerChannel = this.channelField.get(this.networkManager); + if (!(networkManangerChannel instanceof NettyChannelProxy)) { + EventLoop proxyEventLoop = new NettyEventLoopProxy(this.channel.eventLoop(), this); + Channel proxyChannel = new NettyChannelProxy(this.channel, proxyEventLoop, this); + this.channelField.set(this.networkManager, proxyChannel); + } - ChannelPipeline pipeline = this.wrappedChannel.pipeline(); - - // since 1.20.5 the encoder is renamed to outbound_config when the channel is - // waiting for the next protocol phase - String encoderName = pipeline.get("outbound_config") != null - ? "outbound_config" : "encoder"; - - // inject our handlers - pipeline.addAfter( - encoderName, - WIRE_PACKET_ENCODER_NAME, - WIRE_PACKET_ENCODER); - - // since 1.20.2 the packet decoder will remove or reconfigure the protocol for - // terminal packets which is why we need to read it before the packet gets - // decoded - if (MinecraftVersion.CONFIG_PHASE_PROTOCOL_UPDATE.atOrAbove()) { - this.inboundProtocolReader = new InboundProtocolReader(this); - pipeline.addBefore( - "decoder", - PROTOCOL_READER_NAME, - this.inboundProtocolReader); - } + ChannelPipeline pipeline = this.channel.pipeline(); - // inject inbound packet interceptor - pipeline.addAfter( - "decoder", - INTERCEPTOR_NAME, - new InboundPacketInterceptor(this, this.channelListener)); + // since 1.20.5 the en-/decoder is renamed to out-/inbound_config when the + // channel is waiting for the next protocol phase (after terminal packet) + String encoderName = pipeline.get("outbound_config") != null + ? "outbound_config" + : "encoder"; + String decoderName = pipeline.get("inbound_config") != null + ? "inbound_config" + : "decoder"; - this.injected = true; - return true; - } else { - // re-run in event loop, return false as we cannot be sure if the injection actually worked - this.ensureInEventLoop(this::inject); - return false; + // try to add wire packet encoder + if (pipeline.context(WIRE_PACKET_ENCODER_NAME) == null) { + pipeline.addAfter(encoderName, WIRE_PACKET_ENCODER_NAME, WIRE_PACKET_ENCODER); } + + // try to add protocol reader, this is necessary because the en-/decoder will + // remove or reconfigure the protocol for terminal since 1.20.2+ + if (MinecraftVersion.CONFIG_PHASE_PROTOCOL_UPDATE.atOrAbove() && pipeline.context(INBOUND_PROTOCOL_GETTER_NAME) == null) { + this.inboundProtocolReader = new InboundProtocolReader(this); + pipeline.addBefore(decoderName, INBOUND_PROTOCOL_GETTER_NAME, this.inboundProtocolReader); + } + + // try to add inbound packet interceptor + if (pipeline.context(INBOUND_INTERCEPTOR_NAME) == null) { + pipeline.addAfter(decoderName, INBOUND_INTERCEPTOR_NAME, new InboundPacketInterceptor(this)); + } + + // mark injector as injected + this.injected = true; } - @Override - public void uninject() { - // ensure that we injected into the channel before trying to remove anything from it - if (this.injected) { - // uninject on the event loop to ensure the instant visibility of the change and prevent blocks of other threads - if (this.wrappedChannel.eventLoop().inEventLoop()) { - this.injected = false; - - // remove known references to us - this.wrappedChannel.attr(INJECTOR).remove(); - this.channelField.set(this.networkManager, this.wrappedChannel); - - for (String handler : PROTOCOL_LIB_HANDLERS) { - try { - this.wrappedChannel.pipeline().remove(handler); - } catch (NoSuchElementException ignored) { - // ignore that one, probably an edge case - } - } - } else { - this.ensureInEventLoop(this::uninject); + private void uninject() { + // ensure we are in the channel's event loop to be "thread-safe" within netty + // and our own code + if (!this.channel.eventLoop().inEventLoop()) { + channel.eventLoop().execute(this::uninject); + return; + } + + // replace wrapped channel in NetworkManager with original channel + this.channelField.set(this.networkManager, this.channel); + + // remove all ProtocolLib netty handler + ChannelPipeline pipeline = this.channel.pipeline(); + for (String handlerName : NETTY_HANDLER_NAMES) { + if (pipeline.context(handlerName) != null) { + pipeline.remove(handlerName); } } + + // mark injector as uninjected + this.injected = false; } @Override public void close() { // ensure that the injector wasn't close before - if (!this.closed) { - this.closed = true; - + if (this.closed.compareAndSet(false, true)) { // remove all of our references from the channel this.uninject(); + // remove any outgoing references + this.channel.attr(INJECTOR).remove(); + // cleanup this.savedMarkers.clear(); this.skippedPackets.clear(); @@ -299,8 +261,8 @@ public void close() { @Override public void sendServerPacket(Object packet, NetworkMarker marker, boolean filtered) { - // do not send the packet if this injector was already closed / is not injected yet - if (this.closed || !this.injected) { + // ignore call if the injector is closed or not injected + if (this.closed.get() || !this.injected) { return; } @@ -310,18 +272,18 @@ public void sendServerPacket(Object packet, NetworkMarker marker, boolean filter } // save the given packet marker and send the packet - this.saveMarker(packet, marker); + if (marker != null) { + this.savedMarkers.put(packet, marker); + } + try { - if (this.resolvedPlayer instanceof ByteBuddyGenerated) { - MinecraftMethods.getNetworkManagerHandleMethod().invoke(this.networkManager, packet); + Object playerConnection = this.getPlayerConnection(); + + // try to use the player connection if possible + if (playerConnection != null) { + MinecraftMethods.getPlayerConnectionSendMethod().invoke(playerConnection, packet); } else { - // ensure that the player is properly connected before sending - Object playerConnection = this.getPlayerConnection(); - if (playerConnection != null) { - MinecraftMethods.getSendPacketMethod().invoke(playerConnection, packet); - } else { - MinecraftMethods.getNetworkManagerHandleMethod().invoke(this.networkManager, packet); - } + MinecraftMethods.getNetworkManagerSendMethod().invoke(this.networkManager, packet); } } catch (Exception exception) { this.errorReporter.reportWarning(this, Report.newBuilder(REPORT_CANNOT_SEND_PACKET) @@ -333,90 +295,104 @@ public void sendServerPacket(Object packet, NetworkMarker marker, boolean filter @Override public void receiveClientPacket(Object packet) { - // do not do that if we're not injected or this injector was closed - if (this.closed || !this.injected) { + // ignore call if the injector is closed or not injected + if (this.closed.get() || !this.injected) { return; } - - Runnable receiveAction = () -> { + + this.ensureInEventLoop(() -> { try { // try to invoke the method, this should normally not fail MinecraftMethods.getNetworkManagerReadPacketMethod().invoke(this.networkManager, null, packet); } catch (Exception exception) { - // 99% the user gave wrong information to the server - this.errorReporter.reportMinimal(this.injectionFactory.getPlugin(), "receiveClientPacket", exception); + this.errorReporter.reportWarning(this, Report.newBuilder(REPORT_CANNOT_READ_PACKET) + .messageParam(packet, this.playerName) + .error(exception) + .build()); } - }; - - // execute the action on the event loop rather than any thread which we should potentially not block - if (this.wrappedChannel.eventLoop().inEventLoop()) { - receiveAction.run(); - } else { - this.ensureInEventLoop(receiveAction); - } - } - - public PacketType.Protocol getInboundProtocol() { - if (this.inboundProtocolReader != null) { - return this.inboundProtocolReader.getProtocol(); - } - return getCurrentProtocol(PacketType.Sender.CLIENT); + }); } @Override - public Protocol getCurrentProtocol(PacketType.Sender sender) { - return ChannelProtocolUtil.PROTOCOL_RESOLVER.apply(this.wrappedChannel, sender); - } + public void sendWirePacket(WirePacket packet) { + // ignore call if the injector is closed or not injected + if (this.closed.get() || !this.injected) { + return; + } - @Override - public NetworkMarker getMarker(Object packet) { - return this.savedMarkers.get(packet); + this.ensureInEventLoop(() -> { + try { + this.channel.writeAndFlush(packet); + } catch (Exception exception) { + this.errorReporter.reportWarning(this, Report.newBuilder(REPORT_CANNOT_SEND_WRITE_PACKET) + .messageParam(packet, this.playerName) + .error(exception) + .build()); + } + }); } @Override - public void saveMarker(Object packet, NetworkMarker marker) { - if (marker != null && !this.closed) { - this.savedMarkers.put(packet, marker); - } + public Protocol getCurrentProtocol(PacketType.Sender sender) { + return ChannelProtocolUtil.PROTOCOL_RESOLVER.apply(this.channel, sender); } @Override public Player getPlayer() { // if the player was already resolved there is no need to do further lookups - if (this.resolvedPlayer != null) { - return this.resolvedPlayer; + if (this.player != null) { + return this.player; } // check if the name of the player is already known to the injector if (this.playerName != null) { - this.resolvedPlayer = this.server.getPlayerExact(this.playerName); + this.player = Bukkit.getPlayerExact(this.playerName); } // either we resolved it or we didn't... - return this.resolvedPlayer; + return this.player; } @Override public void setPlayer(Player player) { - this.resolvedPlayer = player; + this.player = player; this.playerName = player.getName(); } @Override public void disconnect(String message) { - // we're still during pre-login, just close the connection - if (this.playerConnection == null || this.resolvedPlayer instanceof ByteBuddyGenerated) { - this.wrappedChannel.disconnect(); - } else { - try { - // try to call the disconnect method on the player - MinecraftMethods.getDisconnectMethod(this.playerConnection.getClass()).invoke(this.playerConnection, message); - } catch (Exception exception) { - throw new IllegalArgumentException("Unable to disconnect the current injector", exception); + try { + Object playerConnection = this.getPlayerConnection(); + + // try to use the player connection if possible + if (playerConnection != null) { + // this method prefers the main thread so no event loop here + MethodAccessor accessor = MinecraftMethods.getPlayerConnectionDisconnectMethod(); + + // check if the parameter is a chat component + if (MinecraftReflection.isIChatBaseComponent(accessor.getMethod().getParameters()[0].getClass())) { + Object component = WrappedChatComponent.fromText(message).getHandle(); + accessor.invoke(playerConnection, component); + } else { + accessor.invoke(playerConnection, message); + } + } else { + Object component = WrappedChatComponent.fromText(message).getHandle(); + MinecraftMethods.getNetworkManagerDisconnectMethod().invoke(this.networkManager, component); } + } catch (Exception exception) { + this.errorReporter.reportWarning(this, Report.newBuilder(REPORT_CANNOT_DISCONNECT) + .messageParam(this.playerName, message) + .error(exception) + .build()); } } + @Override + public boolean isConnected() { + return this.channel.isActive(); + } + @Override public boolean isInjected() { return this.injected; @@ -424,139 +400,100 @@ public boolean isInjected() { @Override public boolean isClosed() { - return this.closed; + return this.closed.get(); } - void tryProcessLogin(Object packet) { - // check if the given packet is a login packet - if (LOGIN_PACKET_START_CLASS != null && LOGIN_PACKET_START_CLASS.equals(packet.getClass())) { - if (MinecraftVersion.WILD_UPDATE.atOrAbove()) { - // 1.19 removed the profile from the packet and now sends the plain username directly - // ensure that the game profile accessor is available - if (LOGIN_PROFILE_ACCESSOR == null) { - LOGIN_PROFILE_ACCESSOR = Accessors.getFieldAccessor(LOGIN_PACKET_START_CLASS, String.class, true); - } - - // get the username from the packet - String username = (String) LOGIN_PROFILE_ACCESSOR.get(packet); - - // cache the injector and the player name - this.playerName = username; - this.injectionFactory.cacheInjector(username, this); - } else { - // ensure that the game profile accessor is available - if (LOGIN_PROFILE_ACCESSOR == null) { - LOGIN_PROFILE_ACCESSOR = Accessors.getFieldAccessor( - LOGIN_PACKET_START_CLASS, - MinecraftReflection.getGameProfileClass(), - true); - } - - // the client only sends the name but the server wraps it into a GameProfile, so here we are - WrappedGameProfile profile = WrappedGameProfile.fromHandle(LOGIN_PROFILE_ACCESSOR.get(packet)); - - // cache the injector and the player name - this.playerName = profile.getName(); - this.injectionFactory.cacheInjector(profile.getName(), this); - } - - return; + PacketType.Protocol getInboundProtocol() { + if (this.inboundProtocolReader != null) { + return this.inboundProtocolReader.getProtocol(); } + return getCurrentProtocol(PacketType.Sender.CLIENT); + } - // protocol version begin - if (PACKET_PROTOCOL_CLASS != null && PACKET_PROTOCOL_CLASS.equals(packet.getClass())) { - // ensure the protocol version accessor is available - if (PROTOCOL_VERSION_ACCESSOR == null) { - try { - Field ver = FuzzyReflection.fromClass(PACKET_PROTOCOL_CLASS, true).getField(FuzzyFieldContract.newBuilder() - .banModifier(Modifier.STATIC) - .typeExact(int.class) - .build()); - PROTOCOL_VERSION_ACCESSOR = Accessors.getFieldAccessor(ver); - } catch (IllegalArgumentException exception) { - // unable to resolve that field, continue no-op - PROTOCOL_VERSION_ACCESSOR = NO_OP_ACCESSOR; - } - } - - // read the protocol version from the field if available - if (PROTOCOL_VERSION_ACCESSOR != NO_OP_ACCESSOR) { - int protocolVersion = (int) PROTOCOL_VERSION_ACCESSOR.get(packet); - this.wrappedChannel.attr(PROTOCOL_VERSION).set(protocolVersion); - } - } + /** + * Returns true if any plugin registed listeners or built-in listeners for the + * given packet type exist. See {@link #processInbound(ChannelHandlerContext, PacketContainer)} + * + * @param type the packet type + * @return true if the packet type has listeners; otherwise false + */ + boolean hasInboundListener(PacketType type) { + // always return true for types of the built-in listener (see #processInbound) + return type == PacketType.Handshake.Client.SET_PROTOCOL || + type == PacketType.Login.Client.START || + // check for plugin registed listener + this.channelListener.hasInboundListener(type); } - private void rewriteChannelField() { - // check if we need to rewrite the channel or if the channel is already correct (prevent wrapping a wrapped channel) - Object currentChannel = this.channelField.get(this.networkManager); - if (currentChannel instanceof NettyChannelProxy) { - return; + void processInbound(ChannelHandlerContext ctx, PacketContainer packet) { + // process set protocol packets for the protocol version + if (packet.getType() == PacketType.Handshake.Client.SET_PROTOCOL) { + Integer protocolVersion = packet.getIntegers().readSafely(0); + if (protocolVersion != null) { + this.channel.attr(PROTOCOL_VERSION).set(protocolVersion); + } } - // the field is not correct, rewrite now to our handler - Channel ch = new NettyChannelProxy(this.wrappedChannel, new NettyEventLoopProxy(this.wrappedChannel.eventLoop(), this) { - @Override - protected Runnable doProxyRunnable(Runnable original) { - return NettyChannelInjector.this.processOutbound(original); + // process login packets for the player name + if (packet.getType() == PacketType.Login.Client.START) { + String username; + if (MinecraftVersion.WILD_UPDATE.atOrAbove()) { + // 1.19 replaced the gameprofile with username and uuid field + username = packet.getStrings().readSafely(0); + } else { + WrappedGameProfile profile = packet.getGameProfiles().readSafely(0); + username = profile != null ? profile.getName() : null; } - @Override - protected Callable doProxyCallable(Callable original) { - return NettyChannelInjector.this.processOutbound(original); + if (username != null) { + this.playerName = username; + this.injectionFactory.cacheInjector(username, this); } - }, this); - this.channelField.set(this.networkManager, ch); - } - - private void ensureInEventLoop(Runnable runnable) { - this.ensureInEventLoop(this.wrappedChannel.eventLoop(), runnable); - } + } - private void ensureInEventLoop(EventLoop eventLoop, Runnable runnable) { - if (eventLoop.inEventLoop()) { - runnable.run(); + // invoke plugin listeners + if (this.channelListener.hasInboundListener(packet.getType())) { + this.processInboundInternal(ctx, packet); } else { - eventLoop.execute(runnable); + ctx.fireChannelRead(packet.getHandle()); } } - void processInboundPacket(ChannelHandlerContext ctx, Object packet, PacketType packetType) { - if (this.channelListener.hasMainThreadListener(packetType) && !this.server.isPrimaryThread()) { - // not on the main thread but we are required to be - re-schedule the packet on the main thread - ProtocolLibrary.getScheduler().runTask( - () -> this.processInboundPacket(ctx, packet, packetType)); + private void processInboundInternal(ChannelHandlerContext ctx, PacketContainer packetContainer) { + if (this.channelListener.hasMainThreadListener(packetContainer.getType()) && !Bukkit.isPrimaryThread()) { + // not on the main thread but we are required to be reschedule the packet on the + // main thread + ProtocolLibrary.getScheduler().runTask(() -> this.processInboundInternal(ctx, packetContainer)); return; } - // call packet handlers, a null result indicates that we shouldn't change anything - PacketContainer packetContainer = new PacketContainer(packetType, packet); - PacketEvent interceptionResult = this.channelListener.onPacketReceiving(this, packetContainer, null); - if (interceptionResult == null) { - this.ensureInEventLoop(ctx.channel().eventLoop(), () -> ctx.fireChannelRead(packet)); + // call packet handlers, a null result indicates that we shouldn't change + // anything + PacketEvent event = this.channelListener.onPacketReceiving(this, packetContainer, null); + if (event == null) { + this.ensureInEventLoop(ctx.channel().eventLoop(), () -> ctx.fireChannelRead(packetContainer.getHandle())); return; } - // fire the intercepted packet down the pipeline if it wasn't cancelled - if (!interceptionResult.isCancelled()) { - this.ensureInEventLoop( - ctx.channel().eventLoop(), - () -> ctx.fireChannelRead(interceptionResult.getPacket().getHandle())); + Object packet = event.getPacket().getHandle(); + + // fire the intercepted packet down the pipeline if it wasn't cancelled and isn't null + if (!event.isCancelled() && packet != null) { + this.ensureInEventLoop(ctx.channel().eventLoop(), () -> ctx.fireChannelRead(packet)); // check if there were any post events added the packet after we fired it down the pipeline // we use this way as we don't want to construct a new network manager accidentally - NetworkMarker marker = NetworkMarker.getNetworkMarker(interceptionResult); + NetworkMarker marker = NetworkMarker.getNetworkMarker(event); if (marker != null) { - this.networkProcessor.invokePostEvent(interceptionResult, marker); + this.networkProcessor.invokePostEvent(event, marker); } } } T processOutbound(T action) { - // get the accessor to the packet field - // if we are unable to look up the accessor then just return the runnable, probably nothing of our business + // try to get packet field accessor or return on failure FieldAccessor packetAccessor = this.lookupPacketAccessor(action); - if (packetAccessor == NO_OP_ACCESSOR) { + if (packetAccessor == FieldAccessor.NO_OP_ACCESSOR) { return action; } @@ -583,7 +520,7 @@ T processOutbound(T action) { PacketType.Protocol protocol = this.getCurrentProtocol(PacketType.Sender.SERVER); if (protocol == Protocol.UNKNOWN) { - ProtocolLogger.debug("skipping unknown protocol for {0}", packet.getClass()); + ProtocolLogger.debug("skipping unknown outbound protocol for {0}", packet.getClass()); return action; } @@ -599,10 +536,9 @@ T processOutbound(T action) { } // ensure that we are on the main thread if we need to - if (this.channelListener.hasMainThreadListener(packetType) && !this.server.isPrimaryThread()) { + if (this.channelListener.hasMainThreadListener(packetType) && !Bukkit.isPrimaryThread()) { // not on the main thread but we are required to be - re-schedule the packet on the main thread - ProtocolLibrary.getScheduler().runTask( - () -> this.sendServerPacket(packet, null, true)); + ProtocolLibrary.getScheduler().runTask(() -> this.sendServerPacket(packet, null, true)); return null; } @@ -614,10 +550,11 @@ T processOutbound(T action) { return action; } + Object interceptedPacket = event.getPacket().getHandle(); + // if the event wasn't cancelled by this action we must recheck if the packet changed during the method call - if (!event.isCancelled()) { + if (!event.isCancelled() && interceptedPacket != null) { // rewrite the packet in the given action if the packet was changed during the event call - Object interceptedPacket = event.getPacket().getHandle(); if (interceptedPacket != packet) { packetAccessor.set(action, interceptedPacket); } @@ -667,37 +604,53 @@ private T proxyAction(T action, PacketEvent event, NetworkMarker marker) { } private FieldAccessor lookupPacketAccessor(Object action) { - return PACKET_ACCESSORS.computeIfAbsent(action.getClass(), clazz -> { + return PACKET_ACCESSORS.computeIfAbsent(action.getClass(), key -> { try { - // find the field - Field packetField = FuzzyReflection.fromClass(clazz, true).getField(FuzzyFieldContract - .newBuilder() + Field packetField = FuzzyReflection.fromClass(key, true).getField( + FuzzyFieldContract.newBuilder() .typeSuperOf(MinecraftReflection.getPacketClass()) .build()); return Accessors.getFieldAccessor(packetField); } catch (IllegalArgumentException exception) { - // no such field found :( - return NO_OP_ACCESSOR; + // return noop accessor because computeIfAbsent interprets null as a missing key + return FieldAccessor.NO_OP_ACCESSOR; } }); } + /** + * Returns the PlayerConnection or null if the player instance is null or a + * temporary player + * + * @return the PlayerConnection or null + */ + @Nullable private Object getPlayerConnection() { // resolve the player connection if needed if (this.playerConnection == null) { Player target = this.getPlayer(); - if (target == null) { + // if player is null or temporary return null + if (target == null || target instanceof ByteBuddyGenerated) { return null; } + // this can in some cases still return null because the connection isn't set + // during the configuration phase but the player instance got created this.playerConnection = MinecraftFields.getPlayerConnection(target); } - // cannot be null at this point return this.playerConnection; } - public Channel getWrappedChannel() { - return this.wrappedChannel; + private void ensureInEventLoop(Runnable runnable) { + this.ensureInEventLoop(this.channel.eventLoop(), runnable); + } + + private void ensureInEventLoop(EventLoop eventLoop, Runnable runnable) { + if (eventLoop.inEventLoop()) { + runnable.run(); + } else { + eventLoop.execute(runnable); + } } } diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelMinimalInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelMinimalInjector.java deleted file mode 100644 index 6acce8a73..000000000 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelMinimalInjector.java +++ /dev/null @@ -1,44 +0,0 @@ -package com.comphenix.protocol.injector.netty.channel; - -import com.comphenix.protocol.events.NetworkMarker; -import com.comphenix.protocol.injector.temporary.MinimalInjector; -import java.net.SocketAddress; -import org.bukkit.entity.Player; - -final class NettyChannelMinimalInjector implements MinimalInjector { - - private final NettyChannelInjector injector; - - public NettyChannelMinimalInjector(NettyChannelInjector injector) { - this.injector = injector; - } - - @Override - public SocketAddress getAddress() { - return this.injector.getWrappedChannel().remoteAddress(); - } - - @Override - public void disconnect(String message) { - this.injector.disconnect(message); - } - - @Override - public void sendServerPacket(Object packet, NetworkMarker marker, boolean filtered) { - this.injector.sendServerPacket(packet, marker, filtered); - } - - @Override - public Player getPlayer() { - return this.injector.getPlayer(); - } - - @Override - public boolean isConnected() { - return this.injector.getWrappedChannel().isActive(); - } - - public NettyChannelInjector getInjector() { - return this.injector; - } -} diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyEventLoopProxy.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyEventLoopProxy.java index 2232746cd..c5c0457d2 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyEventLoopProxy.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyEventLoopProxy.java @@ -1,5 +1,15 @@ package com.comphenix.protocol.injector.netty.channel; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Spliterator; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; + import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelPromise; @@ -10,21 +20,12 @@ import io.netty.util.concurrent.ProgressivePromise; import io.netty.util.concurrent.Promise; import io.netty.util.concurrent.ScheduledFuture; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.Spliterator; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.function.Consumer; /** * An abstract event loop implementation which delegates all calls to a given event loop, but proxies all calls which * schedule something on the event loop to methods which decide what should happen to the scheduled task. */ -abstract class NettyEventLoopProxy implements EventLoop { +final class NettyEventLoopProxy implements EventLoop { private static final Callable EMPTY_CALLABLE = () -> null; private static final Runnable EMPTY_RUNNABLE = () -> { @@ -40,7 +41,7 @@ public NettyEventLoopProxy(EventLoop delegate, NettyChannelInjector injector) { private Runnable proxyRunnable(Runnable original) { // execute the proxy and check if we need to do anything - Runnable proxied = this.doProxyRunnable(original); + Runnable proxied = this.injector.processOutbound(original); if (proxied != null && proxied == original) { // was not changed, we need to mark the packet as processed manually return () -> { @@ -55,7 +56,7 @@ private Runnable proxyRunnable(Runnable original) { private Callable proxyCallable(Callable original) { // execute the proxy and check if we need to do anything - Callable proxied = this.doProxyCallable(original); + Callable proxied = this.injector.processOutbound(original); if (proxied != null && proxied == original) { // was not changed, we need to mark the packet as processed manually return () -> { @@ -68,26 +69,6 @@ private Callable proxyCallable(Callable original) { } } - /** - * Proxies the given runnable. The returned runnable will be executed instead of the original. If this method returns - * null a no-op runnable will be scheduled instead, preventing the original action from happening. - * - * @param original the runnable to proxy. - * @return the runnable to execute instead, null to execute no action. - */ - protected abstract Runnable doProxyRunnable(Runnable original); - - /** - * Proxies the given callable. The returned callable will be executed instead of the original. If this method returns - * null a callable which always returns null will be scheduled instead, preventing the original action from - * happening. - * - * @param original the callable to proxy. - * @param the return type of the original callable. - * @return the callable to execute instead of the original, null to use a no-op callable instead. - */ - protected abstract Callable doProxyCallable(Callable original); - @Override public EventLoopGroup parent() { return this.delegate.parent(); diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInboundHandler.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInboundHandler.java index 4d2ca1a84..db433e680 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInboundHandler.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInboundHandler.java @@ -3,7 +3,7 @@ import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.ReportType; import com.comphenix.protocol.injector.netty.channel.InjectionFactory; -import com.comphenix.protocol.injector.temporary.TemporaryPlayerFactory; + import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; @@ -13,16 +13,13 @@ final class InjectionChannelInboundHandler extends ChannelInboundHandlerAdapter private final InjectionFactory factory; private final NetworkManagerInjector listener; - private final TemporaryPlayerFactory playerFactory; public InjectionChannelInboundHandler( InjectionFactory factory, - NetworkManagerInjector listener, - TemporaryPlayerFactory playerFactory + NetworkManagerInjector listener ) { this.factory = factory; this.listener = listener; - this.playerFactory = playerFactory; } @Override @@ -37,7 +34,7 @@ public void channelActive(ChannelHandlerContext ctx) { // that point we might accidentally trigger class loads which result in exceptions. if (!this.factory.isClosed()) { try { - this.factory.fromChannel(ctx.channel(), this.listener, this.playerFactory).inject(); + this.factory.fromChannel(ctx.channel()).inject(); } catch (Exception exception) { this.listener.getReporter().reportDetailed(this, Report.newBuilder(CANNOT_INJECT_CHANNEL) .messageParam(ctx.channel()) @@ -52,7 +49,7 @@ public void channelActive(ChannelHandlerContext ctx) { @Override public boolean isSharable() { - // we do it this way to prevent the lookup overheat + // we do it this way to prevent the lookup overhead return true; } } diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInitializer.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInitializer.java index 937cd2fca..ef1195de4 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInitializer.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/manager/InjectionChannelInitializer.java @@ -28,7 +28,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { @Override public boolean isSharable() { - // we do it this way to prevent the lookup overheat + // we do it this way to prevent the lookup overhead return true; } } diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java index e00961b79..2cc316dab 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java @@ -7,7 +7,7 @@ import java.util.List; import java.util.Set; -import org.bukkit.Server; +import org.bukkit.entity.Player; import org.bukkit.plugin.Plugin; import com.comphenix.protocol.PacketType; @@ -20,8 +20,6 @@ import com.comphenix.protocol.injector.netty.ChannelListener; import com.comphenix.protocol.injector.netty.Injector; import com.comphenix.protocol.injector.netty.channel.InjectionFactory; -import com.comphenix.protocol.injector.player.PlayerInjectionHandler; -import com.comphenix.protocol.injector.temporary.TemporaryPlayerFactory; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.accessors.Accessors; import com.comphenix.protocol.reflect.accessors.FieldAccessor; @@ -35,7 +33,6 @@ public class NetworkManagerInjector implements ChannelListener { private static final String INBOUND_INJECT_HANDLER_NAME = "protocol_lib_inbound_inject"; - private static final TemporaryPlayerFactory PLAYER_FACTORY = new TemporaryPlayerFactory(); // all list fields which we've overridden and need to revert to a non-proxying list afterwards private final Set> overriddenLists = new HashSet<>(); @@ -44,9 +41,7 @@ public class NetworkManagerInjector implements ChannelListener { private final ListenerInvoker listenerInvoker; private final InjectionFactory injectionFactory; - // injectors based on this "global" injector - private final PlayerInjectionHandler playerInjectionHandler; - + // netty handler private final InjectionChannelInitializer pipelineInjectorHandler; private boolean debug = false; @@ -55,22 +50,16 @@ public class NetworkManagerInjector implements ChannelListener { private boolean closed = false; private boolean injected = false; - public NetworkManagerInjector(Plugin plugin, Server server, ListenerInvoker listenerInvoker, ErrorReporter reporter) { + public NetworkManagerInjector(Plugin plugin, ListenerInvoker listenerInvoker, ErrorReporter reporter) { this.errorReporter = reporter; this.listenerInvoker = listenerInvoker; - this.injectionFactory = new InjectionFactory(plugin, server, reporter); + this.injectionFactory = new InjectionFactory(plugin, this, reporter); // hooking netty handlers InjectionChannelInboundHandler injectionHandler = new InjectionChannelInboundHandler( this.injectionFactory, - this, - PLAYER_FACTORY); + this); this.pipelineInjectorHandler = new InjectionChannelInitializer(INBOUND_INJECT_HANDLER_NAME, injectionHandler); - - // other injectors - this.playerInjectionHandler = new NetworkManagerPlayerInjector( - this, - this.injectionFactory); } @Override @@ -133,6 +122,10 @@ public void setDebug(boolean debug) { this.debug = debug; } + public Injector getInjector(Player player) { + return this.injectionFactory.fromPlayer(player); + } + @SuppressWarnings("unchecked") public void inject() { if (this.closed || this.injected) { @@ -231,8 +224,4 @@ public void close() { this.overriddenLists.clear(); this.injectionFactory.close(); } - - public PlayerInjectionHandler getPlayerInjectionHandler() { - return this.playerInjectionHandler; - } } diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPlayerInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPlayerInjector.java deleted file mode 100644 index fb47bb79c..000000000 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPlayerInjector.java +++ /dev/null @@ -1,68 +0,0 @@ -package com.comphenix.protocol.injector.netty.manager; - -import org.bukkit.entity.Player; - -import com.comphenix.protocol.events.NetworkMarker; -import com.comphenix.protocol.events.PacketContainer; -import com.comphenix.protocol.injector.netty.ChannelListener; -import com.comphenix.protocol.injector.netty.Injector; -import com.comphenix.protocol.injector.netty.channel.InjectionFactory; -import com.comphenix.protocol.injector.netty.channel.NettyChannelInjector; -import com.comphenix.protocol.injector.player.PlayerInjectionHandler; - -import io.netty.channel.Channel; - -final class NetworkManagerPlayerInjector implements PlayerInjectionHandler { - - private final ChannelListener listener; - private final InjectionFactory injectionFactory; - - public NetworkManagerPlayerInjector( - ChannelListener listener, - InjectionFactory injectionFactory - ) { - this.listener = listener; - this.injectionFactory = injectionFactory; - } - - @Override - public int getProtocolVersion(Player player) { - return this.injectionFactory.fromPlayer(player, this.listener).getProtocolVersion(); - } - - @Override - public void injectPlayer(Player player, ConflictStrategy strategy) { - this.injectionFactory.fromPlayer(player, this.listener).inject(); - } - - @Override - public boolean uninjectPlayer(Player player) { - this.injectionFactory.fromPlayer(player, this.listener).uninject(); - return true; - } - - @Override - public void sendServerPacket(Player receiver, PacketContainer packet, NetworkMarker marker, boolean filters) { - this.injectionFactory.fromPlayer(receiver, this.listener).sendServerPacket(packet.getHandle(), marker, filters); - } - - @Override - public void receiveClientPacket(Player player, Object mcPacket) { - this.injectionFactory.fromPlayer(player, this.listener).receiveClientPacket(mcPacket); - } - - @Override - public void updatePlayer(Player player) { - this.injectionFactory.fromPlayer(player, this.listener).inject(); - } - - @Override - public Channel getChannel(Player player) { - Injector injector = this.injectionFactory.fromPlayer(player, this.listener); - if (injector instanceof NettyChannelInjector) { - return ((NettyChannelInjector) injector).getWrappedChannel(); - } - - return null; - } -} diff --git a/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java deleted file mode 100644 index bcb56399d..000000000 --- a/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ /dev/null @@ -1,81 +0,0 @@ -package com.comphenix.protocol.injector.player; - -import org.bukkit.entity.Player; - -import com.comphenix.protocol.events.NetworkMarker; -import com.comphenix.protocol.events.PacketContainer; - -import io.netty.channel.Channel; - -public interface PlayerInjectionHandler { - - /** - * Retrieve the protocol version of the given player. - * - * @param player - the player. - * @return The protocol version, or {@link Integer#MIN_VALUE}. - */ - int getProtocolVersion(Player player); - - /** - * Initialize a player hook, allowing us to read server packets. - *

- * This call will be ignored if there's no listener that can receive the given events. - * - * @param player - player to hook. - * @param strategy - how to handle injection conflicts. - */ - void injectPlayer(Player player, ConflictStrategy strategy); - - /** - * Uninject the given player. - * - * @param player - player to uninject. - * @return TRUE if a player has been uninjected, FALSE otherwise. - */ - boolean uninjectPlayer(Player player); - - /** - * Send the given packet to the given receiver. - * - * @param receiver - the player receiver. - * @param packet - the packet to send. - * @param marker - network marker. - * @param filters - whether or not to invoke the packet filters. - */ - void sendServerPacket(Player receiver, PacketContainer packet, NetworkMarker marker, boolean filters); - - /** - * Process a packet as if it were sent by the given player. - * - * @param player - the sender. - * @param mcPacket - the packet to process. - */ - void receiveClientPacket(Player player, Object mcPacket); - - /** - * Ensure that packet readers are informed of this player reference. - * - * @param player - the player to update. - */ - void updatePlayer(Player player); - - Channel getChannel(Player player); - - /** - * How to handle a previously existing player injection. - * - * @author Kristian - */ - enum ConflictStrategy { - /** - * Override it. - */ - OVERRIDE, - - /** - * Immediately exit. - */ - BAIL_OUT; - } -} diff --git a/src/main/java/com/comphenix/protocol/injector/temporary/MinimalInjector.java b/src/main/java/com/comphenix/protocol/injector/temporary/MinimalInjector.java deleted file mode 100644 index 3437bccb5..000000000 --- a/src/main/java/com/comphenix/protocol/injector/temporary/MinimalInjector.java +++ /dev/null @@ -1,50 +0,0 @@ -package com.comphenix.protocol.injector.temporary; - -import com.comphenix.protocol.events.NetworkMarker; -import java.net.SocketAddress; -import org.bukkit.entity.Player; - -/** - * Represents an injector that only gives access to a player's socket. - * - * @author Kristian - */ -public interface MinimalInjector { - - /** - * Retrieve the associated address of this player. - * - * @return The associated address. - */ - SocketAddress getAddress(); - - /** - * Attempt to disconnect the current client. - * - * @param message - the message to display. - */ - void disconnect(String message); - - /** - * Send a packet to the client. - * - * @param packet - server packet to send. - * @param marker - the network marker. - * @param filtered - whether or not the packet will be filtered by our listeners. - */ - void sendServerPacket(Object packet, NetworkMarker marker, boolean filtered); - - /** - * Retrieve the hooked player. - * - * @return The hooked player. - */ - Player getPlayer(); - - /** - * Determines if the player is currently connected. - * - * @return true if the player is connected. - */ - boolean isConnected(); -} diff --git a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayer.java b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayer.java index 7bf0ff265..f38167c71 100644 --- a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayer.java +++ b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayer.java @@ -1,5 +1,9 @@ package com.comphenix.protocol.injector.temporary; +import java.util.Objects; + +import com.comphenix.protocol.injector.netty.Injector; + /** * A temporary player created by ProtocolLib when a true player instance does not exist. *

@@ -8,15 +12,17 @@ */ public class TemporaryPlayer { - private volatile MinimalInjector injector; + protected volatile Injector injector; - MinimalInjector getInjector() { + public Injector getInjector() { return this.injector; } - void setInjector(MinimalInjector injector) { - if (injector == null) { - throw new IllegalArgumentException("Injector cannot be NULL."); + void setInjector(Injector injector) { + Objects.requireNonNull(injector, "injector can't be null"); + + if (this.injector != null) { + throw new IllegalStateException("Can't redefine injector for temporary player"); } this.injector = injector; diff --git a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java index bae2f5821..91511f681 100644 --- a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java +++ b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java @@ -17,17 +17,20 @@ package com.comphenix.protocol.injector.temporary; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; + import com.comphenix.protocol.events.PacketContainer; +import com.comphenix.protocol.injector.netty.Injector; import com.comphenix.protocol.utility.ByteBuddyFactory; import com.comphenix.protocol.utility.ChatExtensions; -import java.lang.reflect.Constructor; -import java.lang.reflect.Method; + import net.bytebuddy.description.ByteCodeElement; -import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; -import net.bytebuddy.implementation.FieldAccessor; -import net.bytebuddy.implementation.MethodCall; import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.AllArguments; import net.bytebuddy.implementation.bind.annotation.FieldValue; @@ -36,8 +39,6 @@ import net.bytebuddy.implementation.bind.annotation.This; import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.matcher.ElementMatchers; -import org.bukkit.Server; -import org.bukkit.entity.Player; /** * Create fake player instances that represents pre-authenticated clients. @@ -46,13 +47,15 @@ public class TemporaryPlayerFactory { private static final Constructor PLAYER_CONSTRUCTOR = setupProxyPlayerConstructor(); + private TemporaryPlayerFactory() {} + /** * Retrieve the injector from a given player if it contains one. * * @param player - the player that may contain a reference to a player injector. * @return The referenced player injector, or NULL if none can be found. */ - public static MinimalInjector getInjectorFromPlayer(Player player) { + public static Injector getInjectorFromPlayer(Player player) { if (player instanceof TemporaryPlayer) { return ((TemporaryPlayer) player).getInjector(); } @@ -65,7 +68,7 @@ public static MinimalInjector getInjectorFromPlayer(Player player) { * @param player - the player to update. * @param injector - the injector to store. */ - public static void setInjectorInPlayer(Player player, MinimalInjector injector) { + public static void setInjectorForPlayer(Player player, Injector injector) { ((TemporaryPlayer) player).setInjector(injector); } @@ -76,11 +79,10 @@ private static Constructor setupProxyPlayerConstructor() { public Object delegate( @This Object obj, @Origin Method method, - @FieldValue("server") Server server, + @FieldValue("injector") Injector injector, @AllArguments Object... args ) throws Throwable { String methodName = method.getName(); - MinimalInjector injector = ((TemporaryPlayer) obj).getInjector(); if (injector == null) { throw new IllegalStateException("Unable to find injector."); @@ -92,7 +94,7 @@ else if (methodName.equals("getPlayer")) { } else if (methodName.equals("getAddress")) { return injector.getAddress(); } else if (methodName.equals("getServer")) { - return server; + return Bukkit.getServer(); } // Handle send message methods @@ -144,21 +146,15 @@ else if (methodName.equals("getPlayer")) { try { final Constructor constructor = ByteBuddyFactory.getInstance() - .createSubclass(TemporaryPlayer.class, ConstructorStrategy.Default.NO_CONSTRUCTORS) + .createSubclass(TemporaryPlayer.class, ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR) .name(TemporaryPlayerFactory.class.getPackage().getName() + ".TemporaryPlayerInvocationHandler") .implement(Player.class) - - .defineField("server", Server.class, Visibility.PRIVATE) - .defineConstructor(Visibility.PUBLIC) - .withParameters(Server.class) - .intercept(MethodCall.invoke(TemporaryPlayer.class.getDeclaredConstructor()) - .andThen(FieldAccessor.ofField("server").setsArgumentAt(0))) .method(callbackFilter) .intercept(implementation) .make() .load(ByteBuddyFactory.getInstance().getClassLoader(), ClassLoadingStrategy.Default.INJECTION) .getLoaded() - .getDeclaredConstructor(Server.class); + .getDeclaredConstructor(); return (Constructor) constructor; } catch (NoSuchMethodException e) { throw new RuntimeException("Failed to find Temporary Player constructor!", e); @@ -172,7 +168,7 @@ else if (methodName.equals("getPlayer")) { * @param message - a message. * @return Always NULL. */ - private static Object sendMessage(MinimalInjector injector, String message) { + private static Object sendMessage(Injector injector, String message) { for (PacketContainer packet : ChatExtensions.createChatPackets(message)) { injector.sendServerPacket(packet.getHandle(), null, false); } @@ -197,27 +193,13 @@ private static Object sendMessage(MinimalInjector injector, String message) { * Note that a temporary player has not yet been assigned a name, and thus cannot be * uniquely identified. Use the address instead. * - * @param server - the current server. * @return A temporary player instance. */ - public Player createTemporaryPlayer(final Server server) { + public static Player createTemporaryPlayer() { try { - return PLAYER_CONSTRUCTOR.newInstance(server); + return PLAYER_CONSTRUCTOR.newInstance(); } catch (ReflectiveOperationException exception) { throw new IllegalStateException("Unable to create temporary player", exception); } } - - /** - * Construct a temporary player with the given associated socket injector. - * - * @param server - the parent server. - * @param injector - the referenced socket injector. - * @return The temporary player. - */ - public Player createTemporaryPlayer(Server server, MinimalInjector injector) { - Player temporary = this.createTemporaryPlayer(server); - ((TemporaryPlayer) temporary).setInjector(injector); - return temporary; - } } diff --git a/src/main/java/com/comphenix/protocol/reflect/accessors/FieldAccessor.java b/src/main/java/com/comphenix/protocol/reflect/accessors/FieldAccessor.java index d2492e126..1881ea93f 100644 --- a/src/main/java/com/comphenix/protocol/reflect/accessors/FieldAccessor.java +++ b/src/main/java/com/comphenix/protocol/reflect/accessors/FieldAccessor.java @@ -9,6 +9,25 @@ */ public interface FieldAccessor { + /** + * NoOp Accessor, does what is says: nothing. + */ + static final FieldAccessor NO_OP_ACCESSOR = new FieldAccessor() { + @Override + public Object get(Object instance) { + return null; + } + + @Override + public void set(Object instance, Object value) { + } + + @Override + public Field getField() { + return null; + } + }; + /** * Retrieve the value of a field for a particular instance. * diff --git a/src/main/java/com/comphenix/protocol/utility/MinecraftMethods.java b/src/main/java/com/comphenix/protocol/utility/MinecraftMethods.java index b484447a3..433a45056 100644 --- a/src/main/java/com/comphenix/protocol/utility/MinecraftMethods.java +++ b/src/main/java/com/comphenix/protocol/utility/MinecraftMethods.java @@ -35,12 +35,13 @@ public final class MinecraftMethods { // For player connection - private volatile static MethodAccessor sendPacketMethod; - private volatile static MethodAccessor disconnectMethod; + private volatile static MethodAccessor playerConnectionSendMethod; + private volatile static MethodAccessor playerConnectionDisconnectMethod; // For network manager - private volatile static MethodAccessor networkManagerHandle; + private volatile static MethodAccessor networkManagerSend; private volatile static MethodAccessor networkManagerPacketRead; + private volatile static MethodAccessor networkManagerDisconnect; // For packet private volatile static MethodAccessor packetReadByteBuf; @@ -78,18 +79,18 @@ public static Function getFriendlyBufBufConstructor() { * * @return The send packet method. */ - public static MethodAccessor getSendPacketMethod() { - if (sendPacketMethod == null) { + public static MethodAccessor getPlayerConnectionSendMethod() { + if (playerConnectionSendMethod == null) { FuzzyReflection serverHandlerClass = FuzzyReflection.fromClass(MinecraftReflection.getPlayerConnectionClass()); try { - sendPacketMethod = Accessors.getMethodAccessor(serverHandlerClass.getMethod(FuzzyMethodContract.newBuilder() + playerConnectionSendMethod = Accessors.getMethodAccessor(serverHandlerClass.getMethod(FuzzyMethodContract.newBuilder() .parameterCount(1) .returnTypeVoid() .parameterExactType(MinecraftReflection.getPacketClass(), 0) .build())); } catch (IllegalArgumentException e) { - sendPacketMethod = Accessors.getMethodAccessor(serverHandlerClass.getMethod(FuzzyMethodContract.newBuilder() + playerConnectionSendMethod = Accessors.getMethodAccessor(serverHandlerClass.getMethod(FuzzyMethodContract.newBuilder() .nameRegex("sendPacket.*") .returnTypeVoid() .parameterCount(1) @@ -97,33 +98,41 @@ public static MethodAccessor getSendPacketMethod() { } } - return sendPacketMethod; + return playerConnectionSendMethod; } /** * Retrieve the disconnect method for a given player connection. * - * @param playerConnection - the player connection. * @return The */ - public static MethodAccessor getDisconnectMethod(Class playerConnection) { - if (disconnectMethod == null) { - FuzzyReflection playerConnectionClass = FuzzyReflection.fromClass(playerConnection); - try { - disconnectMethod = Accessors.getMethodAccessor(playerConnectionClass.getMethod(FuzzyMethodContract.newBuilder() + public static MethodAccessor getPlayerConnectionDisconnectMethod() { + if (playerConnectionDisconnectMethod == null) { + FuzzyReflection playerConnectionClass = FuzzyReflection.fromClass(MinecraftReflection.getPlayerConnectionClass()); + + if (MinecraftVersion.v1_21_0.atOrAbove()) { + playerConnectionDisconnectMethod = Accessors.getMethodAccessor(playerConnectionClass.getMethod(FuzzyMethodContract.newBuilder() .returnTypeVoid() - .nameRegex("disconnect.*") .parameterCount(1) - .parameterExactType(String.class, 0) + .parameterExactType(MinecraftReflection.getIChatBaseComponentClass(), 0) .build())); - } catch (IllegalArgumentException e) { - // Just assume it's the first String method - Method disconnect = playerConnectionClass.getMethodByParameters("disconnect", String.class); - disconnectMethod = Accessors.getMethodAccessor(disconnect); + } else { + try { + playerConnectionDisconnectMethod = Accessors.getMethodAccessor(playerConnectionClass.getMethod(FuzzyMethodContract.newBuilder() + .returnTypeVoid() + .nameRegex("disconnect.*") + .parameterCount(1) + .parameterExactType(String.class, 0) + .build())); + } catch (IllegalArgumentException e) { + // Just assume it's the first String method + Method disconnect = playerConnectionClass.getMethodByParameters("disconnect", String.class); + playerConnectionDisconnectMethod = Accessors.getMethodAccessor(disconnect); + } } } - return disconnectMethod; + return playerConnectionDisconnectMethod; } /** @@ -131,8 +140,8 @@ public static MethodAccessor getDisconnectMethod(Class playerConnection) { * * @return The handle method. */ - public static MethodAccessor getNetworkManagerHandleMethod() { - if (networkManagerHandle == null) { + public static MethodAccessor getNetworkManagerSendMethod() { + if (networkManagerSend == null) { Method handleMethod = FuzzyReflection .fromClass(MinecraftReflection.getNetworkManagerClass(), true) .getMethod(FuzzyMethodContract.newBuilder() @@ -141,10 +150,10 @@ public static MethodAccessor getNetworkManagerHandleMethod() { .parameterCount(1) .parameterExactType(MinecraftReflection.getPacketClass(), 0) .build()); - networkManagerHandle = Accessors.getMethodAccessor(handleMethod); + networkManagerSend = Accessors.getMethodAccessor(handleMethod); } - return networkManagerHandle; + return networkManagerSend; } /** @@ -163,6 +172,27 @@ public static MethodAccessor getNetworkManagerReadPacketMethod() { return networkManagerPacketRead; } + /** + * Retrieve the handle/send packet method of network manager. + * + * @return The handle method. + */ + public static MethodAccessor getNetworkManagerDisconnectMethod() { + if (networkManagerDisconnect == null) { + Method handleMethod = FuzzyReflection + .fromClass(MinecraftReflection.getNetworkManagerClass(), true) + .getMethod(FuzzyMethodContract.newBuilder() + .banModifier(Modifier.STATIC) + .returnTypeVoid() + .parameterCount(1) + .parameterExactType(MinecraftReflection.getIChatBaseComponentClass(), 0) + .build()); + networkManagerDisconnect = Accessors.getMethodAccessor(handleMethod); + } + + return networkManagerDisconnect; + } + /** * Retrieve the Packet.read(PacketDataSerializer) method. * diff --git a/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java b/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java index b0c425e83..2e06b60bb 100644 --- a/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java +++ b/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java @@ -373,6 +373,11 @@ public static boolean is(Class clazz, Object object) { if (clazz == null || object == null) { return false; } + + // check for accidential class objects + if (object instanceof Class) { + return clazz.isAssignableFrom((Class) object); + } return clazz.isAssignableFrom(object.getClass()); } @@ -502,6 +507,10 @@ public static boolean isCraftItemStack(Object obj) { return is(getCraftItemStackClass(), obj); } + public static boolean isIChatBaseComponent(Class target) { + return is(getIChatBaseComponentClass(), target); + } + /** * Retrieve the EntityPlayer (NMS) class. * diff --git a/src/test/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactoryTest.java b/src/test/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactoryTest.java index 8c76353aa..9bc1a4045 100644 --- a/src/test/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactoryTest.java +++ b/src/test/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactoryTest.java @@ -1,6 +1,5 @@ package com.comphenix.protocol.injector.temporary; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import org.bukkit.Server; @@ -12,12 +11,8 @@ public class TemporaryPlayerFactoryTest { - private static final TemporaryPlayerFactory temporaryPlayerFactory = new TemporaryPlayerFactory(); - @Mock Server server; - @Mock - MinimalInjector minimalInjector; @BeforeEach public void initMocks() { @@ -26,17 +21,7 @@ public void initMocks() { @Test public void testUnavailableSocketInjector() { - Player player = temporaryPlayerFactory.createTemporaryPlayer(this.server); + Player player = TemporaryPlayerFactory.createTemporaryPlayer(); assertThrows(IllegalStateException.class, player::getPlayer); } - - @Test - public void createTemporaryPlayer() { - - Player player = temporaryPlayerFactory.createTemporaryPlayer(this.server, this.minimalInjector); - assertEquals(this.server, player.getServer()); - - // May seem dumb, but this makes sure that the .equals method is still instact. - assertEquals(player, player); - } } diff --git a/src/test/java/com/comphenix/protocol/utility/MinecraftMethodsTest.java b/src/test/java/com/comphenix/protocol/utility/MinecraftMethodsTest.java index 6b94098da..5917c6b67 100644 --- a/src/test/java/com/comphenix/protocol/utility/MinecraftMethodsTest.java +++ b/src/test/java/com/comphenix/protocol/utility/MinecraftMethodsTest.java @@ -2,11 +2,13 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; -import com.comphenix.protocol.BukkitInitialization; import java.lang.reflect.Field; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.comphenix.protocol.BukkitInitialization; + public class MinecraftMethodsTest { @BeforeAll @@ -16,8 +18,12 @@ public static void initializeReflection() { @Test public void testSendPacketMethods() { - assertNotNull(MinecraftMethods.getSendPacketMethod()); - assertNotNull(MinecraftMethods.getNetworkManagerHandleMethod()); + assertNotNull(MinecraftMethods.getPlayerConnectionSendMethod()); + assertNotNull(MinecraftMethods.getPlayerConnectionDisconnectMethod()); + + assertNotNull(MinecraftMethods.getNetworkManagerSendMethod()); + assertNotNull(MinecraftMethods.getNetworkManagerReadPacketMethod()); + assertNotNull(MinecraftMethods.getNetworkManagerDisconnectMethod()); } private void setNull(final String fieldName) throws NoSuchFieldException, IllegalAccessException {