From 90970d1b9b5dfa3c80e9d222ea0701f7c94cb87c Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Wed, 26 Sep 2012 06:00:26 +0200 Subject: [PATCH] Ensure that server operators are informed about incompatibility. --- .../protocol/events/ListeningWhitelist.java | 17 ++++ .../injector/NetworkFieldInjector.java | 13 ++- .../injector/NetworkObjectInjector.java | 15 ++-- .../injector/NetworkServerInjector.java | 9 ++- .../injector/PacketFilterManager.java | 81 ++++++++++++++----- .../protocol/injector/PlayerInjector.java | 9 ++- 6 files changed, 110 insertions(+), 34 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/events/ListeningWhitelist.java b/ProtocolLib/src/com/comphenix/protocol/events/ListeningWhitelist.java index 31cfadbb..cca5740d 100644 --- a/ProtocolLib/src/com/comphenix/protocol/events/ListeningWhitelist.java +++ b/ProtocolLib/src/com/comphenix/protocol/events/ListeningWhitelist.java @@ -86,6 +86,23 @@ public class ListeningWhitelist { return Objects.hashCode(priority, whitelist); } + /** + * Determine if any of the given IDs can be found in the whitelist. + * @param whitelist - whitelist to test. + * @param idList - list of packet IDs to find. + * @return TRUE if any of the packets in the list can be found in the whitelist, FALSE otherwise. + */ + public static boolean containsAny(ListeningWhitelist whitelist, int... idList) { + if (whitelist != null) { + for (int i = 0; i < idList.length; i++) { + if (whitelist.getWhitelist().contains(idList[i])) + return true; + } + } + + return false; + } + @Override public boolean equals(final Object obj){ if(obj instanceof ListeningWhitelist){ diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java index 7275d5e1..56903e35 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java @@ -10,6 +10,9 @@ import java.util.concurrent.ConcurrentHashMap; import org.bukkit.entity.Player; +import com.comphenix.protocol.Packets; +import com.comphenix.protocol.events.ListeningWhitelist; +import com.comphenix.protocol.events.PacketListener; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; @@ -83,11 +86,13 @@ class NetworkFieldInjector extends PlayerInjector { } } - @Override - public boolean canInject() { - // Probably - return true; + public void checkListener(PacketListener listener) { + // Unfortunately, we don't support chunk packets + if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), + Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK)) { + throw new IllegalStateException("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners."); + } } @Override diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java index c9bb001d..9091390d 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java @@ -11,6 +11,10 @@ import java.util.Set; import org.bukkit.entity.Player; +import com.comphenix.protocol.Packets; +import com.comphenix.protocol.events.ListeningWhitelist; +import com.comphenix.protocol.events.PacketListener; + /** * Injection method that overrides the NetworkHandler itself, and it's sendPacket-method. * @@ -43,11 +47,12 @@ class NetworkObjectInjector extends PlayerInjector { } @Override - public boolean canInject() { - // We only support 1.3.0 at the moment. Fixing it require us to - // add jMock, which would add another dependency. - return networkManager != null && - networkManagerField.getType().isInterface(); + public void checkListener(PacketListener listener) { + // Unfortunately, we don't support chunk packets + if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), + Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK)) { + throw new IllegalStateException("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners."); + } } @Override diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index d5f145e9..ba9d6ee0 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java @@ -12,6 +12,7 @@ import net.sf.cglib.proxy.MethodProxy; import org.bukkit.entity.Player; +import com.comphenix.protocol.events.PacketListener; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.instances.CollectionGenerator; @@ -66,6 +67,7 @@ public class NetworkServerInjector extends PlayerInjector { @Override public void injectManager() { + if (serverHandlerRef == null) throw new IllegalStateException("Cannot find server handler."); // Don't inject twice @@ -142,10 +144,9 @@ public class NetworkServerInjector extends PlayerInjector { e.printStackTrace(); } } - + @Override - public boolean canInject() { - // Probably always - return true; + public void checkListener(PacketListener listener) { + // We support everything } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 18fc3738..3d5d94b3 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -74,7 +74,7 @@ public final class PacketFilterManager implements ProtocolManager { NETWORK_MANAGER_OBJECT, /** - * Override the server handler object. Versatile, but slow. + * Override the server handler object. Versatile, but a tad slower. */ NETWORK_SERVER_OBJECT; } @@ -106,6 +106,9 @@ public final class PacketFilterManager implements ProtocolManager { // The default class loader private ClassLoader classLoader; + // The last successful player hook + private PlayerInjector lastSuccessfulHook; + // Error logger private Logger logger; @@ -142,6 +145,13 @@ public final class PacketFilterManager implements ProtocolManager { */ public void setPlayerHook(PlayerInjectHooks playerHook) { this.playerHook = playerHook; + + // Make sure the current listeners are compatible + if (lastSuccessfulHook != null) { + for (PacketListener listener : packetListeners) { + checkListener(listener); + } + } } public Logger getLogger() { @@ -176,6 +186,9 @@ public final class PacketFilterManager implements ProtocolManager { if (hasReceiving) { recievedListeners.addListener(listener, receiving); enablePacketFilters(ConnectionSide.CLIENT_SIDE, receiving.getWhitelist()); + + // We don't know if we've hooked any players yet + checkListener(listener); } // Inform our injected hooks @@ -183,6 +196,20 @@ public final class PacketFilterManager implements ProtocolManager { } } + /** + * Determine if a listener is valid or not. + * @param listener - listener to check. + * @throws IllegalStateException If the given listener's whitelist cannot be fulfilled. + */ + public void checkListener(PacketListener listener) { + try { + if (lastSuccessfulHook != null) + lastSuccessfulHook.checkListener(listener); + } catch (Exception e) { + throw new IllegalStateException("Registering listener " + PacketAdapter.getPluginName(listener) + " failed", e); + } + } + @Override public void removePacketListener(PacketListener listener) { if (listener == null) @@ -367,15 +394,11 @@ public final class PacketFilterManager implements ProtocolManager { /** * Used to construct a player hook. * @param player - the player to hook. + * @param hook - the hook type. * @return A new player hoook * @throws IllegalAccessException Unable to do our reflection magic. */ - protected PlayerInjector getPlayerHookInstance(Player player) throws IllegalAccessException { - return getHookInstance(player, playerHook); - } - - // Helper - private PlayerInjector getHookInstance(Player player, PlayerInjectHooks hook) throws IllegalAccessException { + protected PlayerInjector getHookInstance(Player player, PlayerInjectHooks hook) throws IllegalAccessException { // Construct the correct player hook switch (hook) { case NETWORK_HANDLER_FIELDS: @@ -394,20 +417,42 @@ public final class PacketFilterManager implements ProtocolManager { * @param player - player to hook. */ protected void injectPlayer(Player player) { + + PlayerInjector injector = null; + PlayerInjectHooks currentHook = playerHook; + boolean firstPlayer = lastSuccessfulHook == null; + // Don't inject if the class has closed if (!hasClosed && player != null && !playerInjection.containsKey(player)) { - try { - PlayerInjector injector = getPlayerHookInstance(player); - - injector.injectManager(); - playerInjection.put(player, injector); - connectionLookup.put(injector.getInputStream(false), player); - - } catch (IllegalAccessException e) { - // Mark this injection attempt as a failure - playerInjection.put(player, null); - logger.log(Level.SEVERE, "Unable to access fields.", e); + while (true) { + try { + injector = getHookInstance(player, currentHook); + injector.injectManager(); + playerInjection.put(player, injector); + connectionLookup.put(injector.getInputStream(false), player); + break; + + } catch (Exception e) { + // Mark this injection attempt as a failure + logger.log(Level.SEVERE, "Player hook " + currentHook.toString() + " failed.", e); + + if (currentHook.ordinal() > 0) { + // Choose the previous player hook type + currentHook = PlayerInjectHooks.values()[currentHook.ordinal() - 1]; + logger.log(Level.INFO, "Switching to " + currentHook.toString() + " instead."); + } else { + // UTTER FAILURE + playerInjection.put(player, null); + return; + } + } } + + // Update values + if (injector != null) + lastSuccessfulHook = injector; + if (currentHook != playerHook || firstPlayer) + setPlayerHook(currentHook); } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java index 612e6e70..9bacc311 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java @@ -31,6 +31,7 @@ import org.bukkit.entity.Player; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; +import com.comphenix.protocol.events.PacketListener; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; @@ -205,10 +206,12 @@ abstract class PlayerInjector { public abstract void cleanupAll(); /** - * Determine if we actually can inject. - * @return TRUE if this injector is compatible with the current CraftBukkit version, FALSE otherwise. + * Invoked before a new listener is registered. + *

+ * The player injector should throw an exception if this listener cannot be properly supplied with packet events. + * @param listener - the listener that is about to be registered. */ - public abstract boolean canInject(); + public abstract void checkListener(PacketListener listener); /** * Allows a packet to be recieved by the listeners.