From 3c97cffc09f2c0179d803b426fc0e75e5fca8783 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 12 Mar 2013 00:52:09 +0100 Subject: [PATCH] After Minecraft 1.4.4, CraftBukkit no longer redirects MAP_CHUNK. We can therefore relax the requirements in NetworkFieldInjector and NetworkObjectInjetor. --- .../comphenix/protocol/ProtocolLibrary.java | 18 ++++++++----- .../injector/PacketFilterManager.java | 12 +++++++-- .../injector/player/NetworkFieldInjector.java | 27 +++++++++++++------ .../player/NetworkObjectInjector.java | 26 +++++++++++++----- .../player/NetworkServerInjector.java | 3 ++- .../player/PlayerInjectionHandler.java | 4 ++- .../injector/player/PlayerInjector.java | 5 +++- .../player/PlayerInjectorBuilder.java | 14 +++++++++- .../player/ProxyPlayerInjectionHandler.java | 13 ++++++--- .../injector/spigot/DummyPlayerHandler.java | 2 +- .../{ => utility}/MinecraftVersion.java | 4 +-- .../protocol/MinecraftVersionTest.java | 2 ++ 12 files changed, 96 insertions(+), 34 deletions(-) rename ProtocolLib/src/main/java/com/comphenix/protocol/{ => utility}/MinecraftVersion.java (98%) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java index 271c1181..68fc0808 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java @@ -43,6 +43,7 @@ import com.comphenix.protocol.metrics.Updater; import com.comphenix.protocol.metrics.Updater.UpdateResult; import com.comphenix.protocol.reflect.compiler.BackgroundCompiler; import com.comphenix.protocol.utility.ChatExtensions; +import com.comphenix.protocol.utility.MinecraftVersion; /** * The main entry point for ProtocolLib. @@ -132,12 +133,15 @@ public class ProtocolLibrary extends JavaPlugin { // Check for other versions checkConflictingVersions(); + // Handle unexpected Minecraft versions + MinecraftVersion version = verifyMinecraftVersion(); + // Set updater updater = new Updater(this, logger, "protocollib", getFile(), "protocol.info"); unhookTask = new DelayedSingleTask(this); protocolManager = new PacketFilterManager( - getClassLoader(), getServer(), unhookTask, detailedReporter); + getClassLoader(), getServer(), version, unhookTask, detailedReporter); // Setup error reporter detailedReporter.addGlobalParameter("manager", protocolManager); @@ -248,10 +252,7 @@ public class ProtocolLibrary extends JavaPlugin { } else { logger.info("Structure compiler thread has been disabled."); } - - // Handle unexpected Minecraft versions - verifyMinecraftVersion(); - + // Set up command handlers registerCommand(CommandProtocol.NAME, commandProtocol); registerCommand(CommandPacket.NAME, commandPacket); @@ -282,7 +283,7 @@ public class ProtocolLibrary extends JavaPlugin { } // Used to check Minecraft version - private void verifyMinecraftVersion() { + private MinecraftVersion verifyMinecraftVersion() { try { MinecraftVersion minimum = new MinecraftVersion(MINIMUM_MINECRAFT_VERSION); MinecraftVersion maximum = new MinecraftVersion(MAXIMUM_MINECRAFT_VERSION); @@ -296,9 +297,14 @@ public class ProtocolLibrary extends JavaPlugin { if (current.compareTo(maximum) > 0) logger.warning("Version " + current + " has not yet been tested! Proceed with caution."); } + return current; + } catch (Exception e) { reporter.reportWarning(this, "Unable to retrieve current Minecraft version.", e); } + + // Unknown version + return null; } private void checkConflictingVersions() { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index c5b8c0ac..0bdb1023 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -61,6 +61,7 @@ import com.comphenix.protocol.injector.spigot.SpigotPacketInjector; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.utility.MinecraftReflection; +import com.comphenix.protocol.utility.MinecraftVersion; import com.google.common.base.Objects; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; @@ -149,9 +150,15 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok /** * Only create instances of this class if protocol lib is disabled. - * @param unhookTask */ public PacketFilterManager(ClassLoader classLoader, Server server, DelayedSingleTask unhookTask, ErrorReporter reporter) { + this(classLoader, server, new MinecraftVersion(server), unhookTask, reporter); + } + + /** + * Only create instances of this class if protocol lib is disabled. + */ + public PacketFilterManager(ClassLoader classLoader, Server server, MinecraftVersion mcVersion, DelayedSingleTask unhookTask, ErrorReporter reporter) { if (reporter == null) throw new IllegalArgumentException("reporter cannot be NULL."); if (classLoader == null) @@ -201,6 +208,7 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok classLoader(classLoader). packetListeners(packetListeners). injectionFilter(isInjectionNecessary). + version(mcVersion). buildHandler(); this.packetInjector = PacketInjectorBuilder.newBuilder(). @@ -561,7 +569,7 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok return; } - playerInjection.processPacket(sender, mcPacket); + playerInjection.recieveClientPacket(sender, mcPacket); } @Override diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java index 88d9beec..559c93ef 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java @@ -39,6 +39,7 @@ import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.VolatileField; +import com.comphenix.protocol.utility.MinecraftVersion; import com.google.common.collect.Sets; /** @@ -56,6 +57,12 @@ class NetworkFieldInjector extends PlayerInjector { // Nothing } + // After commit 336a4e00668fd2518c41242755ed6b3bdc3b0e6c (Update CraftBukkit to Minecraft 1.4.4.), + // CraftBukkit stopped redirecting map chunk and map chunk bulk packets to a separate queue. + // Thus, NetworkFieldInjector can safely handle every packet (though not perfectly - some packets + // will be slightly processed). + private MinecraftVersion safeVersion = new MinecraftVersion("1.4.4"); + // Packets to ignore private Set ignoredPackets = Sets.newSetFromMap(new ConcurrentHashMap()); @@ -99,7 +106,6 @@ class NetworkFieldInjector extends PlayerInjector { @Override public void sendServerPacket(Object packet, boolean filtered) throws InvocationTargetException { - if (networkManager != null) { try { if (!filtered) { @@ -122,14 +128,19 @@ class NetworkFieldInjector extends PlayerInjector { } @Override - public UnsupportedListener checkListener(PacketListener listener) { - int[] unsupported = { Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK }; - - // Unfortunately, we don't support chunk packets - if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) { - return new UnsupportedListener("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners.", unsupported); - } else { + public UnsupportedListener checkListener(MinecraftVersion version, PacketListener listener) { + if (version != null && version.compareTo(safeVersion) > 0) { return null; + + } else { + int[] unsupported = { Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK }; + + // Unfortunately, we don't support chunk packets + if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) { + return new UnsupportedListener("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners.", unsupported); + } else { + return null; + } } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java index 69acf343..2ea0da49 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java @@ -40,6 +40,7 @@ import com.comphenix.protocol.injector.GamePhase; import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.injector.PacketFilterManager.PlayerInjectHooks; import com.comphenix.protocol.injector.server.TemporaryPlayerFactory; +import com.comphenix.protocol.utility.MinecraftVersion; /** * Injection method that overrides the NetworkHandler itself, and its queue-method. @@ -53,6 +54,12 @@ public class NetworkObjectInjector extends PlayerInjector { // Used to construct proxy objects private ClassLoader classLoader; + // After commit 336a4e00668fd2518c41242755ed6b3bdc3b0e6c (Update CraftBukkit to Minecraft 1.4.4.), + // CraftBukkit stopped redirecting map chunk and map chunk bulk packets to a separate queue. + // Thus, NetworkFieldInjector can safely handle every packet (though not perfectly - some packets + // will be slightly processed). + private MinecraftVersion safeVersion = new MinecraftVersion("1.4.4"); + // Shared callback filter - avoid creating a new class every time private volatile static CallbackFilter callbackFilter; @@ -117,14 +124,19 @@ public class NetworkObjectInjector extends PlayerInjector { } @Override - public UnsupportedListener checkListener(PacketListener listener) { - int[] unsupported = { Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK }; - - // Unfortunately, we don't support chunk packets - if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) { - return new UnsupportedListener("The NETWORK_OBJECT_INJECTOR hook doesn't support map chunk listeners.", unsupported); - } else { + public UnsupportedListener checkListener(MinecraftVersion version, PacketListener listener) { + if (version != null && version.compareTo(safeVersion) > 0) { return null; + + } else { + int[] unsupported = { Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK }; + + // Unfortunately, we don't support chunk packets + if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) { + return new UnsupportedListener("The NETWORK_OBJECT_INJECTOR hook doesn't support map chunk listeners.", unsupported); + } else { + return null; + } } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java index ef6858bc..207bcba3 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java @@ -40,6 +40,7 @@ import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.reflect.instances.ExistingGenerator; import com.comphenix.protocol.utility.MinecraftMethods; import com.comphenix.protocol.utility.MinecraftReflection; +import com.comphenix.protocol.utility.MinecraftVersion; /** * Represents a player hook into the NetServerHandler class. @@ -326,7 +327,7 @@ class NetworkServerInjector extends PlayerInjector { } @Override - public UnsupportedListener checkListener(PacketListener listener) { + public UnsupportedListener checkListener(MinecraftVersion version, PacketListener listener) { // We support everything return null; } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java index 8c59c15a..b26769fa 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -126,11 +126,12 @@ public interface PlayerInjectionHandler { * @throws IllegalAccessException If the reflection machinery failed. * @throws InvocationTargetException If the underlying method caused an error. */ - public abstract void processPacket(Player player, Object mcPacket) + public abstract void recieveClientPacket(Player player, Object mcPacket) throws IllegalAccessException, InvocationTargetException; /** * Determine if the given listeners are valid. + * @param version - the current Minecraft version, or NULL if unknown. * @param listeners - listeners to check. */ public abstract void checkListener(Set listeners); @@ -139,6 +140,7 @@ public interface PlayerInjectionHandler { * Determine if a listener is valid or not. *

* If not, a warning will be printed to the console. + * @param version - the current Minecraft version, or NULL if unknown. * @param listener - listener to check. */ public abstract void checkListener(PacketListener listener); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java index 524bd40a..a2fa3362 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -43,6 +43,7 @@ import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.VolatileField; import com.comphenix.protocol.utility.MinecraftReflection; +import com.comphenix.protocol.utility.MinecraftVersion; abstract class PlayerInjector implements SocketInjector { @@ -508,11 +509,13 @@ abstract class PlayerInjector implements SocketInjector { * Invoked before a new listener is registered. *

* The player injector should only return a non-null value if some or all of the packet IDs are unsupported. + * @param version * + * @param version - the current Minecraft version, or NULL if unknown. * @param listener - the listener that is about to be registered. * @return A error message with the unsupported packet IDs, or NULL if this listener is valid. */ - public abstract UnsupportedListener checkListener(PacketListener listener); + public abstract UnsupportedListener checkListener(MinecraftVersion version, PacketListener listener); /** * Allows a packet to be sent by the listeners. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectorBuilder.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectorBuilder.java index 120d5dc9..77ccbf5f 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectorBuilder.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectorBuilder.java @@ -14,6 +14,7 @@ import com.comphenix.protocol.events.PacketListener; import com.comphenix.protocol.injector.GamePhase; import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.injector.PacketFilterManager; +import com.comphenix.protocol.utility.MinecraftVersion; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; @@ -37,6 +38,7 @@ public class PlayerInjectorBuilder { protected ListenerInvoker invoker; protected Set packetListeners; protected Server server; + protected MinecraftVersion version; /** * Set the class loader to use during class generation. @@ -107,6 +109,16 @@ public class PlayerInjectorBuilder { return this; } + /** + * Set the current Minecraft version. + * @param server - the current Minecraft version, or NULL if unknown. + * @return This builder, for chaining. + */ + public PlayerInjectorBuilder version(MinecraftVersion version) { + this.version = version; + return this; + } + /** * Called before an object is created with this builder. */ @@ -140,6 +152,6 @@ public class PlayerInjectorBuilder { return new ProxyPlayerInjectionHandler( classLoader, reporter, injectionFilter, - invoker, packetListeners, server); + invoker, packetListeners, server, version); } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java index 35020261..5a9be055 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java @@ -45,6 +45,7 @@ import com.comphenix.protocol.injector.server.AbstractInputStreamLookup; import com.comphenix.protocol.injector.server.InputStreamLookupBuilder; import com.comphenix.protocol.injector.server.SocketInjector; import com.comphenix.protocol.utility.MinecraftReflection; +import com.comphenix.protocol.utility.MinecraftVersion; import com.google.common.base.Predicate; import com.google.common.cache.Cache; @@ -91,6 +92,9 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { // Used to invoke events private ListenerInvoker invoker; + // Current Minecraft version + private MinecraftVersion version; + // Enabled packet filters private IntegerSet sendingFilters = new IntegerSet(Packets.MAXIMUM_PACKET_ID + 1); @@ -105,13 +109,14 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { public ProxyPlayerInjectionHandler( ClassLoader classLoader, ErrorReporter reporter, Predicate injectionFilter, - ListenerInvoker invoker, Set packetListeners, Server server) { + ListenerInvoker invoker, Set packetListeners, Server server, MinecraftVersion version) { this.classLoader = classLoader; this.reporter = reporter; this.invoker = invoker; this.injectionFilter = injectionFilter; this.packetListeners = packetListeners; + this.version = version; this.inputStreamLookup = InputStreamLookupBuilder.newBuilder(). server(server). @@ -501,14 +506,14 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { } /** - * Process a packet as if it were sent by the given player. + * Recieve a packet as if it were sent by the given player. * @param player - the sender. * @param mcPacket - the packet to process. * @throws IllegalAccessException If the reflection machinery failed. * @throws InvocationTargetException If the underlying method caused an error. */ @Override - public void processPacket(Player player, Object mcPacket) throws IllegalAccessException, InvocationTargetException { + public void recieveClientPacket(Player player, Object mcPacket) throws IllegalAccessException, InvocationTargetException { PlayerInjector injector = getInjector(player); // Process the given packet, or simply give up @@ -619,7 +624,7 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { @Override public void checkListener(PacketListener listener) { if (lastSuccessfulHook != null) { - UnsupportedListener result = lastSuccessfulHook.checkListener(listener); + UnsupportedListener result = lastSuccessfulHook.checkListener(version, listener); // We won't prevent the listener, as it may still have valid packets if (result != null) { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/DummyPlayerHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/DummyPlayerHandler.java index dc6f5e9d..08b1a8d2 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/DummyPlayerHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/DummyPlayerHandler.java @@ -74,7 +74,7 @@ class DummyPlayerHandler implements PlayerInjectionHandler { } @Override - public void processPacket(Player player, Object mcPacket) throws IllegalAccessException, InvocationTargetException { + public void recieveClientPacket(Player player, Object mcPacket) throws IllegalAccessException, InvocationTargetException { injector.processPacket(player, mcPacket); } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/MinecraftVersion.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java similarity index 98% rename from ProtocolLib/src/main/java/com/comphenix/protocol/MinecraftVersion.java rename to ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java index 53fb9898..e78dc266 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/MinecraftVersion.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java @@ -15,7 +15,7 @@ * 02111-1307 USA */ -package com.comphenix.protocol; +package com.comphenix.protocol.utility; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -31,7 +31,7 @@ import com.google.common.collect.Ordering; * * @author Kristian */ -class MinecraftVersion implements Comparable { +public class MinecraftVersion implements Comparable { /** * Regular expression used to parse version strings. */ diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/MinecraftVersionTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/MinecraftVersionTest.java index 8f98de70..2d127c23 100644 --- a/ProtocolLib/src/test/java/com/comphenix/protocol/MinecraftVersionTest.java +++ b/ProtocolLib/src/test/java/com/comphenix/protocol/MinecraftVersionTest.java @@ -21,6 +21,8 @@ import static org.junit.Assert.*; import org.junit.Test; +import com.comphenix.protocol.utility.MinecraftVersion; + public class MinecraftVersionTest { @Test