Added support for default values in packets.

ProtocolLib/src/com/comphenix/protocol/ProtocolLibrary.java

@@ -35,11 +35,12 @@ public class ProtocolLibrary extends JavaPlugin {
 	@Override
 	public void onDisable() {
 		protocolManager.close();
+		protocolManager = null;
 	}
 
 	/**
 	 * Retrieves the packet protocol manager.
-	 * @return Packet protocol manager.
+	 * @return Packet protocol manager, or NULL if it has been disabled.
 	 */
 	public static ProtocolManager getProtocolManager() {
 		return protocolManager;

ProtocolLib/src/com/comphenix/protocol/ProtocolManager.java

@@ -75,6 +75,20 @@ public interface ProtocolManager {
 	 * @return New encapsulated Minecraft packet.
 	 */
 	public PacketContainer createPacket(int id);
+	
+	/**
+	 * Constructs a new encapsulated Minecraft packet with the given ID.
+	 * <p>
+	 * If set to true, the skip default option will prevent the system from assigning 
+	 * non-primitive fields in the packet to a new default instance. For instance, certain
+	 * packets - like Packet60Explosion - require a List or Set to be non-null. If the
+	 * skipDefaults option is false, the List or Set will be automatically created.
+	 * 
+	 * @param id - packet ID.
+	 * @param skipDefaults - TRUE to skip setting default values, FALSE otherwise.
+	 * @return New encapsulated Minecraft packet.
+	 */
+	public PacketContainer createPacket(int id, boolean skipDefaults);
 
 	/**
 	 * Retieves a set of every enabled packet.

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

@@ -217,7 +217,23 @@ public final class PacketFilterManager implements ProtocolManager {
 	
 	@Override
 	public PacketContainer createPacket(int id) {
-		return new PacketContainer(id);
+		return createPacket(id, false);
+	}
+	
+	@Override
+	public PacketContainer createPacket(int id, boolean skipDefaults) {
+		PacketContainer packet = new PacketContainer(id);
+		
+		// Use any default values if possible
+		if (!skipDefaults) {
+			try {
+				packet.getModifier().writeDefaults();
+			} catch (IllegalAccessException e) {
+				throw new RuntimeException("Security exception.", e);
+			}
+		}
+		
+		return packet;
 	}
 	
 	@Override
@@ -324,6 +340,8 @@ public final class PacketFilterManager implements ProtocolManager {
 		if (packetInjector != null)
 			packetInjector.cleanupAll();
 		
+		// Remove listeners
+		packetListeners.clear();
 		playerInjection.clear();
 		connectionLookup.clear();
 		hasClosed = true;

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

@@ -2,14 +2,19 @@ package com.comphenix.protocol.injector;
 
 import java.io.DataInputStream;
 import java.lang.reflect.Field;
-import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
+import java.util.Collections;
+import java.util.List;
+import java.util.ArrayList;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 import net.minecraft.server.EntityPlayer;
 import net.minecraft.server.Packet;
+import net.sf.cglib.proxy.Enhancer;
+import net.sf.cglib.proxy.MethodInterceptor;
+import net.sf.cglib.proxy.MethodProxy;
 
 import org.bukkit.craftbukkit.entity.CraftPlayer;
 import org.bukkit.entity.Player;
@@ -18,9 +23,19 @@ import com.comphenix.protocol.events.PacketContainer;
 import com.comphenix.protocol.events.PacketEvent;
 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.google.common.collect.Sets;
 
 class PlayerInjector {
+	
+	/**
+	 * Marker interface that indicates a packet is fake and should not be processed.
+	 * @author Kristian
+	 */
+	public interface FakePacket {
+		// Nothing
+	}
 
 	// Cache previously retrieved fields
 	private static Field serverHandlerField;
@@ -28,15 +43,27 @@ class PlayerInjector {
 	private static Field inputField;
 	private static Field netHandlerField;
 	
+	// To add our injected array lists
+	private static StructureModifier<Object> networkModifier;
+	
 	// And methods
 	private static Method queueMethod;
 	private static Method processMethod;
 		
 	private Player player;
 	private boolean hasInitialized;
-
+	
 	// Reference to the player's network manager
-	private VolatileField networkManager;
+	private Object networkManager;
+	
+	// Current net handler
+	private Object netHandler;
+	
+	// Overridden fields
+	private List<VolatileField> overridenLists = new ArrayList<VolatileField>();
+	
+	// Packets to ignore
+	private Set<Packet> ignoredPackets = Sets.newSetFromMap(new ConcurrentHashMap<Packet, Boolean>());
 	
 	// The packet manager and filters
 	private PacketFilterManager manager;
@@ -44,9 +71,6 @@ class PlayerInjector {
 	
 	// Previous data input
 	private DataInputStream cachedInput;
-	
-	// Current net handler
-	private Object netHandler;
 
 	public PlayerInjector(Player player, PacketFilterManager manager, Set<Integer> packetFilters) throws IllegalAccessException {
 		this.player = player;
@@ -70,9 +94,11 @@ class PlayerInjector {
 			Object serverHandler = FieldUtils.readField(serverHandlerField, notchEntity);
 			
 			// Next, get the network manager 
-			if (networkManagerField == null)
+			if (networkManagerField == null) {
 				networkManagerField = FuzzyReflection.fromObject(serverHandler).getFieldByType(".*NetworkManager");
-			networkManager = new VolatileField(networkManagerField, serverHandler);
+				networkModifier = new StructureModifier<Object>(networkManagerField.getType(), null);
+			}
+			networkManager = FieldUtils.readField(networkManagerField, serverHandler);
 			
 			// And the queue method
 			if (queueMethod == null)
@@ -81,7 +107,7 @@ class PlayerInjector {
 			
 			// And the data input stream that we'll use to identify a player
 			if (inputField == null)
-				inputField = FuzzyReflection.fromObject(networkManager.getOldValue(), true).
+				inputField = FuzzyReflection.fromObject(networkManager, true).
 								getFieldByType("java\\.io\\.DataInputStream");
 		}
 	}
@@ -111,7 +137,7 @@ class PlayerInjector {
 		
 		// Get the handler
 		if (netHandler != null)
-			netHandler = FieldUtils.readField(netHandlerField, networkManager.getOldValue(), true);
+			netHandler = FieldUtils.readField(netHandlerField, networkManager, true);
 		return netHandler;
 	}
 	
@@ -152,12 +178,15 @@ class PlayerInjector {
 	 * @param InvocationTargetException If an error occured when sending the packet.
 	 */
 	public void sendServerPacket(Packet packet, boolean filtered) throws InvocationTargetException {
-		Object networkDelegate = filtered ? networkManager.getValue() : networkManager.getOldValue();
 		
-		if (networkDelegate != null) {
+		if (networkManager != null) {
 			try {
+				if (!filtered) {
+					ignoredPackets.add(packet);
+				}
+				
 				// Note that invocation target exception is a wrapper for a checked exception
-				queueMethod.invoke(networkDelegate, packet);
+				queueMethod.invoke(networkManager, packet);
 				
 			} catch (IllegalArgumentException e) {
 				throw e;
@@ -171,47 +200,101 @@ class PlayerInjector {
 		}
 	}
 	
+	@SuppressWarnings("serial")
 	public void injectManager() {
 		
 		if (networkManager != null) {
-			final Class<?> networkInterface = networkManagerField.getType();
+
-			final Object networkDelegate = networkManager.getOldValue();
+			@SuppressWarnings("rawtypes")
+			StructureModifier<List> list = networkModifier.withType(List.class);
 			
-			// Create our proxy object
+			// Subclass both send queues
-			Object networkProxy = Proxy.newProxyInstance(networkInterface.getClassLoader(), 
+			for (Field field : list.getFields()) {
-					new Class<?>[] { networkInterface }, new InvocationHandler() {
+				VolatileField overwriter = new VolatileField(field, networkManager, true);
 				
-				@Override
+				overwriter.setValue(Collections.synchronizedList(new ArrayList<Packet>() {
-				public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+					@Override
-					// OH OH! The queue method!
+					public boolean add(Packet packet) {
-					if (method.equals(queueMethod)) {
+
-						Packet packet = (Packet) args[0];
+						// Check for fake packets and ignored packets
-						
+						if (packet instanceof FakePacket) {
-						if (packet != null) {
+							return true;
+						} else if (ignoredPackets.contains(packet)) {
+							ignoredPackets.remove(packet);
+						} else {
 							packet = handlePacketRecieved(packet);
+						}
+						
+						// A NULL packet indicate cancelling
+						try {
+							if (packet != null) {
+								super.add(packet);
+							} else {
+								// We'll use the FakePacket marker instead of preventing the filters
+								sendServerPacket(createNegativePacket(packet), true);
+							}
 							
-							// A NULL packet indicate cancelling
+							// Collection.add contract
-							if (packet != null)
+							return true;
-								args[0] = packet;
+							
-							else
+						} catch (InvocationTargetException e) {
-								return null;
+							throw new RuntimeException("Reverting cancelled packet failed.", e.getTargetException());
 						}
 					}
-					
+				}));
-					// Delegate to our underlying class
+				overridenLists.add(overwriter);
-					try {
+			}
-						return method.invoke(networkDelegate, args);
-					} catch (InvocationTargetException e) {
-						throw e.getCause();
-					}
-				}
-			});
-		
-			// Inject it, if we can.
-			networkManager.setValue(networkProxy);
 		}
 	}
 	
+	/**
+	 * Used by a hack that reverses the effect of a cancelled packet. Returns a packet
+	 * whereby every int method's return value is inverted (a => -a).
+	 * 
+	 * @param source - packet to invert.
+	 * @return The inverted packet.
+	 */
+	private Packet createNegativePacket(Packet source) {
+		Enhancer ex = new Enhancer();
+		Class<?> type = source.getClass();
+		
+		// We want to subtract the byte amount that were added to the running
+		// total of outstanding packets. Otherwise, cancelling too many packets
+		// might cause a "disconnect.overflow" error.
+		//
+		// We do that by constructing a special packet of the same type that returns 
+		// a negative integer for all zero-parameter integer methods. This includes the
+		// size() method, which is used by the queue method to count the number of
+		// bytes to add.
+		//
+		// Essentially, we have:
+		//
+		//   public class NegativePacket extends [a packet] {
+		//      @Override
+		//      public int size() {
+		//         return -super.size();
+		//      }
+		//   ect.
+		//   }
+		ex.setInterfaces(new Class[] { FakePacket.class } );
+		ex.setUseCache(true);
+		ex.setClassLoader(type.getClassLoader());
+		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);
+				}
+			}
+		});
+		
+		return (Packet) ex.create();
+	}
+	
 	/**
 	 * Allows a packet to be recieved by the listeners.
 	 * @param packet - packet to recieve.
@@ -246,7 +329,7 @@ class PlayerInjector {
 				return cachedInput;
 			
 			// Save to cache
-			cachedInput = (DataInputStream) FieldUtils.readField(inputField, networkManager.getOldValue(), true);
+			cachedInput = (DataInputStream) FieldUtils.readField(inputField, networkManager, true);
 			return cachedInput;
 			
 		} catch (IllegalAccessException e) {
@@ -256,6 +339,9 @@ class PlayerInjector {
 	
 	public void cleanupAll() {
 		// Clean up
-		networkManager.revertValue();
+		for (VolatileField overriden : overridenLists) {
+			overriden.revertValue();
+		}
+		overridenLists.clear();
 	}
 }

ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java

@@ -37,7 +37,7 @@ public class StructureCache {
 		
 		// Use the vanilla class definition
 		if (result == null) {
-			result = new StructureModifier<Object>(MinecraftRegistry.getPacketClassFromID(id, true));
+			result = new StructureModifier<Object>(MinecraftRegistry.getPacketClassFromID(id, true), Packet.class);
 			structureModifiers.put(id, result);
 		}
 		

ProtocolLib/src/com/comphenix/protocol/reflect/StructureModifier.java

@@ -4,10 +4,14 @@ import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
+import com.google.gson.internal.Primitives;
 
 import net.minecraft.server.Packet;
 
@@ -25,27 +29,40 @@ public class StructureModifier<TField> {
 	private Class fieldType;
 	private List<Field> data = new ArrayList<Field>();
 	
+	// Improved default values
+	private Set<Field> defaultFields;
+	
 	// Cache of previous types
 	private Map<Class, StructureModifier> subtypeCache;
 	
-	public StructureModifier(Class targetType) {
+	public StructureModifier(Class targetType, Class superclassExclude) {
-		this(targetType, Object.class, getFields(targetType), null, new HashMap<Class, StructureModifier>());
+		List<Field> fields = getFields(targetType, superclassExclude);
-	}
+		
-	
+		initialize(targetType, Object.class, 
-	private StructureModifier(Class targetType, Class fieldType, List<Field> data, 
+				fields, generateDefaultFields(fields), null, 
-							  EquivalentConverter<TField> converter, Map<Class, StructureModifier> subTypeCache) {
+				new HashMap<Class, StructureModifier>());
-		this.targetType = targetType;
-		this.fieldType = fieldType;
-		this.data = data;
-		this.converter = converter;
-		this.subtypeCache = subTypeCache;
 	}
 	
 	private StructureModifier(StructureModifier<TField> other, Object target) {
-		this(other.targetType, other.fieldType, other.data, other.converter, other.subtypeCache);
+		initialize(other.targetType, other.fieldType, other.data, other.defaultFields, other.converter, other.subtypeCache);
 		this.target = target;
 	}
 	
+	private StructureModifier() {
+		// Consumers of this method should call "initialize"
+	}
+	
+	private void initialize(Class targetType, Class fieldType, 
+			  List<Field> data, Set<Field> defaultFields,
+			  EquivalentConverter<TField> converter, Map<Class, StructureModifier> subTypeCache) {
+		this.targetType = targetType;
+		this.fieldType = fieldType;
+		this.data = data;
+		this.defaultFields = defaultFields;
+		this.converter = converter;
+		this.subtypeCache = subTypeCache;
+	}
+	
 	/**
 	 * Reads the value of a field given its index.
 	 * @param fieldIndex - index of the field.
@@ -67,6 +84,20 @@ public class StructureModifier<TField> {
 		else
 			return (TField) result;
 	}
+
+	/**
+	 * Reads the value of a field if and ONLY IF it exists.
+	 * @param fieldIndex - index of the field.
+	 * @return Value of the field, or NULL if it doesn't exist.
+	 * @throws IllegalAccessException If we're unable to read the field due to a security limitation.
+	 */
+	public TField readSafely(int fieldIndex) throws IllegalAccessException {
+		if (fieldIndex >= 0 && fieldIndex < data.size()) {
+			return read(fieldIndex);
+		} else {
+			return null;
+		}
+	}
 	
 	/**
 	 * Writes the value of a field given its index.
@@ -89,6 +120,20 @@ public class StructureModifier<TField> {
 		return this;
 	}
 	
+	/**
+	 * Writes the value of a given field IF and ONLY if it exists.
+	 * @param fieldIndex - index of the potential field.
+	 * @param value - new value of the field.
+	 * @return This structure modifer - for chaining.
+	 * @throws IllegalAccessException If we're unable to write to the field due to a security limitation.
+	 */
+	public StructureModifier<TField> writeSafely(int fieldIndex, TField value) throws IllegalAccessException {
+		if (fieldIndex >= 0 && fieldIndex < data.size()) {
+			write(fieldIndex, value);
+		}
+		return this;
+	}
+	
 	/**
 	 * Correctly modifies the value of a field.
 	 * @param fieldIndex - index of the field to modify.
@@ -110,6 +155,22 @@ public class StructureModifier<TField> {
 		return withType(fieldType, null);
 	}
 	
+	/**
+	 * Sets all non-primitive fields to a more fitting default value. See {@link DefaultInstance}.
+	 * @return The current structure modifier - for chaining.
+	 * @throws IllegalAccessException If we're unable to write to the fields due to a security limitation.
+	 */
+	public StructureModifier<TField> writeDefaults() throws IllegalAccessException {
+		
+		// Write a default instance to every field
+		for (Field field : defaultFields) {
+			FieldUtils.writeField(field, target, 
+					DefaultInstances.getDefault(field.getType()), true);
+		}
+		
+		return this;
+	}
+	
 	/**
 	 * Retrieves a structure modifier that only reads and writes fields of a given type.
 	 * @param fieldType - the type, or supertype, of every field to modify.
@@ -128,16 +189,21 @@ public class StructureModifier<TField> {
 			
 		} else if (result == null) {
 			List<Field> filtered = new ArrayList<Field>();
+			Set<Field> defaults = new HashSet<Field>();
 			
 			for (Field field : data) {
 				if (fieldType.isAssignableFrom(field.getType())) {
 					filtered.add(field);
+					
+					if (defaultFields.contains(field))
+						defaults.add(field);
 				}
 			}
 			
 			// Cache structure modifiers
-			result = new StructureModifier<T>(targetType, fieldType, filtered, 
+			result = new StructureModifier<T>();
-											  converter, new HashMap<Class, StructureModifier>());
+			result.initialize(targetType, fieldType, filtered, defaults, 
+							  converter, new HashMap<Class, StructureModifier>());
 			subtypeCache.put(fieldType, result);
 		}
 		
@@ -199,8 +265,37 @@ public class StructureModifier<TField> {
 		return copy;
 	}
 	
+	/**
+	 * Retrieves a list of the fields matching the constraints of this structure modifier.
+	 * @return List of fields.
+	 */
+	public List<Field> getFields() {
+		return ImmutableList.copyOf(data);
+	}
+	
+	// Used to generate plausible default values
+	private static Set<Field> generateDefaultFields(List<Field> fields) {
+		
+		Set<Field> requireDefaults = new HashSet<Field>();
+
+		for (Field field : fields) {
+			Class<?> type = field.getType();
+			
+			// First, ignore primitive fields
+			if (!Primitives.isPrimitive(type)) {
+				// Next, see if we actually can generate a default value
+				if (DefaultInstances.getDefault(type) != null) {
+					// If so, require it
+					requireDefaults.add(field);
+				}
+			}
+		}
+		
+		return requireDefaults;
+	}
+	
 	// Used to filter out irrelevant fields
-	private static List<Field> getFields(Class type) {
+	private static List<Field> getFields(Class type, Class superclassExclude) {
 		List<Field> result = new ArrayList<Field>();
 		
 		// Retrieve every private and public field
@@ -208,7 +303,10 @@ public class StructureModifier<TField> {
 			int mod = field.getModifiers();
 			
 			// Ignore static, final and "abstract packet" fields
-			if (!Modifier.isFinal(mod) && !Modifier.isStatic(mod) && !field.getDeclaringClass().equals(Packet.class)) {
+			if (!Modifier.isFinal(mod) && !Modifier.isStatic(mod) && (
+					superclassExclude == null || !field.getDeclaringClass().equals(Packet.class)
+				)) {
+				
 				result.add(field);
 			}
 		}

ProtocolLib/test/com/comphenix/protocol/reflect/StructureModifierTest.java

@@ -6,6 +6,7 @@ import net.minecraft.server.Packet103SetSlot;
 
 import org.junit.Test;
 
+import com.avaje.ebeaninternal.server.cluster.Packet;
 import com.comphenix.protocol.reflect.StructureModifier;
 
 public class StructureModifierTest {
@@ -14,7 +15,7 @@ public class StructureModifierTest {
 	public void test() throws IllegalAccessException {
 
 		Packet103SetSlot move = new Packet103SetSlot();
-		StructureModifier<Object> modifier = new StructureModifier<Object>(Packet103SetSlot.class);
+		StructureModifier<Object> modifier = new StructureModifier<Object>(Packet103SetSlot.class, Packet.class);
 
 		move.a = 1;
 		int value = (Integer) modifier.withTarget(move).withType(int.class).read(0);