Prevented map chunk listeners from crashing the server.

If the NetServerHandler injector fails and we revert to the network
field injector instead, any map chunk listener will crash the server
due to complications with Bukkit's ChunkCompressionThread. 

We avoid this by disabling map chunk listening entirely.

ProtocolLib/src/main/java/com/comphenix/protocol/CleanupStaticMembers.java

@@ -43,7 +43,7 @@ class CleanupStaticMembers {
 		// This list must always be updated
 		Class<?>[] publicClasses = { 
 				AsyncListenerHandler.class, ListeningWhitelist.class, PacketContainer.class, 
-				BukkitUnwrapper.class, CollectionGenerator.class, DefaultInstances.class, 
+				BukkitUnwrapper.class, DefaultInstances.class, CollectionGenerator.class,
 				PrimitiveGenerator.class, FuzzyReflection.class, MethodUtils.class, 
 				BackgroundCompiler.class, StructureCompiler.class,
 				ObjectCloner.class, PrimitiveUtils.class, Packets.Server.class, 

ProtocolLib/src/main/java/com/comphenix/protocol/injector/ListenerInvoker.java

@@ -46,4 +46,25 @@ public interface ListenerInvoker {
 	 * @return The packet ID.
 	 */
 	public abstract int getPacketID(Packet packet);
+
+	/**
+	 * Associate a given class with the given packet ID. Internal method.
+	 * @param clazz - class to associate.
+	 * @param packetID - the packet ID.
+	 */
+	public abstract void unregisterPacketClass(Class<?> clazz);
+
+	/**
+	 * Remove a given class from the packet registry. Internal method.
+	 * @param clazz - class to remove.
+	 */
+	public abstract void registerPacketClass(Class<?> clazz, int packetID);
+
+	/**
+	 * Retrieves the correct packet class from a given packet ID.
+	 * @param packetID - the packet ID.
+ 	 * @param forceVanilla - whether or not to look for vanilla classes, not injected classes.
+	 * @return The associated class.
+	 */
+	public abstract Class<?> getPacketClassFromID(int packetID, boolean forceVanilla);
 }

ProtocolLib/src/main/java/com/comphenix/protocol/injector/MinecraftRegistry.java

@@ -157,7 +157,7 @@ class MinecraftRegistry {
 	/**
 	 * Retrieves the correct packet class from a given packet ID.
 	 * @param packetID - the packet ID.
- 	 * @param vanilla - whether or not to look for vanilla classes, not injected classes.
+ 	 * @param forceVanilla - whether or not to look for vanilla classes, not injected classes.
 	 * @return The associated class.
 	 */
 	public static Class getPacketClassFromID(int packetID, boolean forceVanilla) {
@@ -172,7 +172,9 @@ class MinecraftRegistry {
 		// Will most likely not be used
 		for (Map.Entry<Class, Integer> entry : getPacketToID().entrySet()) {
 			if (Objects.equal(entry.getValue(), packetID)) {
-				return entry.getKey();
+				// Attempt to get the vanilla class here too
+				if (!forceVanilla || entry.getKey().getName().startsWith("net.minecraft.server"))
+					return entry.getKey();
 			}
 		}
 		

ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java

@@ -174,7 +174,7 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok
 		
 		try {
 			// Initialize injection mangers
-			this.playerInjection = new PlayerInjectionHandler(classLoader, reporter, isInjectionNecessary, this, server);
+			this.playerInjection = new PlayerInjectionHandler(classLoader, reporter, isInjectionNecessary, this, packetListeners, server);
 			this.packetInjector = new PacketInjector(classLoader, this, playerInjection);
 			this.asyncFilterManager = new AsyncFilterManager(reporter, server.getScheduler(), this);
 			
@@ -210,9 +210,6 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok
 	 */
 	public void setPlayerHook(PlayerInjectHooks playerHook) {
 		playerInjection.setPlayerHook(playerHook);
-		
-		// Make sure the current listeners are compatible
-		playerInjection.checkListener(packetListeners);
 	}
 
 	@Override
@@ -662,6 +659,21 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok
 		return MinecraftRegistry.getPacketToID().get(packet.getClass());
 	}
 	
+	@Override
+	public void registerPacketClass(Class<?> clazz, int packetID) {
+		MinecraftRegistry.getPacketToID().put(clazz, packetID);
+	}
+	
+	@Override
+	public void unregisterPacketClass(Class<?> clazz) {
+		MinecraftRegistry.getPacketToID().remove(clazz);
+	}
+
+	@Override
+	public Class<?> getPacketClassFromID(int packetID, boolean forceVanilla) {
+		return MinecraftRegistry.getPacketClassFromID(packetID, forceVanilla);
+	}
+	
 	// Yes, this is crazy.
 	@SuppressWarnings({ "unchecked", "rawtypes" })
 	private void registerOld(PluginManager manager, Plugin plugin) {

ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java

@@ -141,10 +141,10 @@ class PacketInjector {
 		
 		try {
 			// Override values
-			putMethod.invoke(intHashMap, packetID, proxy);
 			previous.put(packetID, old);
 			registry.put(proxy, packetID);
 			overwritten.put(packetID, proxy);
+			putMethod.invoke(intHashMap, packetID, proxy);
 			return true;
 			
 		} catch (IllegalArgumentException e) {

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java

@@ -22,6 +22,8 @@ import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Set;
 
+import com.comphenix.protocol.Packets;
+import com.comphenix.protocol.injector.ListenerInvoker;
 import com.comphenix.protocol.injector.player.NetworkFieldInjector.FakePacket;
 
 import net.minecraft.server.Packet;
@@ -73,7 +75,7 @@ class InjectedArrayList extends ArrayList<Packet> {
 				// We'll use the FakePacket marker instead of preventing the filters
 				injector.sendServerPacket(createNegativePacket(packet), true);
 			}
-			
+
 			// Collection.add contract
 			return true;
 			
@@ -90,8 +92,10 @@ class InjectedArrayList extends ArrayList<Packet> {
 	 * @return The inverted packet.
 	 */
 	Packet createNegativePacket(Packet source) {
-		Enhancer ex = new Enhancer();
-		Class<?> type = source.getClass();
+		ListenerInvoker invoker = injector.getInvoker();
+		
+		int packetID = invoker.getPacketID(source);
+		Class<?> type = invoker.getPacketClassFromID(packetID, true);
 		
 		// We want to subtract the byte amount that were added to the running
 		// total of outstanding packets. Otherwise, cancelling too many packets
@@ -111,22 +115,38 @@ class InjectedArrayList extends ArrayList<Packet> {
 		//      }
 		//   ect.
 		//   }
+		Enhancer ex = new Enhancer();
+		ex.setSuperclass(type);
 		ex.setInterfaces(new Class[] { FakePacket.class } );
 		ex.setUseCache(true);
 		ex.setClassLoader(classLoader);
-		ex.setSuperclass(type);
-		ex.setCallback(new MethodInterceptor() {
-			@Override
-			public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
-				if (method.getReturnType().equals(int.class) && args.length == 0) {
-					Integer result = (Integer) proxy.invokeSuper(obj, args);
-					return -result;
-				} else {
-					return proxy.invokeSuper(obj, args);
-				}
-			}
-		});
+		ex.setCallbackType(InvertedIntegerCallback.class);
+
+		Class<?> proxyClass = ex.createClass();
+
+		// Temporarily associate the fake packet class
+		invoker.registerPacketClass(proxyClass, packetID);
+
+		Packet fake = (Packet) Enhancer.create(proxyClass, new InvertedIntegerCallback());
 		
-		return (Packet) ex.create();
+		// Remove this association
+		invoker.unregisterPacketClass(proxyClass);
+		return fake;
+	}
+	
+	/**
+	 * Inverts the integer result of every integer method.
+	 * @author Kristian
+	 */
+	private class InvertedIntegerCallback implements MethodInterceptor {
+		@Override
+		public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
+			if (method.getReturnType().equals(int.class) && args.length == 0) {
+				Integer result = (Integer) proxy.invokeSuper(obj, args);
+				return -result;
+			} else {
+				return proxy.invokeSuper(obj, args);
+			}
+		}
 	}
 }

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java

@@ -123,11 +123,14 @@ class NetworkFieldInjector extends PlayerInjector {
 	}
 	
 	@Override
-	public void checkListener(PacketListener listener) {
+	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(), 
-				Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK)) {
-			throw new IllegalStateException("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners.");
+		if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) {
+			return new UnsupportedListener("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners.", unsupported);
+		} else {
+			return null;
 		}
 	}
 	

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java

@@ -88,11 +88,14 @@ class NetworkObjectInjector extends PlayerInjector {
 	}
 	
 	@Override
-	public void checkListener(PacketListener listener) {
+	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(), 
-				Packets.Server.MAP_CHUNK, Packets.Server.MAP_CHUNK_BULK)) {
-			throw new IllegalStateException("The NETWORK_FIELD_INJECTOR hook doesn't support map chunk listeners.");
+		if (ListeningWhitelist.containsAny(listener.getSendingWhitelist(), unsupported)) {
+			return new UnsupportedListener("The NETWORK_OBJECT_INJECTOR hook doesn't support map chunk listeners.", unsupported);
+		} else {
+			return null;
 		}
 	}
 	

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java

@@ -301,8 +301,9 @@ public class NetworkServerInjector extends PlayerInjector {
 	}
 	
 	@Override
-	public void checkListener(PacketListener listener) {
+	public UnsupportedListener checkListener(PacketListener listener) {
 		// We support everything
+		return null;
 	}
 
 	@Override

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java

@@ -83,6 +83,9 @@ public class PlayerInjectionHandler {
 	// Enabled packet filters
 	private IntegerSet sendingFilters = new IntegerSet(MAXIMUM_PACKET_ID + 1);
 	
+	// List of packet listeners
+	private Set<PacketListener> packetListeners;
+	
 	// The class loader we're using
 	private ClassLoader classLoader;
 	
@@ -90,12 +93,13 @@ public class PlayerInjectionHandler {
 	private Predicate<GamePhase> injectionFilter;
 	
 	public PlayerInjectionHandler(ClassLoader classLoader, ErrorReporter reporter, Predicate<GamePhase> injectionFilter, 
-								  ListenerInvoker invoker, Server server) {
+								  ListenerInvoker invoker, Set<PacketListener> packetListeners, Server server) {
 		
 		this.classLoader = classLoader;
 		this.reporter = reporter;
 		this.invoker = invoker;
 		this.injectionFilter = injectionFilter;
+		this.packetListeners = packetListeners;
 		this.netLoginInjector = new NetLoginInjector(reporter, this, server);
 		this.serverInjection = new InjectedServerConnection(reporter, server, netLoginInjector);
 		serverInjection.injectList();
@@ -143,6 +147,9 @@ public class PlayerInjectionHandler {
 			loginPlayerHook = playerHook;
 		if (phase.hasPlaying())
 			playingPlayerHook = playerHook;
+		
+		// Make sure the current listeners are compatible
+		checkListener(packetListeners);
 	}
 	
 	/**
@@ -534,26 +541,30 @@ public class PlayerInjectionHandler {
 		// Make sure the current listeners are compatible
 		if (lastSuccessfulHook != null) {
 			for (PacketListener listener : listeners) {
-				try {
-					checkListener(listener);
-				} catch (IllegalStateException e) {
-					reporter.reportWarning(this, "Unsupported listener.", e);
-				}
+				checkListener(listener);
 			}
 		}
 	}
 	
 	/**
 	 * Determine if a listener is valid or not.
+	 * <p>
+	 * If not, a warning will be printed to the console. 
 	 * @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);
+		if (lastSuccessfulHook != null) {
+			UnsupportedListener result = lastSuccessfulHook.checkListener(listener);
+
+			// We won't prevent the listener, as it may still have valid packets
+			if (result != null) {
+				reporter.reportWarning(this, "Cannot fully register listener for " + 
+						  PacketAdapter.getPluginName(listener) + ": " + result.toString());
+				
+				// These are illegal
+				for (int packetID : result.getPackets())
+					removePacketHandler(packetID);
+			}
 		}
 	}
 	

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java

@@ -489,10 +489,12 @@ abstract class PlayerInjector {
 	/**
 	 * Invoked before a new listener is registered.
 	 * <p>
-	 * The player injector should throw an exception if this listener cannot be properly supplied with packet events. 
+	 * The player injector should only return a non-null value if some or all of the packet IDs are unsupported.
+	 * 
 	 * @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 void checkListener(PacketListener listener);
+	public abstract UnsupportedListener checkListener(PacketListener listener);
 	
 	/**
 	 * Allows a packet to be sent by the listeners.
@@ -589,6 +591,14 @@ abstract class PlayerInjector {
 		return player;
 	}
 	
+	/**
+	 * Object that can invoke the packet events.
+	 * @return Packet event invoker.
+	 */
+	public ListenerInvoker getInvoker() {
+		return invoker;
+	}
+	
 	/**
 	 * Retrieve the hooked player object OR the more up-to-date player instance.
 	 * @return The hooked player, or a more up-to-date instance.

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/UnsupportedListener.java

@@ -0,0 +1,47 @@
+package com.comphenix.protocol.injector.player;
+
+import java.util.Arrays;
+
+import com.google.common.base.Joiner;
+
+/**
+ * Represents an error message from a player injector.
+ * 
+ * @author Kristian
+ */
+public class UnsupportedListener {
+	private String message;
+	private int[] packets;
+	
+	/**
+	 * Create a new error message.
+	 * @param message - the message.
+	 * @param packets - unsupported packets.
+	 */
+	public UnsupportedListener(String message, int[] packets) {
+		super();
+		this.message = message;
+		this.packets = packets;
+	}
+
+	/**
+	 * Retrieve the error message.
+	 * @return Error message.
+	 */
+	public String getMessage() {
+		return message;
+	}
+	
+	/**
+	 * Retrieve all unsupported packets.
+	 * @return Unsupported packets.
+	 */
+	public int[] getPackets() {
+		return packets;
+	}
+	
+	@Override
+	public String toString() {
+		return String.format("%s (%s)", message, Joiner.on(", ").join(Arrays.asList(packets)));
+	}
+}